When to create helper methods and separate files

Question

Background: I have a large (several hundred lines) class that manages a concept based on some primitive-type data structures

long[] slist;  //list of unique patterns (related to polyominoes)
int[][][] sref;//patterns at each place [location][depth][<list of indices in slist>]

Question: The two methods that populate and update these data are going to be quite long, with handfuls of 5-20 line tasks, some shared, others unique. I probably want to make a helper method for each sub-task.

update(...){
    //do A
    //do B
    //do C
    //...
}
build(){
    //do D
    //do B
    //do E
    //...
}

The problem is if there are too many unrelated helper methods in one file, readability does not improve.

The answer to this question gets me most of the way there. I can declare the structures in the same package, in their own class, and access the primitive member field or call related methods. But I still want to know the accepted wisdom here because this organization did not come easily to mind.

Would you ever go so far as to put update() and build() functions in their own files? If so, where should their common tasks be declared?

Answer 1

I highly recommend reading Refactoring (Amazon link) by Martin Fowler; it should be in every programmer's library and will help you with situations like this. I will refer to it in the post.

If a class has too much code, then it's usually time to split up the class. That may entail creating member variables that have classes (delegating functionality), or it may mean creating an object on the fly (replace method with method object). Things that share commonalities are good cases for applying inheritance or the state/strategy pattern.

Short answer

Yes, you would go so far as to have those functions in their own files. However, I would instead make them classes. Perhaps Updater and Builder objects. You can inherit from BuilderCommon and UpdaterCommon classes. These new objects will be coupled to the old object, but that's okay. You may consider putting these new sets of classes in their own package. Hierarchical organization will help with readability and reuse of common code. Try to take advantage of concepts like inheritance and abstraction techniques like generics to do the work for you. If you can find commonality between doA, doB, etc., make UpdateHelper classes out of them and put them in a list. Then simply iterate over the list.

This is just one of the many ways to do it:

public class Updater
{
    public Updater(List<IUpdateHelper> helpers)
    {
        helpers = new ArrayList<UpdateHelper>();
        this.helpers.add(helpers);
    }
    public void update()
    {
        for (IUpdateHelper helper : helpers)
        {
            helper.performHelp();
        }
    }

    protected List<IUpdateHelper> helpers;
}

public class UpdaterCommon extends Updater
{
    public UpdaterCommon()
    {
        helpers.add(new UpdateHelperA());
        ... // Etc.
    }
}

/*
 * This uses inheritance for common helpers, but it could just as well use
 * delegation. Also, this assumes that order of invocation for each helper 
 * doesn't matter.
 */
public class UpdaterOne extends UpdaterCommon {...}

interface IUpdateHelper
{
    public void performHelp();
}
public class UpdateHelperA implements IUpdateHelper {...}

Definitely replace those arrays with objects and add an interface.

Closing

In my experience it usually only takes the application of a few of these concepts on a regular basis to make a significant difference in code quality. If a package, class, method, conditional, etc. gets to be unruly, break it out into a smaller unit. Keep pushing the nitty-gritty functionality down into very small methods so that you can look at the code from a high level.

Answer 2

There are many different ways to write the same piece of code. I like to write my code to a way where I could explain it in real terms.

For example, lets say I was creating a person. If I were to put all the body parts in one class, it would be a bit confusing to read.

I COULD put the Head, Limbs and Torso into seperate classes, then put them all into my Human class, but even then, each of those body parts are pretty complex. You might wanna break it down a bit more.

I could make a class for the Eyes, Nose, Mouth and Ears, then reference them in your Head class.

Fingers, Finger Joints, Fingernails... all of that can go into the Hand class.

It's all about your mind-set. Once you have all your classes built, you can reference them however fits your preference.

To continue this example, for me at least, I would reference the Hand class in the Arm class, since each arm contains a hand (hopefully...)

If I were to call for the hand, it would look something like this:

Arm leftArm = new Arm();
Arm rightArm = new Arm();

leftArm.hand.makeFist();
rightArm.hand.raiseMiddleFinger();

although it would be quite tedious to write it out like that (if you wanna refer to the hand, you would have to go through the arm to get to it), which is why I like to use static values and return methods. It's all about how you see programming. I like to compare programming to the real world.

As for helper methods, I like to see them as actions. If there is something you wanna do, like 'turnOnTv' or 'tossBall', then you would wanna put that method in the respected class.

For example, lets say you wanted someone to toss a ball. You would want the 'public void tossBall()' method in the class you have your user information, so when you call it, it can look something a bit like

Person personNumberOne = new Person();

personNumberOne.tossBall()

This is just my personal opinion. Not saying this is the RIGHT way to do it, but honestly, there really is no RIGHT way, seeing how things can be done many ways. It's always good to look for efficiency wherever you can, but you dont wanna work with code you dont understand either

Answer 3

A Design Pattern that may help here is the Abstract Factory Pattern, where you create an abstract class/interface, define in it your helper methods (as abstract), and use this interface in your original update() and build() methods

and create a subclass to your abstract Factory (class/interface) and in this subclass do all your jobs

you can pass parameters to your abstract methods that will be used while implementing them in the subclass you will create, so to keep things connected to each other, but loosely coupled as well

for example :

class Client {
    private AbstractFactory factory = null;

    public Client(AbstractFactory factory){
        this.factory = factory;
    }

    void update(){
        String a = factory.getA();
        MyObject b = factory.getB(a);
        b.doSomeStuff();
        int c = factory.getC(b);
    }

    void build(){
        AnotherObject d = factory.getD();
        d.doMoreStuff();
    }
}

public interface AbstractFactory{
    String getA();
    MyObject getB(String a);
    int getC(MyObject b);
    AnotherObject getD();
}


public class Helper implements AbstractFactory{
    // implement your methods here
}

public class MyObject{ /*some helper methods here as well */}
public class AnotherObject{ /*another helper methods here as well */}

this will provide loose coupling, and easier separation between the code so that when you want to modify any thing, you wont go to your Client class, instead, will go to the implementing class of this part, leaving your original Client class untouched

and as OO Principles require ... the least dependencied in your code, the more flexibility it will gain, when ever you find the word "new" in your code, the less flexible it will be, notice that in the Client class for example, there were nearly no "new" keyword, which means it is very flexible, and easy to maintain

Answer 4

Another solution to this problem is the Template Method Pattern.