Ayende @ Rahien

Refunds available at head office

A good refactoring session is…

One in which I get to remove a lot of code. Here is a particularly interesting tidbits, SqlStatement is one of the core classes in NH Prof, look what I did to it:

image

For those of you not used to reading diffs, the orange marking on the side bars are removed lines. It dropped from a 484 lines to 153.

Comments

Frank Quednau
05/27/2009 09:09 PM by
Frank Quednau

I tend to say that the best line of code is the one you never wrote, but this is the next best thing.

Kelly Stuard
05/27/2009 09:19 PM by
Kelly Stuard

So obviously that means it does 30% less now. Or that you put 30% of the code on one line. Oh, I know ... the curley braces are no longer on their own line! Because, of course, there is no way to get rid of or refactor code.

Kelly Stuard
05/27/2009 09:19 PM by
Kelly Stuard

BTW: not liking that I have to fire up an alternate browser to IE8 to post comments - not even compat mode works

Kelly Stuard
05/27/2009 09:21 PM by
Kelly Stuard

Oh, and there was a tag at the end of the first post; but I guess it got eaten.

Gloubidou
05/27/2009 09:22 PM by
Gloubidou

YUICompressor for CSharp :-)

configurator
05/28/2009 12:04 AM by
configurator

Clearly this:

class SqlStatement { public string Whatever { get { return whatever; } set { whatever = value; } } private string whatever; ... }

is much better than this:

class SqlStatement {

public string Whatever {

  get {

     return whatever;

  }

  set {

     whatever = value;

  }

}

private string whatever;

...

}

Jeremy Wiebe
05/28/2009 05:00 AM by
Jeremy Wiebe

So did some of the functionality move to other classes or were you really able to remove ~300 lines and maintain similar functionality?

Erik van Brakel
05/28/2009 08:53 AM by
Erik van Brakel

@configurator:

Sometimes, putting stuff on one line makes the whole more readable. I often use that with properties, to cut down on the curly-braces-porn that's going on in there:

public string Whatever { get { return whatever; } set { whatever = value; } }

public string Whenever { get { return whenever; } set { whenever = value; } }

public string However { get { return however; } set { however = value; } }

Or, if you don't like it like that:

public string Whatever

{

get { return whatever; }

set { whatever = value; }

}

En then continue using proper spacing in the rest of the code. Those properties are SO obvious that you don't really need to space it out over 8 lines.

Either way, I don't think Ayende just removed whitespace, else he wouldn't be so happy with it ;-)

Ian Nelson
05/28/2009 09:50 AM by
Ian Nelson

Admit it Ayende, you moved 300+ lines of code to static methods in a SqlStatementHelper class? ;-)

Ayende Rahien
05/28/2009 12:05 PM by
Ayende Rahien

Jeremy,

I create a separate class for handling that

Ayende Rahien
05/28/2009 12:06 PM by
Ayende Rahien

Ian,

Almost, it is called SqlStatementProcessor, and it is a command object

Jeremy Wiebe
05/28/2009 12:14 PM by
Jeremy Wiebe

Ah, so the refactoring was geting SqlStatement to be more focused. I'm always a big fan of those types of refactorings. :-)

Stephen
05/28/2009 12:21 PM by
Stephen

So you didn't just randomly comment lines out and cross your fingers when hitting compile?

I remember going from a prototype to a 'v0.1' the code was about a fifth the size, was faster, easier to program against and was far more extensible..

I don't think thats so uncommon for a prototype to be 'obese' but it was really nice that its evolution was so much smaller and yet tons better.

(usual day for a dev right? I always enjoy that feeling anyway, I do a lap around the office for high fives borat style).

Comments have been closed on this topic.