Ideological Programming Question...
-
Would you break up a method into more then one method just to make it smaller even if the code is not going to be used elsewhere? The program I am working on has one method that will probably be about 200-250 lines of code. Is too much for a single method? Should I create a couple helper methods just to make it more readable?
I didn't get any requirements for the signature
IMHO 200 lines of code is at least 10 times the size a method should be if you want it to be testable. Make it testable, and maintainable will follow quite happily. :)
Anna :rose: Having a bad bug day? Tech Blog | Anna's Place | Tears and Laughter "If mushy peas are the food of the devil, the stotty cake is the frisbee of God"
-
Would you break up a method into more then one method just to make it smaller even if the code is not going to be used elsewhere? The program I am working on has one method that will probably be about 200-250 lines of code. Is too much for a single method? Should I create a couple helper methods just to make it more readable?
I didn't get any requirements for the signature
-
ToddHileHoffer wrote:
Would you break up a method into more then one method just to make it smaller even if the code is not going to be used elsewhere?
sure, as long as it's an obvious break. i'm not so picky about function size that i'll spend too much time trying to find ways to break down a function that gets a little long. and if it's a performance-critical function, i'll avoid function calls as best i can.
Chris Losinger wrote:
and if it's a performance-critical function, i'll avoid function calls as best i can
Unmanaged code - yes, managed code - maybe. The CLR JIT compiler operates on method level granularity, so if the method has sections of code that is executed very rarely, for example, then moving them to a separate method actually improves performance - the JITter is faster because it doesn't have to JIT the other method right away, the generated code is smaller etc..
Regards Senthil [MVP - Visual C#] _____________________________ My Home Page |My Blog | My Articles | My Flickr | WinMacro
-
Would you break up a method into more then one method just to make it smaller even if the code is not going to be used elsewhere? The program I am working on has one method that will probably be about 200-250 lines of code. Is too much for a single method? Should I create a couple helper methods just to make it more readable?
I didn't get any requirements for the signature
One of the really cool (non-functional) features of Visual Studio is regions. I like to group sections of code together with regions if the code becomes less self documenting, and then I often find myself extracting an entire region to a sub method later...
____________________________________________________________ Be brave little warrior, be VERY brave
-
Would you break up a method into more then one method just to make it smaller even if the code is not going to be used elsewhere? The program I am working on has one method that will probably be about 200-250 lines of code. Is too much for a single method? Should I create a couple helper methods just to make it more readable?
I didn't get any requirements for the signature
Yes. If I can make a code sequence easier to understand by breaking it out into a function with a good name, I'll do it -- even if the original function only holds five lines... As todays compilers are rather smart the cost of function calls are virtually nil and making it easy for the next guy (who might be your elderly alzheimerish self) is a righteous goal in itself. Now, if I only could find a way to break up a VB6 select case monster... [voice fading away]
-
Would you break up a method into more then one method just to make it smaller even if the code is not going to be used elsewhere? The program I am working on has one method that will probably be about 200-250 lines of code. Is too much for a single method? Should I create a couple helper methods just to make it more readable?
I didn't get any requirements for the signature
As a hobby OS developing, I create bootable hex editor, which a __cdecl based draw function of more than 1000 lines of ASM (Since NASM accept only one instruction per line). Then I tried to optimize it (for a 512-bytes bootable program contest). After I spent around four hours trying to understand what and where MY program do expected tasks, I decided to rewrite it, but this time using comments. So you can have 200-250 lines for method if you keep comments, but I really suggest you NOT to be in ASM ;)
-
Would you break up a method into more then one method just to make it smaller even if the code is not going to be used elsewhere? The program I am working on has one method that will probably be about 200-250 lines of code. Is too much for a single method? Should I create a couple helper methods just to make it more readable?
I didn't get any requirements for the signature
It depends as to whether or not the logic of the operation belongs together, typically though if I get more than about 150 lines I'll try and break it up or if I end up going more than about 4-5 conditional statements deep. Also if the code is doing business logic, database access and file I/O then I'll break those parts out into separate functions and possible even different code files/projects.
-
I think the men in the white coats will be along soon...
-
Would you break up a method into more then one method just to make it smaller even if the code is not going to be used elsewhere? The program I am working on has one method that will probably be about 200-250 lines of code. Is too much for a single method? Should I create a couple helper methods just to make it more readable?
I didn't get any requirements for the signature
Readability, Readability, Readability. Oh yeah. It should be understandable. Sure. Break it up a little, into logical pieces. Make it yours. It sounds like it might be any way. After our family packed up from a weekend of camping, we'd stand side-by-side and "walk" our camp site, and pick up any trash or sticks. We'd leave the site in better condition than we found it. I apply the same policy to codeing. Leave the site in better condition than you found it. Break up a method here. Comment better there. ... you might return there again someday. ;)
-
Chris Losinger wrote:
and if it's a performance-critical function, i'll avoid function calls as best i can
Unmanaged code - yes, managed code - maybe. The CLR JIT compiler operates on method level granularity, so if the method has sections of code that is executed very rarely, for example, then moving them to a separate method actually improves performance - the JITter is faster because it doesn't have to JIT the other method right away, the generated code is smaller etc..
Regards Senthil [MVP - Visual C#] _____________________________ My Home Page |My Blog | My Articles | My Flickr | WinMacro
But doesn't the compiler ever inline called code? Say I have a small block of just a few lines of code, but that is used in multiple places. For the sake of an example, let's say a "blend" method that takes a foreground and background pixel and an alpha parameter. If I now write a method to do this over an entire image, I'd have a nested loop where the inner loop called blend() on each iteration. It seems clear to me that it'd be nice if this was inlined, though at the same time I would of course love not having to introduce redundancy in my code. I believe the compiler - when compiling "with optimization" - does this sort of thing, but I often thought it would have been nice to have some way of telling the compiler where I'd like it to inline code, leave code external, or - by default - decide for itself what to do. It seems to me a preprocessor directive would be appropriate, or even an attribute, like this:
[Inline("blend")]
Image blend(Image foreground, Image background, double alpha)
{
// ...
for (x=0; x < result.Width; x++)
for (y=0; y < result.Height; y++)
result.SetPixel(x, y, blend(...));
return result;
} -
I think it is worth a little extra effort to split up the functionality so that it will be easier to maintain.
I didn't get any requirements for the signature
More important is to do this sort of thing as you go. Or if you *anyway* have to maintain some code because it needs to change - whether because of bugs or changed requirements or that it's too slow. Spending time on code that may never need to change is a bad business proposition, at least when the cost of making the change is no higher should it become necessary down the road - as seems to be the case here since it wasn't you who wrote it. (If you've recently written your own method and realize it's not easy to understand, by all means go ahead and do something about it!) And do not forget that the code that most needs cleanup is the hardest to clean up without introducing new bugs. It may be aesthetically displeasing to see the awfulness, but if there's a high risk of breaking stuff I'd advise against touching anything without a prior discussion with the powers that be, so they can take the risks into account and plan proper testing and quality control.
-
But doesn't the compiler ever inline called code? Say I have a small block of just a few lines of code, but that is used in multiple places. For the sake of an example, let's say a "blend" method that takes a foreground and background pixel and an alpha parameter. If I now write a method to do this over an entire image, I'd have a nested loop where the inner loop called blend() on each iteration. It seems clear to me that it'd be nice if this was inlined, though at the same time I would of course love not having to introduce redundancy in my code. I believe the compiler - when compiling "with optimization" - does this sort of thing, but I often thought it would have been nice to have some way of telling the compiler where I'd like it to inline code, leave code external, or - by default - decide for itself what to do. It seems to me a preprocessor directive would be appropriate, or even an attribute, like this:
[Inline("blend")]
Image blend(Image foreground, Image background, double alpha)
{
// ...
for (x=0; x < result.Width; x++)
for (y=0; y < result.Height; y++)
result.SetPixel(x, y, blend(...));
return result;
}dojohansen wrote:
but I often thought it would have been nice to have some way of telling the compiler where I'd like it to inline code,
There is one - MethodImplOptions.NoInlining[^] tells the JITter not to inline the method. This[^] is a (rather old) good blog post on JIT inlining. But yeah, if you split the code into methods, the JITter has an option - it can choose to inline or not. With one single large method, the JITter has no option but to generate machine code for the entire method body. The blog post I referred to also says that the quality of generated code goes down as the amount of code to JIT increases, so it's a win both ways :).
Regards Senthil [MVP - Visual C#] _____________________________ My Home Page |My Blog | My Articles | My Flickr | WinMacro
-
Would you break up a method into more then one method just to make it smaller even if the code is not going to be used elsewhere? The program I am working on has one method that will probably be about 200-250 lines of code. Is too much for a single method? Should I create a couple helper methods just to make it more readable?
I didn't get any requirements for the signature
-
Would you break up a method into more then one method just to make it smaller even if the code is not going to be used elsewhere? The program I am working on has one method that will probably be about 200-250 lines of code. Is too much for a single method? Should I create a couple helper methods just to make it more readable?
I didn't get any requirements for the signature
My metric is usually about 50 lines of code at the most. Many times it doesn't even get to that for me to split the code. It is much more readable, easier to mantain and debug. So, to answer your question, IMHO, yes, 200 lines are too much.
-
Would you break up a method into more then one method just to make it smaller even if the code is not going to be used elsewhere? The program I am working on has one method that will probably be about 200-250 lines of code. Is too much for a single method? Should I create a couple helper methods just to make it more readable?
I didn't get any requirements for the signature
Before Visual Studio 2005's "Refactor -> Extract Method" I'd have said 200 - 250 lines was pushing it but not worth missing a deadline over. With Extract Method, it's a cinch to go back and move large swaths of code into a method call so it's easier to be a little tighter on the length of a function. I have never gotten to the point where every method is entirely visible on a single page. People who sprang for VisualAssist (or whatever it was called) may have a lower tolerance for long methods.
-
IMHO 200 lines of code is at least 10 times the size a method should be if you want it to be testable. Make it testable, and maintainable will follow quite happily. :)
Anna :rose: Having a bad bug day? Tech Blog | Anna's Place | Tears and Laughter "If mushy peas are the food of the devil, the stotty cake is the frisbee of God"
I have to sorta object here. If you think of every conditional as doubling (at least) the number of tests required for complete coverage, and estimate that 1/10th of all lines of source are conditionals, then a single method that has 200 lines of code has at least 20 conditions. That's 2^20 variations to test (~million). Add a few more such methods and it quickly becomes entirely infeasible to test every execution path. Testing is critical. Very important. But testing is not a substitute for good coding habits. "Provable Correctness" has not yet found its way into the commercial world and testing is no shortcut to it. Apologies for hijacking this thread!
-
Dalek Dave wrote:
Who will be caring for the poor wee beastie?
Well, this is a problem. Another "programmer" was responsible for this project. But he really can't cut the mustard, so I am doing all the coding. My boss said he will be responsible for it and if not he'll be out of job... Since this other "programmer" has not helped at all, for he has not written one line of code, I don't know what is going to happen. It will probably be me for a little while...
I didn't get any requirements for the signature
ToddHileHoffer wrote:
Well, this is a problem. Another "programmer" was responsible for this project. But he really can't cut the mustard, so I am doing all the coding. My boss said he will be responsible for it and if not he'll be out of job... Since this other "programmer" has not helped at all, for he has not written one line of code, I don't know what is going to happen. It will probably be me for a little while...
Then do the poor backstitch a favor and break it up into subroutines with easily understood names so that, when he breaks it, he'll break it in one spot that can be trapped with ease. Large blocks of code are almost always more difficult to debug because the time it takes for the eyes to traverse them can cause "code blindness". Also, it's easier to label an error message with a subroutine name than try to designate a block of larger code with a label. Finally, if each subroutine that is used by the method implements its own error handling including a subroutine-speoific message, even this other should be able to isolate the error pretty quickly...and get enough help to get the thing fixed before his boss notices.
-
ToddHileHoffer wrote:
Well, this is a problem. Another "programmer" was responsible for this project. But he really can't cut the mustard, so I am doing all the coding. My boss said he will be responsible for it and if not he'll be out of job... Since this other "programmer" has not helped at all, for he has not written one line of code, I don't know what is going to happen. It will probably be me for a little while...
Then do the poor backstitch a favor and break it up into subroutines with easily understood names so that, when he breaks it, he'll break it in one spot that can be trapped with ease. Large blocks of code are almost always more difficult to debug because the time it takes for the eyes to traverse them can cause "code blindness". Also, it's easier to label an error message with a subroutine name than try to designate a block of larger code with a label. Finally, if each subroutine that is used by the method implements its own error handling including a subroutine-speoific message, even this other should be able to isolate the error pretty quickly...and get enough help to get the thing fixed before his boss notices.
Yes, I created six well named subroutines. I have it down to about 50 lines of code. It is much more readable.
I didn't get any requirements for the signature
-
Yes, I created six well named subroutines. I have it down to about 50 lines of code. It is much more readable.
I didn't get any requirements for the signature
ToddHileHoffer wrote:
Yes, I created six well named subroutines. I have it down to about 50 lines of code. It is much more readable.
Yup, sounds about right. Now, comment the living heck out of it, with some explanations of what's going on as well as (if you can figure this out) what the most likely errors might be in the various sections. I've programmed for the chimpanzee class before. Every little bit of help can make the difference between a usefully employed chimpanzee or an empty desk waiting for another potential Frankenstein's Monster.
-
I have to sorta object here. If you think of every conditional as doubling (at least) the number of tests required for complete coverage, and estimate that 1/10th of all lines of source are conditionals, then a single method that has 200 lines of code has at least 20 conditions. That's 2^20 variations to test (~million). Add a few more such methods and it quickly becomes entirely infeasible to test every execution path. Testing is critical. Very important. But testing is not a substitute for good coding habits. "Provable Correctness" has not yet found its way into the commercial world and testing is no shortcut to it. Apologies for hijacking this thread!
That's exactly why I consider that keeping functions short is important. A 200 line function is not likely to be testable; a 20 line one is. :) Although mathematical correctness of code is unlikely to be provable in the general case, a good test coverage can mitigate this by warning when behaviour has changed in an unintended way - perhaps as an unintended side effect of a change. If you disagree, you could always come along to the ACCU Conference[^] next year and present your hypothesis. If you do, believe me, the debate will be lively and interesting[^]... ;)
Anna :rose: Having a bad bug day? Tech Blog | Anna's Place | Tears and Laughter "If mushy peas are the food of the devil, the stotty cake is the frisbee of God"
modified on Wednesday, June 3, 2009 12:14 PM