How to properly organize class code?

Question

What is the preferred way to organize the code in a class if you need to modify the data via a private method?

For example, is it better to do this:

private String data1;

callMethod(data1);

private void callMethod(String stuff) { 
    // Do stuff to data1
}

Or this:

private String data1;

callMethod();

private void callMethod() { 
    // Do stuff to data1
}

I've seen it done in a variety of ways and I'm trying to understand what is an industry standard best practice since I'm new to the developing world.

Answer 1

If the data is private to the object, the method has full access to it. You should not have to pass it in.

Objects encapsulate state and behavior into a single software module. The operations should manipulate the state.

Answer 2

I don't see the point of making private function with arguments, if you're aware that this argument is class private member. If function is specific and its result depends on members state then i would always go with second option.

Answer 3

It really depends on the data, the method and what you are trying to do. In other words this is a part of the design of the class.

How this private method modifies the data? If it performs a specific calculation that makes sense only for the data1 field then you could simply use callMethod().

If on the other hand your callMethod() is perhaps a small utility inside the class (maybe the same calculation can be performed on two different fields data1 and data2) then it makes sense to not have two separate methods but pass the member to be modified as an argument.

If you have a specific example then we could probably provide more help

Answer 4

It makes no sense to pass a reference internally if the method already knows the members it has to access.

class FooHolder {
    private Foo foo;

    private void ensureInitialized() {
        if (foo == null)
            foo = new Foo();
    }

    public Foo getFoo() {
        ensureInitialized();
        return foo;
    }
}

But it's sometimes useful to do that if you can prevent code duplication that way. Those internal utility methods could sometimes be static like here:

class FooAndBar {
    private List<Foo> foos;
    private List<Bar> bars;

    public void addFoo(Foo foo) {
        foos = ensureList(foos);
        foos.add(foo);
    }

    public void addBar(Bar bar) {
        bars = ensureList(bars);
        bars.add(bar);
    }

    // assume this method is not generically useful and therefore not better off in a utility class
    private static <T> List<T> ensureList(List<T> list) {
        return list != null ? list : new ArrayList<T>();
    }
}

Sometimes they can't / should not be

class FooFoos {
    private final Map<String, List<Foo>> fooMap = new HashMap<String, List<Foo>>();
    private List<Foo> getListForKey(String key) {
        List<Foo> list = fooMap.get(key);
        if (list == null) {
            list = new ArrayList<Foo>();
            fooMap.put(key, list);
        }
        return list;
    }

    public void addFoo(String key, Foo foo) {
        getListForKey(key).add(foo);
    }
    public List<Foo> getList(String key) {
        return getListForKey(key);
    }
}

Note that getListForKey is not passed a reference to fooMap. Not necessery because that part is already clear and typing it out in every method would just clutter the code.

If you can achieve less code duplication and some internal encapsulation that way, pass references to your private methods. But don't do it if that leads to more code because every method has to specify the reference again.

Also note that a lot of internal encapsulation of functionality through methods means that you should consider to refactor that functionality into another class. For the last example consider using / creating something like a MultiMap