Not as easy as it looks, Part Two

re: Not as easy as it looks, Part Two

Shuggy — Mon, 24 Jan 2011 13:03:41 GMT

@Reordering.

Ugh that's awful. Either work through it to understand what was the connections are and understand it/split it up/ sort it or leave it as is (low risk, no nasty to merge/annotate changes)

re: Not as easy as it looks, Part Two

Reordering — Mon, 24 Jan 2011 08:42:50 GMT

Classes get in this state because after x years y people have worked on them and they have added code somewhere.

You get committed to extend an existing application with new features, the application is already 4 years old, you receive the code and it looks like this one, what do you do? Do you think to refactor it? No, absolutely not! It's not your duty, nobody asked you to do it and you won't lose your time doing it, you are paid to add new feature not to reordering a class, moreover, as already said, it's really dangerous reordering a class by hand, the risk of a copy paste error is very high.

At the end you add your code at the end or at the top of the class, and so will do the next one who has to add some new code. At this point have a function to reorder classes could be really useful, at least you get a better structure than a tasty spaghetti class.

re: Not as easy as it looks, Part Two

configurator — Sat, 22 Jan 2011 01:31:48 GMT

@Reordering: How would a class get that way anyway? Unless you just drop code off at the end of the class when you need to add anything, which is a very bad practice, the class should be organized according to whatever order it is that you like them to be, which would normally not be alphabetic or according to accessibility.

re: Not as easy as it looks, Part Two

Reordering — Thu, 20 Jan 2011 13:02:53 GMT

Perhaps, still it could be a starting point, I stopped to use resharper because of the useless comments of the type "make a method static" or "don't use the "this" keyword" and moreover it had some bugs which make it worst than the built in functions of visual studio, like the find all references, it was replaced with a resharper method which for some reason didn't find all references.

But at this point I would suggest to remove also the "sort and remove the using" option from VS because it could order or remove them in an way that somebody doesn't like, so because somebody doesn't like it nobody should use it, correct?

And for me make more sense a class ordered like a in the class diagram than a spaghetti class:

public class MySpaghettiClass

{

public void OnSpathettiAreUsed(){TheSpaghettiAreUsed(this,EventArgs.Empty);}

public void GetSomeSause() {....}

public event EventHandler TheSpaghettiAreThere;

private int howManySpaghetti=0;

public string TheName {get; set;}

pubclic void IsSomeSpaghettiThere(){ ...}

public MySpaghettiClass() : this(11) {}

public int HowManySpaghettiAreLeft(){return _spaghetti;}

public event EventHandler TheSpaghettiAreUsed;

public int HowManySpaghetti{ger{return howManySpaghetti;} set{ howManySpaghetti=value;}}

public MySpaghettiClass(istring name) {TheName=name;}

public void UseSpaghetti(int spaghetti){howManySpaghetti-=spaghetti; OnSpaghettiAreUsed();}

....

}

re: Not as easy as it looks, Part Two

configurator — Wed, 19 Jan 2011 13:47:53 GMT

Reordering: if that happens to you, you're doing it wrong.

That said, if you do have a method to reorder a class automatically to an order that doesn't make sense, it still wouldn't help you...

re: Not as easy as it looks, Part Two

Weeble — Wed, 19 Jan 2011 10:37:05 GMT

(Sorry if this is a double-post. The first attempt to post seemed to do nothing at all.)

Resharper does member reordering. I learned this the hard way.

At a previous company we had a moderate-to-large size of code-base with maybe 20 to 30 developers. We were building a game, but we had lots of associated command-line programs too. At some point the command-line programs

all

started

writing

their

--help

output

this.

It seems that nobody noticed or cared for a while, because they rarely invoked the help. I only noticed when I was adding a new command-line tool. It turned out that the help tool, for reasons best known to the original author, used P/Invoke to determine the dimensions of the console (rather than, say, Console.BufferWidth) in order to do word-wrapping. It called GetConsoleScreenBufferInfo, for which it needed to define the structs CONSOLE_SCREEN_BUFFER_INFO, SMALL_RECT and COORD. More recently, somebody had applied what was *supposed* to be a global clean-up using Resharper in order to bring things like spacing, identifiers and comments in-line with our newly agreed coding style. Unfortunately this clean-up also alphabetically sorted the members of every class, and somehow this wasn't noticed when it was checked in. Since in C# structs default to LayoutKind.Sequential, the original code was correct, but alphabetically sorted it became gibberish.

I removed all the P/Invoke and replaced them with Console.BufferWidth, grumbled at the people who had re-ordered everything, and wondered why anyone would even *want* to sort their members alphabetically. It makes about as much sense to me as ordering a music collection by the colour of the album art.

re: Not as easy as it looks, Part Two

Reordering — Wed, 19 Jan 2011 10:11:06 GMT

At the beginning the class is good organized and the class entropy is very low. Then a few changes are needed, which are added somewhere in the class structure, but still the class is good organized, but those few changes begin to raise the class entropy. After a couple of years of "few changes" the class entropy is so high that it would take too much time to reorganize it and the risk of a copy paste error is concrete. This means the class has already overcome the point of no return, the entropy now will only grow higher, it's too late, bye bye good organized class, welcome tasty spaghetti class, the can of worms is open and nobody will close it.

re: Not as easy as it looks, Part Two

configurator — Wed, 19 Jan 2011 03:06:27 GMT

Reordering: But that would destroy the natural order of a class... Take this example, to make things clear.

Class RectangleCalculator has a public interface which accept a Rectangle, two Points or four integers

public int Area(Rectangle r);

public int Area(Point p1, Point p2);

public int Area(int x1, int y1, int x2, int y2);

public int Perimeter(Rectangle r);

public int Perimeter(Point p1, Point p2);

public int Perimeter(int x1, int y1, int x2, int y2);

Each of these does the argument validation, and then they delegate to the private CalculateArea and CalculatePermiter.

Wouldn't it make sense to order the class like this?

public int Area(Rectangle r);

public int Area(Point p1, Point p2);

public int Area(int x1, int y1, int x2, int y2);

private int CalculateArea(Whatever parameters);

public int Perimeter(Rectangle r);

public int Perimeter(Point p1, Point p2);

public int Perimeter(int x1, int y1, int x2, int y2);

private int CalculatePerimeter(Whatever parameters);

re: Not as easy as it looks, Part Two

configurator — Wed, 19 Jan 2011 03:01:22 GMT

Weeble: That's a good point! Nobody mentioned that...

re: Not as easy as it looks, Part Two

configurator — Wed, 19 Jan 2011 03:00:02 GMT

DRBlaise: Static variables can still be disambiguated although not as easily - B.y here would disambiguate just fine.