Jeffrey Palermo 12:12 am on April 11, 2006
Tags: Coding Principles ( 26 )

How to design a single method – level 200

What? I know how to design a single method! What can
Jeffrey Palerm possibly have to tell me about how to design a single
method?

Take a look at your product’s code base. Are any of the following true?:

    * You have a method that you can’t view without scrolling.
    * You have a method that takes 10 arguments.

* You have a method that not only returns a result but causes
disastrous things to happen if called more than once.
    * You have a method that throws exceptions if you don’t set certain properties first.
    * You have a method that has 5 nested foreach/if/try-catch code blocks.
    * You have a method whose name doesn’t give you a clue as to what it does.
    * You have a method that will not throw an exception because it swallows the exception without handling it.

I
could go on and on (ok, actually I’m out of ideas right now), but
chances are that you can find a method that meets one of the criteria
above. Find one of those methods, and fix it. Here are
general rules I try to live by when designing a single method:

    * The name of the method should describe what the method actually does.
    * A method should do one and only one thing.

* A method should either DO something or RETURN something. If it
returns something, you should be able to call the method over and over
with no negative consequences.
    * The method
should be viewable without scrolling. Fewer than 10 lines of code
is desirable (my pain point).
    * The method should
take few arguments. If the method needs more, refactor to require
and object that contains all the information required (parameter
object).
    * The method depends only on method parameters or members passed into the class constructor.
    * A method should only have one indented code block – don’t nest code blocks. Extract method first.

* Don’t try/catch in a method unless you can handle the exception or
add value. Let the exception bubble up to a point where you can
actually do something about the exception.

If you have any good points for method development, please feel free to leave a comment.

Helen 8:30 am on April 11, 2006

Umm… how about refactoring your post to get rid of all the typos and spelling mistakes?
Products / product’s, viewer / fewer, desireable / desirable, argument / arguments, and / an, blocks / blog.
It’s hard to take a post seriously when it appears so hastily written as this one.
jpalermo 12:39 pm on April 11, 2006

Thank you Helen. I _was_ in a rush writing this one. I’ve corrected the typeos.
rlewallen 5:32 pm on April 11, 2006

Something I stress often that people don’t get sometimes, is that methods should be atomic in nature, which you stated above, but is often one of the most common problems one comes across in code. TDD, and unit tests in general, help keep methods atomic based on the simple fact that a unit test should be atomic in nature. But even if you don’t have unit tests, make sure your method serves a single, testable function.

I tend to use “I want to see all code for a method without scrolling my screen”. Keeping my methods singular in purpose helps me to conform to this. One thing to keep in mind when refactoring and keeping methods atomic and short, is that there is a point when the call stack starts to get unhappy (the number is 8 calls, I recall K. Cwalina telling me once) and it starts to drag down the performance of the application, so there is really only so far you can refactor and maintain benefit when trying to keep methods small. There is certainly a line to be walked when deciding about refactorings and the size of your call stack, while at the same time maintaining readability and easily understandable purpose.

Nice, quick and simple to remember guildelines you have posted.
Jeffrey Palermo 6:04 pm on April 11, 2006

Raymond,
I hadn’t heard about a perf hit at 8 levels deep. I find that sometimes a simple call into the BCL will have a 8-deep call stack. Have you done any measurements on this?
johnwood 6:13 pm on April 11, 2006

How about describing *why* each of these points are important. Of course most of them are obvious, but to others perhaps not. (and I should point out that sometimes it just isn’t worth being a perfectionist, it’s a yagni thing.).
rlewallen 7:09 pm on April 11, 2006

Jeffrey, I haven’t done any performance testing, because its going to depend so much on size of objs, number of allocations, GC’s required and so much other stuff. Chances are, under most normal circumstances, I wouldn’t think the performance hit would be too noticable. I had this conversation during a C# chat a few years ago and was talking about refactorings and call stacks levels and the part of the answer was this number about how far deep the calls go before there starts to be *possible* problems starting to occur. Again, this was years ago and I know of no literature regarding that information, but what was said about “going beyond 8 levels deep in the call stack” has always stuck in my head.
David M. Kean 1:31 am on April 13, 2006

I’m sorry but I’m going to have to disagree with the following statement:

* You have a method that throws exceptions if you don’t set certain properties first.

This prevents the use of the Create-Set-Call pattern used extensively thoughout the .NET Framework, ie:

EventLog log = new EventLog();

log.Log = “Foo”;
log.MachineName = “Bar”;
log.WriteEntry(…);

See Framework Design Guidelines page 241.
jpalermo 2:17 am on April 13, 2006

David,
I understand the this is used in the .Net Framework, and that it was the most common usage in COM since you didn’t have constructors with COM, and you had no other choice for members needed by multiple methods in the class.

With object-oriented designs, I find it more desirable to push defining information into the constructor. For instance, the EventLog object serves little purpose without “Foo” and “Bar”, so I’d elect to pass them into the constructor.

With the Create-Set-Call method you talk about, it requires every developer to have intimate knowledge of that log.WriteEntry() method. Instead, for this specific instance, I’d either pass Log and MachineName into the method itself or the EventLog constructor. This make the method and class API more intention-revealing.
johnwood 2:18 am on April 13, 2006

I agree – not to mention how you code up your XAML objects in WPF. Sometimes having a constructor for every conceivable permutation of parameter combinations just isn’t practical. It’s a pity to have to rely on documentation, but without optional parameters create-set-call is all you have.
jpalermo 2:57 am on April 13, 2006

I used to have constructor overloads on some classes because one method needed two things, and another method needed some other thing. Then I realized that this class had two responsibilities, and I refactored it into two classes with a single responsibility each. Each class required well-defined dependencies and clearly revealed it’s capability.

With huge classes, I’m sure you would have to rely on documentation because they might to several different things based on the input. That’s why refactoring is important and makes code easier to read and use.

I, personally, don’t use create-set-call and haven’t heard the term used much.
johnwood 3:25 am on April 13, 2006

Perhaps in your case it was different, but you can really take the “realized this class had two responsibilities” thing too far IMO and end up drowning in classes, and hitting all the other drawbacks of an over-engineered application. You really have to be pragmatic when you’re developing, it’s a balancing act. If you do everything by the text book to the extreme then you’re seriously going to compromise the life expectancy of the code base. I know, I’ve experienced it repeatedly. IMO If you need to refactor that’s fine, but not just because you hit an instance you needed to overload a constructor.
johnwood 3:32 am on April 13, 2006

I should add that a nice compromise between the two is to use the builder pattern, where the consructor data is abstracted to a separate class where it is built independently, and passed into the constructor.
http://www.dofactory.com/Patterns/PatternBuilder.aspx
jpalermo 3:37 am on April 13, 2006

John,
I’m speaking from methods I’m currently employing, so I can only relate my personal experience.

My team’s classes are very small, and we have a lot of them. The system is not overengineered. It’s actually dirt simple and very easy to test. I don’t do anything by a text book, but I do everything based on results. Making classes smaller has led to loose coupling an a simpler system and code that is easier to read. If it led to a less desireable result, I would have ditched that method and used another.
johnwood 3:55 am on April 13, 2006

If it’s working for you then that’s great. I guess when you post something that, to some degree, is ‘laying down the law’ it comes off as a little textbox like. It’s just that I’ve met a lot of developers who love creating classes.. it’s like an addiction of sorts. The more classes they have the more opportunity they think they have for re-use when usually the opposite is the case because it’s all become so complex and intradependent.

Comments are closed.

Programming with Palermo

Jeffrey Palermo, Microsoft MVP, Author, Speaker, Clear Measure Chief Architect, Azure DevOps Expert

How to design a single method – level 200

Related