0

I recently started working in domain of mesh generation . My programs usually contains large chunks of procedural code consisting of several phases. Eg.

class MeshAlgo1
{
    /* A very long function */
    void DoSomething(MeshData data)
    {
       // Phase 1
       ...
       // Phase 2
       ...
       // Phase N

    }
};

I would like to break down these long member functions into smaller functions for better readability and maintainability.

class MeshAlgo1
{
    /* Shorter function! :-) */
    void DoSomething(MeshData data)
    {
       DoSomething1();
       //...
       DoSomething2();
       //...
       DoSomethingN();
    }
};

But since this code is procedural, DoSomething1(), DoSomething2(),etc would not be independent of each other, and so do not fit into OOP style. That is, DoSomething2() assumes that DoSomething1() has been called before it. I would like to know what is the correct way of writing such code in Object Oriented style. Thanks a lot.

  • A method having some set preconditions (like "this method was not called before" or "this variable is initialized") is perfectly fine. The fact that those preconditions are perfectly satisfied by calling a certain sequence of other methods in the class is also pretty normal. As long as those preconditions can be well formulated by something other than "Method 1 and Method 2 was called", you're good. Also, private methods can be pretty much anything. It's public methods that you should worry about. – Ordous Jul 22 '15 at 12:25
  • 2
    1 class, lots of methods. OO is just a different way of organising your procedural code to make it more manageable. – gbjbaanb Jul 22 '15 at 12:27
  • @Ordous, odd that you describe exactly my answer in your comment, yet downvoted that answer. Looks like I wandered into the wrong question as I couldn't disagree more with A method having some set preconditions (like "this method was not called before" or "this variable is initialized") is perfectly fine. The fact that those preconditions are perfectly satisfied by calling a certain sequence of other methods in the class is also pretty normal. Code smells all the way in that quote. – David Arno Jul 22 '15 at 12:33
  • 1
    @DavidArno Your answer was described exactly in the question. Are you surprised it's also referenced in the comments? – Ordous Jul 22 '15 at 12:37
1

Though I agree to @Philipp that a more explicit passing of parameters can solve the problem, procedural code tends to work on some "global data" (or some member variables), and changing this might have a too big impact on the exsiting code base to be done easily. For this case, I can think of a different approach to solve this:

DoSomething2() assumes that DoSomething1() has been called before it.

But why? Maybe DoSomething1 has to initialize a member variable which is expected to be initialised by DoSomething2:

class MeshAlgo1
{
    MyMesh mesh;    

    private void DoSomething1()
    {
        mesh=new MyMesh();
        // initialize `mesh`
    }

    private void DoSomething2()
    {
        // ... prepare Phase 2 here ...
        // ...
        mesh.DoSomethingSpecial();
        // ...
    }
}

So if you are afraid of someone changing unintentionally the order, you can add a run time check to your method to make it fail more fast and signale more clearly what went wrong:

    private void DoSomething2()
    {
        if(mesh==null)
           throw new Exception("mesh has to be initialized before calling DoSomething2");
        // ... prepare Phase 2 here ...
        // ...
        mesh.DoSomethingSpecial();
        // ...
    }

Additionally, proper naming of your methods will help, and adding a comment about the necessary preconditions will also lower the risk of getting the order wrong.

What you do not get this way is a compile time check for the order, of course, but in lots of real-world scenarios a run time check is all you need.

  • I was going to suggest to throw exceptions when the data is in an invalid state for the requested operation, but the problem is that this only checks for the correct order at runtime. It would be preferable to find a way to enforce the correct order at compile-time. – Philipp Jul 22 '15 at 14:15
  • @Philipp: that is correct, and that is why I voted your answer up. But it does not make my answer wrong or invalid, in reality, often a runtime check is all you really need. – Doc Brown Jul 22 '15 at 14:17
  • I never said your answer was wrong. I upvoted it. – Philipp Jul 22 '15 at 14:18
1

Others mention methods for classes which is the Object Oriented way. Sounds like your task is to safely order the procedures. Well, a procedure is (abstractly) a behavior of an object. Are you locked into a single class to manage these procedures as methods, or can you create two classes? Say, DoerOne withDoerOne::DoSomethingOne(), and DoerTwo withDoerTwo::DoSomethingTwo(). Now, with two objects of different class types, the problem of sequencing your procedures becomes sequencing the lifecycle of those two objects (e.g. when each is instantiated). How would you do that?

0

When you are afraid that someone might call the methods in the wrong order, it seems to indicate that each of the private method transforms the data from one state into a different state. When that is the case you could represent each of these states with a different type. Each method would then accept an object of the previous state as input and return an object which represents the data in a different state as output. This would look like this:

private MeshDataState1 DoSomething1(MeshDataState0 data) {
    //...
}

private MeshDataState2 DoSomething2(MeshDataState1 data) {
    //...
}

private MeshDataStateFinal DoSomething3(MeshDataState2 data) {
    //...
}


public DoEverything(MeshDataState0 data) {

    MeshDataState1 dataState1 = DoSomething1(data);

    MeshDataState2 dataState2 = DoSomething2(dataState1);

    MeshDataStateFinal dataStateFinal = DoSomething3(dataState2);
}

However, a more faithful implementation of the object-oriented paradigm would be when each state would have the method to transform itself to the next state:

public DoEverything(MeshDataState0 data) {

    MeshDataState1 dataState1 = data.toState1();

    MeshDataState2 dataState2 = dataState1.toState2();

    MeshDataStateFinal dataStateFinal = dataState2.toStateFinal();
}

or shorter and without the intermediate variables:

public DoEverything(MeshDataState0 data) {

    MeshDataStateFinal dataStateFinal = data.toState1().toState2().toStateFinal();
}
  • 1
    Note that MeshDataStateN can be an interface, and even an empty interface extending MeshData – Caleth Jul 22 '15 at 12:52
  • 4
    Before, the OP had a mass of tightly couple code in one class. Now, he has a mass of tightly coupled code spread over a bunch of tightly coupled classes. The classic 00 approach to "improving things"... – David Arno Jul 22 '15 at 13:05
  • 1
    I already made a suggestion: don't adopt an OO approach in this day and age; use a functional approach instead. It got rapidly downvoted, so I deleted it... – David Arno Jul 22 '15 at 14:21
  • 2
    @DavidArno: Philips first part of the answer is a more functional style solution, not a more OO style solution. – Doc Brown Jul 22 '15 at 14:28
  • 1
    I don't see why this is downvoted. It is the answer that takes the most advantage of the OO paradigm. It also exposes any mis-orderings at compile time with a missing method error. Sure there are lots of objects, but this is object-oriented design. +1 – cbojar Jul 22 '15 at 15:01
0

In my opinion, these methods themselves (as separate entities) mustn't be aware of any change of state. The concern with state should be in the form of a conditional invocation.

I may be wrong, I may be under-thinking it, but I'd simply structure it like so:

class MeshAlgo1
{
    void DoSomething(MeshData data)
    {
       DoSomething1();

       if(data.ReadyForSomething2)
           DoSomething2();

       if(data.ReadyForSomething3)
           DoSomething3();
    }
};

Where .ReadyForSomething2 is the change in state that occurred as the result of a 'successful' call to DoSomething1().

  • But how do you handle the case where the data is not in the correct state? – Philipp Jul 22 '15 at 14:07
  • Define 'correct'. – JᴀʏMᴇᴇ Jul 22 '15 at 14:08
  • if(data.ReadyForSomething2) being false. Then DoSomething2(); never gets called. – Philipp Jul 22 '15 at 14:09
  • Was there a requirement to always call all methods? I'm not asking rhetorically or being funny by the way. Have I misunderstood the OP? – JᴀʏMᴇᴇ Jul 22 '15 at 14:09
  • 2
    The way I understood the question, the requirement was to ensure that all the methods always get called in the correct order. Hiding the mistake would likely be counter-productive. – Philipp Jul 22 '15 at 14:10

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.