2

I'd like to ask you about one simple thing. I have class A that do something (for example counts something ). There is also one class B that handle some parameters to this class (class A is member of class B). Class B calls one method of class A that does something and writes value to database.

Class A uses parameters that got from Class B. Class A has few methods to made clean code (it's good I think). But... Is it ok to save parameters as private members?? It's not necessary. I don't have to remember them after writing to Database. But if I don't have private data I have to handle it to every private method as parameters (during processing). I think that it's ok to save this parameters as members of this class. Am I right? Or maybe I should avoid this when it's not necessary (when I don't have to remember them)?

When I don't have private members my publicMethod (called from class B) is looked something like that:

void publicMethod(int param1, int param2, int param3, int param4)
{
    privateMethod1(param1, param2, param3, param4);
    privateMethod2(param1, param2, param3, param4);
    privateMethod3(param1, param2, param3, param4);
}

and in private methods I call other private method and I have to handle parameters.... I think it doesn't look good... Am I right? When I have private members it looks like:

void method(int param1, int param2, int param3, int param4)
{
    privateMethod1();
    privateMethod2();
    privateMethod3();
}

It's better I guess. But I'm not professional and I'm not sure...

(I write in C++)

  • What does "sth" mean? – Michael O'Neill Jan 27 '14 at 21:19
  • It's lazy txtspk for "something." I've fixed it. – Robert Harvey Jan 27 '14 at 21:24
  • 2
    Depending on the problem's domain, it could be meaningful to encapsulate (some of) those parameters in a single structure/class of its own, which would lead to clearer code without the need of storing it as a private member of A. – rucamzu Jan 27 '14 at 22:34
  • 1
    Anyway, I see your problem symptomatic of a lack of separation of concerns, as class A is directly responsible of both making arbitrary computations and saving their result on a database. I think that the Command pattern would be useful in this case. – rucamzu Jan 27 '14 at 22:42
  • @rucamzu: It makes sense to attempt to separate the concerns, but they may not always be separable. If the function operates on the data or if it writes a lot of data, there is no other reasonable way to represent the results than in the database. – Jan Hudec Jan 28 '14 at 8:01
4

If you'd have an object that you would construct, call one method on it and destroy it again, than you don't need an object and therefore shouldn't have it.

Of course for complex logic it makes sense to store the state in an object. But it can, and therefore should, be an implementation detail of the public function. So I'd do:

  • header:

    void publicFunction(int param1, int param2, int param3, int param4);
    

    (most functions should return something, but if it writes the result to db, returning void on success and throwing on error is fine)

  • source:

    namespace {
        class privateImplementation {
            friend void publicFunction(int, int, int, int);
            int param1;
            int param2;
            int param3;
            int param4;
            int state;
            // ...
            void privateMethod1();
            void privateMethod2();
            // ...
        };
    };
    
    void publicFunction(int param1, int param2, int param3, int param4)
    {
         privateImplementation impl{param1, param2, param3, param4, 0};
         impl.privateMethod1();
         impl.privateMethod2();
         // ...
    }
    

This way the logic is internally structured, state encapsulated etc, but additionally the whole operation is encapsulated and hidden away.

The same technique applies also to functions that have good reasons to be member functions. If they have state that only exists until their end, it should be stored on the stack in an auxiliary internal object, not in the invocant.

On a side note, if it writes to database, it probably should be getting the database connection as one of the arguments.

  • I wouldn't call this a "state" object - that would suggest that it stores the state for some other object, which is not true. privateState's stores the state for itself - like any other object. Also, a state object should only have methods for accessing the state, not using it. This seems more like the command pattern, and should be named appropriately. – Idan Arye Jan 28 '14 at 8:23
  • @IdanArye: Sorry, I don't see any relation to command pattern whatsoever. Command pattern is for preparing an action to be executed at later time, but nothing is being deferred here. You are right about the state though. I have renamed it, though I am not sure about good generic name; it should be named simply according to the operation it is implementing. – Jan Hudec Jan 28 '14 at 8:43
  • In the command pattern you store all the data needed for running a method inside an object so you can run it later on. Here you store all the data needed for running a method inside an object and run it right away. IMO the difference between "later on" and "right away" is neglectable here. – Idan Arye Jan 28 '14 at 9:43
  • @IdanArye: The later on and right away is not important. The important difference is that a Command object is created by one component to be executed by another component and as such it's interface is public, while here the object is just a collection of parameters that need to be passed through several steps of a complex calculation inside one component and as such implementation detail of the component. – Jan Hudec Jan 28 '14 at 17:39
1

You can actually group parameters into a separate object and manage them as a single entity. This way a lot of questions are much easier to answer.

Like, what are these parameters? Is it some configuration? Then I should just configure the object once. Or is it some parameters of a query? Then I should make query object explicit, configure it with parameters, and let my new Query process all the Query-related stuff itself. Et cetera.

Because if your parameters always go together, most probably there is some abstraction/entity that you are missing, and which is represented by the grouping of this parameters. And its behaviour is spread thin between objects using this grouping of parameters.

0

It's a little unclear exactly what the question is, but if you hold references to variables you don't need in the scope of the application, you will certainly encounter a memory leak and eventually run out of memory.

Consider a stateless model such as Ruby on Rails where the session is persisted in the db. Here any 'state' is readily discarded/garbage collected (depending on the platform for the stateless model) and then any subsequent request for the user session is retrieved again from the DB. While this has some overhead (performance) costs, it does offer more scalable solutions because the web applications don't need to share memory across a cluster.

I hope that somewhat answers the question.

  • I write in C++. It's not important (in this question) DB. It's an example. I'd like to simplify (maybe I did it wrong). I'll edit my question to make it cleared. Thanks for reply. – pawell55555 Jan 27 '14 at 21:15

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.