11

A suggestion from Bob Martin's "Clean code" has me scratching my head.. "If once function calls another, they should be vertically close and the caller should be above the callee"

So far, I've been more or less sticking close to the .Net guidelines, which group class members by type (properties, ctors, functions) and visibility (public / prot. / private). The tip seems like trouble at first.. but it "just might work". I personally have encountered cases where I liked this layout - easier to drill down when you're in the right call chain.

The idea behind the tip seems sound but other scenarios like "let me look at the public interface of this class" might get worse. Maybe Uncle Bob is banking on the small classes and IDE support for viewing types...

Has anyone tried this out for an extended period ?

Update: Seems like a code-snippet is in order

class SomeType()
{
  /// fields, ctors, et. all
  public void Method1()   { // calls HelperMethod1 and HelperMethod2 }
  private void HelperMethod1 { // calls HelperMethod3 }
  private void HelperMethod3 {}
  private void HelperMethod2 {}

  public void Method2 () { // and so on... }

}
5
  • 2
    The ghastly "Uncle Bob" is not exactly the sharpest pencil in the box. Jun 8, 2011 at 13:17
  • 1
    The idea is just "give me the big picture before the nitty-gritty details". Adapt as necessary. Jun 8, 2011 at 14:18
  • 2
    The Eagles must be getting close to getting back together again, because I find myself agreeing with Neil's comment. I grew up with PASCAL and "put the little stuff first" because the PASCAL compilers all required things to be defined before they were referenced, and FORWARD declarations were generally frowned upon. Jun 8, 2011 at 16:09
  • @Neil - I'm trying to judge the merit of the advice.. irrespective of the source. @ John - and the tip is the opposite of forward declarations.. you put the caller first .. the 'callee's are declared just below the callers.
    – Gishu
    Jun 9, 2011 at 9:01
  • @ryanc - the prelude to that paragraph emphasizes that "closely-related/cohesive" concepts should be vertically close together [Prevents scrolling around when you're trying to figure something out]. The called functions are laid out beneath the caller in order of calls. See added code-snippet
    – Gishu
    Jun 9, 2011 at 9:06

3 Answers 3

3

I might be going out on a limb here, but I wonder if the tool you use has an impact on this. I'm referring to the text editor versus IDE decision that developers must make.

In an IDE, you have a lot more functionality to view source files. Typically, you can get a list of the methods sorted alphabetically, by visibility, or even return type in a sidebar. You can also jump to a method if you have a use for it. You can also generate call trees for methods and drill down. You also typically have a powerful find command that might support regular expressions. In this situation, the order of methods that you create really doesn't matter as you have views other than the source code available.

In a text editor, you typically don't have these features - the closest that you'll have is probably a strong find/replace. Here, you're going to want to pay more attention to the structure of your file since it might be harder to navigate. You want to minimize the time spent scrolling around the file to find what you are looking for, and a consistent and logical order of methods can help.

1
  • +1 for the IDE; the better the IDE, the less one has to worry about such things
    – user281377
    Jun 9, 2011 at 11:45
2

The point is that called things are less interesting than calling things. The more a method calls other methods, the more likely it is that that method is part of the external API of the object (as opposed to being an implementation detail). That means the external API of the class - public methods, if your language supports that concept - will naturally "want" to be at the top of the file, making it easier to find those methods. Conversely, helper functions and such will "want" to be at the bottom of the file.

(I'm explaining the concept, not evaluating its effectiveness.)

2
  • Yes but that would imply all the public functions should float to the top of the file as one group viz. conventional approach. The proposed approach is different (or at least how I read it).. see update in question
    – Gishu
    Jun 9, 2011 at 8:57
  • Yes indeed, your public functions should float to the top. Of course some languages don't have visibility modifiers at all... Jun 9, 2011 at 15:22
1

If by extended period you mean for more than a couple of days? Then No.
A couple of years ago I started doing this on some new code, and slowly drove myself insane, until I stopped.

My personal preference for laying out classes is

class MyClass
{
    // static fields
    // fields
    // constructors
    // properties
    // methods
} 

But that's not religious, properties and methods can get mixed together. Visibility doesn't come into it (I don't group by public/protected/private)

We've a guy here in office keeps a strict structure on everything in a class file, with everything grouped together in main groups and sub groups, all nicely nested in regions . . . I have to admit I think regions are the work of Satan, they drive me around the feckin twist.

Everytime I open one of his classes I die a little inside :(

4
  • I'm not advocating large classes with regions added to mask the smell. Not trying to get religious.. but having a consistent layout within a project speeds up things -- knowing where to look. Grouping bu visibility as the added benefit of having the public API close together so that you can find your specific entry point and drill down from there...
    – Gishu
    Jun 9, 2011 at 9:09
  • And constructors? Those go under "methods"? Jun 9, 2011 at 9:13
  • @Cody Gray: Apologies, forgot ctors! Jun 9, 2011 at 10:44
  • @Gishu: I find that modern visualisation and navigation tools have removed the need for strict file layouts. Does it matter where a method is implemented when I can right click the usage and "Go to Definition"? Jun 9, 2011 at 15:32

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.