0

Assume I have a class containing 3 methods and an attribute. The class is supposed to encode the values in an array called result. So 3 functions are run in order to get the last encoded result that is stored in result.

class A{
    public: 
        int size;
        int* result;

        A(int si){
            size=si;
            result=new int[size];
            for(int i=0;i<size;i++)    
                result[i]=5;
        }

        void func_1(){
            for(int i=0;i<size;i++)
                result[i]=i+1;
        }
        void func_2(){
            for(int j=0;j<size;j++)
                result[j]=j+10;
        }
        void func_3(){
            for(int k=0;k<size;k++)
                result[k]=k+4;
        }
};

int main(){
    A a(10);
    a.func_1(); // consider each method as an encoding function (e.g. encryption, randomization, etc)
    a.func_2();
    a.func_3();
    return 0;
}

Here I stored intermediate results in data member array called result.

My question is whether storing intermediate results in an class's attribute and keep updating it is a bad idea? (i.e. violates the class definition)

If it isn't a good idea what would an alternative be?

Adrian
  • 51
  • 7
  • 2
    It's difficult to evaluate the correctness of this class design without knowing how this class will be used. Can you provide more context, perhaps some code showing example usage of an A object? – akira Apr 01 '15 at 15:18
  • 1
    Your code will not compile as is. Your for loops in your functions are incorrect. – NathanOliver Apr 01 '15 at 15:19
  • 1
    Also, please stop putting everything on one line. White space is cheap. – 3Dave Apr 01 '15 at 15:19
  • @DavidLively Code is modified, hope it'l help. – Adrian Apr 01 '15 at 15:34
  • Your code example is actually obscuring your question, rather than clarifying it. In your example, any object of class A will have an array with all elements set to 5 upon construction. Calling any one of the three methods will set the array to {1,2,3,...} regardless of its current internal state. So your example doesn't actually do anything, regardless of how anyone would use it. – akira Apr 01 '15 at 15:57
  • @akira It's been changed. However, the values in the array do not matter. What matters is that each function encodes it on its own way (all function may encode it in the same way as the other and this is a subclass of general description) – Adrian Apr 01 '15 at 16:14
  • Thanks. FYI, the code changes you made don't actually clear up the problem - the problem is that all three methods still completely reset the object state, regardless of what the current object state is. (Changing it so that the three methods set the array to three different arbitrary static sequences does not matter.) Thus it obscures your question of proper handling of object state across method calls. – akira Apr 01 '15 at 16:25
  • 1
    Furthermore, the data is not really accessible (or is it intentional that the members are public?) and thus never used. Even if it was accessible, it could easily be computed on demand. In summary, your code doesn't solve any problems, so it could be removed. Maybe you want to clarify your question a bit still, because as it stands, it is not really clear what you want, let alone what could be an alternative. – Ulrich Eckhardt Apr 01 '15 at 16:36

2 Answers2

0

My question is whether storing intermediate results in an class's attribute and keep updating it is a bad idea?

As a general guideline, an object should not be left in an invalid state after the completion of a function. What constitutes valid state depends on the object and the application.

Having said that, I have seen many instances where an object is in an invalid state after the completion of a call. One example I can give you comes from my domain knowledge. A finite element mesh has to be in a certain state to be a considered a valid mesh. However, whether you are constructing a mesh by reading the data from a file or building it in memory from geometry using some sort of meshing algorithm, there will be a lot of function calls before the mesh object is defined completely and is in a valid state. In the mean time, it will be in an invalid state for a while.

Follow the guideline but give yourself some slack.

R Sahu
  • 204,454
  • 14
  • 159
  • 270
  • Thanks for your answer and example. Do you know any textbook using this idea so I can use it as a reference? – Adrian Apr 01 '15 at 16:17
  • I don't personally find this answer useful. The concept of "valid" versus "invalid" is not intrinsically meaningful, as it is very easy to put the most well-designed class in an "invalid" state with the wrong usage. For example you can close an IO stream and then attempt to read from it. – akira Apr 01 '15 at 16:20
  • @Adrian, I don't have a reference to a textbook. My answer was based on experience. – R Sahu Apr 01 '15 at 16:22
  • @RSahu Why would it be a bad idea to let each function return an array of updated values ? – Adrian Apr 01 '15 at 18:14
  • @Adrian, it won't necessarily be bad. I'll try to come up with an example where it will be a bad idea for a function to leave the object in an "invalid" state. – R Sahu Apr 01 '15 at 18:20
0

I think I now understand your question. (Your code example is confusing rather than clarifying, which made it difficult for me.)

In the abstract, it is fine for a class to have internal state that is modified by externally called methods. If this were not desirable, then there would not be a point to having any internal state for a class at all.

The real question is about the specific methods that are provided for external use. This is a core question of good OOD, so you should research that. As a starting point, see What are some best object-oriented design practices?

Your example is quite general so it's hard to describe, but I'll do my best. If your three encoding methods can be used independently, then it's correct to have them as three separate callable methods. If calling one of them would then logically preclude the use of another (e.g., you probably shouldn't encryptUsingAES() and then encodeToMP3()), then you should document as such, and have your code throw the correct error/exception if the user attempts to do this. If a method call must always be immediately followed by another method call, then you should refactor the methods so the required operations are grouped together. In all these cases, your class should be thoroughly documented so any user can understand how to correctly use it.

Community
  • 1
  • 1
akira
  • 161
  • 5