views:

112

answers:

7

I am recieving warning about my file by StyleCop.

  • Warning 1 CR0005: The file is very long at 508 lines. Please consider refactoring to make it 500 or fewer lines.

  • Warning 2 CR0002: Method is too long. It is 58 lines long. Consider refactoring to make it 50 or fewer lines.

How are you guys making changes to your codes? What are the best practices for this? I have no idea to divide my codes to smaller ones - being afraid of making my codes become so-complex.

So, please help ^_^ !

Nam.

+4  A: 

You should read Martin's Fowler book "Refactoring: Improving the Design of Existing Code" and "Professional Refactoring in C# & ASP.NET" of Danijel Arsenovski.

Eugene Cheverda
And **Clean Code by Robert C. Martin** so you don't end up making the same mistakes
Chad
+4  A: 

Does the class try do to much? Could it be split into multiple smaller classes that each had a more specific and better defined purposes? If so, refactor it into multiple classes.

Could some code from the method be extracted out into it's own method to make it easier to understand? If so, do so.

Would either of the changes above make the code more difficult to understand? If so, ignore StyleCop. Remember, it's just a generic tool to help make your code easier to read. There will almost certainly be at least some recommendations that won't make sense for your circumstances.

ho1
+1  A: 

I'd suppress the warnings and worry about more important things.

I'm not sure it makes sense to impose such an arbitrary limit on the size of a file or method. It's not so much the numbers 50 & 500 themselves, but the fact that there is such a number. Where does it come from? Why is 50 lines considered readable, but 58 isn't?

As demonstrated here, concentrating on these metrics can be counter productive, and draw attention away from real issues. Perhaps time might be better spent, and good design principles better served, ensuring something like proper separation of concerns, for example. Split your lines and methods according to what should logically go in them, rather than breaking them up to meet arbitrary size criteria.

Winston Smith
I think a reasonable limit for a method is what you can fit on a screen, as you can read it all without scrolling. 50 is pretty arbitrary, but it's around that size so seems reasonable. You can always suppress the warnings if you've a good reason for a long method.
Grant Crofton
+1. The thought alone, having uncle bob as an automated program telling you to add extra interfaces or making that one line more 'solid' in the face of real deadlines. Just what you need....
Toad
@Grant Of course, the number of lines that fits on screen depends on the screen resolution and font size, so again that's arbitrary.
Winston Smith
@Winston sure, but if you're running StyleCop, presumably you're interested in checking your code against some metrics, many of which will be pretty arbitrary. And (@Toad), while I'm not sure StyleCop really helps with this, I personally don't think making your code well structured is a waste of time, regardless of deadlines.
Grant Crofton
@Grant My point is that *well structured* doesn't mean *less than X lines*
Winston Smith
@Winston I agree, although large methods/classes can be an indication of bad structure. I was more addressing @Toad's comment about interfaces and SOLID principles.
Grant Crofton
A: 

Well, the 'Extract Method' refactoring is very useful for making methods shorter (by putting them into another method), which I believe is in 2010. (Highlight some code, right click and it should be in the menu somewhere).

The best way to break a file up (assuming you only have one class in it) is to extract some of the functionality into another class. Google 'Extract Class' and you'll find some info on it.

Like @Justin says, doing this might seem more complex at first because there are more files/methods to deal with, but because each file/method is smaller, there's less to deal with at any one time. Some (respected) people take this really far. It takes a little getting used to but you code will be (arguably) better for it.

Grant Crofton
A: 

Others have mentioned refactoring, and also focusing on breaking the class down to do just 1x responsibility (part of the S.O.L.I.D. rules for OOP). However; if your class is still 500x lines, and performing one responsibility, then you're not in too terrible a position.

If your code-file contains XML documentation and white-spaces, then 500x lines is only slightly large (and that's dependent on what it does). Many of my "simple" classes end up around 350 lines once completed. Smaller is better, but concise is really what you want.

Another good book for understanding how your code should look is Robert C. Martin's Clean Code. It lays down many good rules in designing well thought-out classes and methods.

STW
A: 

It is possible to split the definition of a class or a struct, or an interface over two or more source files. Each source file contains a section of the class definition, and all parts are combined when the application is compiled. For example here the 'bar' class is split between foo1.cs and foo2.cs.

foo1.cs

public partial class Bar
{
    public void bat()
    {
    }
}

foo2.cs

public partial class Bar
{
    public void baz()
    {
    }
}

For more information on this see Partial Class Definitions (C# Programming Guide)

Fraser
A: 

This is not a rule that ships with StyleCop. Is it something your company developed in house? I'm curious about the rationale behind the rule. 500 lines seems like a pretty strict limitation.

Jason Allor