0

I get to review code that often looks like this:

class MyClass1 {

    constructor MyClass(initialiseParam1: Object) {
        // do something with initialiseParam1
    }

    initialiseStep2() {
        // do some more initialisation
    }
}

To use MyClass1 you need to do two steps, first to create an object and second to call initialiseStep2 method.

Another variation of the same idea looks like this:

class MyClass2 {        
    doSomethingPart1() {
        // part of some action
    }

    doSomethingPart2() {
        // complete the same action
    }
}

And to make a single use, you need to call two separate methods.

What I don't like a lot about both of them is that a user of this class needs some inside knowledge how to use it and one of the great design goals I fight for is to make a public interface in way that it is impossible to misuse it. i.e. you should get a compile error if you do something stupid.

That's why I try to design in a way that whatever you want to do is a single action only. No follow up steps, no follow up configuration of an object that is already constructed.

When explaining this to my peers I would like to provide them some design principle with a name and good examples and explanations on the internet, but I failed so far.

Is anyone aware of such a principle?

2
  • 1
    Regarding MyClass1 do a search for "two step initialization". Many hits are for C++, but they are equally valid for other languages. Oct 16, 2018 at 14:30
  • Stateless code and pure functions come to mind Oct 16, 2018 at 15:25

2 Answers 2

4

The trouble with objects obeying such an API is that they have hidden state.

A MyClass2 object can already have received a doSomethingPart1() all, or not. There is no way to tell just from holding the object and looking at its type. Similarly, a MyClass1 object can already have been initialized via initializeStep2(), or not. Again, there is no clear way of telling the difference.

This is generally a bad thing. Asking an object to do one service should require one interaction with it, not several interactions that have to be in the correct order. The very fact that it is possible to perform operations out of order introduces the possibility of usage errors where there shouldn't be one.

As a rule, you should only split operations in two if one step requires enormous amounts of resources that have to amortized across several subsequent operations of the same kind. For instance, it is a huge effort to sort a list, but after that each look-up in the list becomes much easier.

Even then, although it is a good idea to encapsulate such a workflow into objects, it shouldn't come in the form of two methods sort() and lookup() with the caveat "You can't lookup until you've sorted!". A better solution would be to have sorting happen in the constructor and include only lookup() in the API.

4
  • I agree with everything else you said, but maybe the problem is not that the object is stateful, but that the API is? Objects are most useful when they encapsulate state. If an object is not stateful, it's just a collection of functions. That can be useful, but you would be better off without the concept of an object. Oct 16, 2018 at 12:18
  • 2
    @JacobRaihle You're right, the real problem is that the object has state that you must know for proper use but cannot know. Oct 16, 2018 at 12:20
  • Thanks a lot, in my explanations I use similar arguments, but that's helpful. My question though is whether there is some sort of standard known principle that describes this? For example, I looked through the SOLID principles and couldn't find one that would describe this problem
    – eddyP23
    Oct 16, 2018 at 12:28
  • Well put, and nice edit Oct 16, 2018 at 15:12
1

You are right that this current design is bad and your goal of having one call to complete on action is the proper convention to follow. There are a lot of different ways to show how this is bad as it violates overlapping ideas of many different principles. Some or all of these may be useful examples to explain why this is bad.

  • There's the single responsibility principle, because your constructor is doing less than one thing when it should be doing exactly one thing.

  • Similarly there is repetition that should be avoided if you must always declare new class() and class.initialize() then they really should be handled by a single method.

  • There is also the principle of least astonishment/surprise, once I have a new object the expectation is I should be able to call any public method, but these classes surprise me by breaking if initialze2() isn't called first.

  • Your objects are failing to manage their own state from inside the object, they aren't encapsulating their state.

  • The spirit of tell don't ask is also being violated, you don't have any methods/properties that would be asking about state, but you have a real need for them in the current design. The proper way to follow this principle is to have objects do what you tell them when you tell them, not remove any way to discover internal state but require knowledge of it to use objects properly.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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