3

This question already has an answer here:

Here's a question that's always bugged me. I'm going to use java as an example because I've almost never run into a problem in java where I didn't need to use helper methods in its class structure.

Let's say I have a method that does a thing or two and then calls a helper method:

public void doStuff() {
    int x = 4;
    int y = 5;
    doStuffHelper(x, y);
}

Now I need to create a private method, doStuffHelper, that doStuff calls with x and y. For readability I always put the helper directly above or below the function, but I can never quite decide which one is better. What are your thoughts/experiences with it?

I know the question seems trivial, but since I don't have any guiding principles on it, I find that it always kills coding time deciding what to do and re-organizing my code if it doesn't work out. If there is no consensus, I think it's time to finally commit to one over the other for me.

marked as duplicate by user40980, gnat, Bart van Ingen Schenau, Kilian Foth, ratchet freak Sep 16 '14 at 9:03

This question has been asked before and already has an answer. If those answers do not fully address your question, please ask a new question.

  • 2
    According to Clean Code you should order your methods from top to bottom, in the order they are called. So it reada like a poem. I try to follow this advice, and it works quite well for me. – Bruno Schäpper May 4 '13 at 4:54
  • @VainFellowman I was going to write the same as an answer, but since you got here first, I'll let you do so and get the rep :) – Michael Brown May 4 '13 at 5:51
5

I always put the function doStuffHelper afterwards. This makes it easier to read, as when you start reading the code from the top, you won't have to first remember what doStuffHelper does (without understanding its context) and then while rememering that, read function doStuff(). Instead you read function doStuff() which should be the "main" function of the class, understand what it's trying to do, and then read function doStuffHelper().

Sort of like the difference between a passive and active sentence in English.

4

Ordering functions is a valid and important concern, and some schemes work better than others. Like comments and @Lachlan suggested, putting the fundamental aspects of the class earlier often makes the most sense.

I often prefer to see static things precede instance things, and public before private. I prefer to see data and creational things before I see task things. Specifically, I prefer to see constructors, initialization, getters and setters, and then task methods. I try to often put helper functions immediately following the first function that uses it, instead of dropping them to the bottom. Why this basic order?

  • Constructors: before anything can be done with an object, it has to be created. Like with getters and setters, I want to know right away if there is something different about how the object handles its construction, and what if any other side-effects might occur from the various methods of construction (do they all explicitly initialize, or only in certain cases?).

  • Initialization: what setup is done for me, and what do I likely need to provide to actually do something useful with this object

  • Getters and setters: I want to know early on if there is anything peculiar about how values are read or written. Are they using any kind of multi-threading lock to restrict access that could skew how I would normally anticipate the getter or setter working? Are they accessing the backing store in a non-standard way, like using an indexed array or collection of values instead of a discreet variable? Or are they writing changes to a database as the property is changed [, from within the property]? And what other side-effects might occur from changing or accessing underlying state?

  • Task functions: these are the functions that define the primary responsibility of the class or module. As I mentioned, I like to put the helper methods that are explicitly part of the class, nearest to where they are first, or likely to first be used.

As you arrange those 'extra' functions with the primary task functions, you should always keep the following question in mind: How does this helper function relate to the one responsibility and one reason to change for this class?

Extra [private] functions are inevitable in the course of refactoring functions down to their primary purpose, but beware that some of those extra functions may be violating the Single Responsibility Principle (SRP) and "one reason to change" for the class.

The case may be that helper functions deserve a class of their own, perhaps several. One way to approach evaluating whether a function stays or moves on to its own home is by looking at cohesion. Are the parameters the function accepts, and the internal values it manipulates, variables that are fundamental to the class? Are they variables that are commonly used throughout the rest of the class, or are they mostly only used by a very small number of secondary functions? If the function doesn't use, or only uses a few of those class variables or internal state, then it may indicate low cohesion and a strong candidacy for promotion to a separate class, and a slightly different method of invoking that helper method.

A fairly common example of where this occurs is with objects that have formatting helpers, or input and output helpers. Helpers that do a conversion of the objects representation to or from the representation maintained by the model are usually good candidates for their own formatting class. Normally, an object shouldn't have the responsibility of loading or saving it's own state, as they bleeds the implementation details into the model. Such activity often is indicated by words or pairings such as:

  • from or to
  • convert or configure or transform
  • read or write
  • load or save
  • play or record
  • launch or suspend (and possibly reset depending on the class/context)

Many languages and frameworks often have some built in methods for converting the internal representation of an object's state to another base form. Often it takes the form of 'to string' or some slight variation of this. Usually this is provided for two reasons: to simplify certain aspects and expectations of the platform and for convenience. It is a license to moderately over-ride and extend the method so that a base representation of a complex object can be communicated to the rest of the framework, but it shouldn't be treated as a license to give a class more responsibility than it needs.

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