Coding practices in C++, what is your pick and why?

Question

I have a big object say MyApplicationContext which keeps information about MyApplication such as name, path, loginInformation, description, details and others..

//MyApplicationCtx

class MyApplicationCtx{
       // ....
    private:
        std::string name;
        std::string path;
        std::string desciption;
        struct loginInformation loginInfo;
        int appVersion;
        std::string appPresident;
        //others
}

this is my method cloneApplication() which actually sets up a new application. there are two ways to do it as shown in Code 1 and Code 2. Which one should I prefer and why?

//Code 1

public void cloneApplication(MyApplicationCtx appObj){
    setAppName(appObj);
    setAppPath(appObj);
    setAppAddress(&appObj); // Note this address is passed
    setAppDescription(appObj);
    setAppLoginInformation(appObj);
    setAppVersion(appObj);
    setAppPresident(appObj);
}

public void setAppLoginInformation(MyApplicationCtx appObj){
    this->loginInfo = appObj.loginInfo; //assume it is correct
}

public void setAppAddress(MyApplicationCtx *appObj){
    this->address = appObj->address;
}

 .... // same way other setAppXXX(appObj) methods are called.

Q1. Does passing the big object appObj everytime has a performance impact?

Q2. If I pass it using reference, what should be the impact on performance?

public void setAppLoginInformation(MyApplicationCtx &appObj){ 
    this->loginInfo = appObj.loginInfo;
}

//Code 2

public void setUpApplication(MyApplicationCtx appObj){
    std::string appName;
    appName += appOj.getName();
    appName += "myname";
    setAppName(appName);

    std::string appPath;
    appPath += appObj.getPath();
    appPath += "myname";
    setAppPath(appPath);

    std::string appaddress;
    appaddress += appObj.getAppAddress();
    appaddress += "myname";
    setAppAddress(appaddress); 

    ... same way setup the string for description and pass it to function
    setAppDescription(appdescription);

    struct loginInformation loginInfo = appObj.getLoginInfo();
    setAppLoginInformation(loginInfo);

    ... similarly appVersion
    setAppVersion(appVersion);
    ... similarly appPresident
    setAppPresident(appPresident);
}

Q3. Compare code 1 and code 2, which one should I use? Personally i like Code 1

Answer 1

You're better off defining a Copy Constructor and an Assignment Operator:

// Note the use of passing by const reference!  This avoids the overhead of copying the object in the function call.
MyApplicationCtx(const MyApplicationCtx& other);
MyApplicationCtx& operator = (const MyApplicationCtx& other);

Better still, also define a private struct in your class that looks like:

struct AppInfo
{
    std::string name;
    std::string path;
    std::string desciption;
    struct loginInformation loginInfo;
    int appVersion;
    std::string appPresident;
};

In your App class' copy constructor and assignment operator you can take advantage of AppInfo's automatically generated assignment operator to do all of the assignment for you. This is assuming you only want a subset of MyApplicationCtx's members copied when you "clone".

This will also automatically be correct if you add or remove members of the AppInfo struct without having to go and change all of your boilerplate.

Answer 2

Q1..

Pass by object is heavy operation since it will create copy by invoking copy constructor.

Q2.

Pass reference to constant , it will improve perfomance.

Answer 3

Q1 - yes, every time you pass an object a copy is done

Q2 - minimal since an object passed by reference is basically just a pointer

Q3 - It is generally not a good idea to have large monolithic objects, instead you should split your objects into smaller objects, this allows for better re-usability and makes the code easier to read.

Answer 4

Luke beat me to tell you about copy constructors, to answer your other questions passing a large object by value has a performance impact when compared to passing by reference, make it a const if the function won't change it as in this case.

Answer 5

The best practice for cloning an object where all the members are copyable, like "Code 1" appears to be doing, is to use the default copy constructor - you don't have to write any code at all. Just copy like this:

MyApplicationCtx new_app = old_app;

"Code 2" is doing something different to "Code 1", so choosing one over the other is a matter of what you want the code to do, not a matter of style.

Q1. Passing a large object by value will cause it to be copied, which will have an impact on performance.

Q2. Yes, passing a reference to a large structure is more efficient than passing a copy. The only way to tell how large the impact is is to measure it with a profiler.

Answer 6

General:

• Why do you need to copy application object? Isn't it better to use singleton for this (with completely disabled copying by the way)?

Q1:

• Not only performance (yes, they will be copied) but memory too. As soon as I saw std::string implementations they at least occupy 2 memory chunks and first is in any case significantly less then minimal allocation size so such objects could cause memory efficiency problem if cloned extensively.

Q2:

• Passing reference is barely different (from performance point of view) from passing pointer so this should in general constant complexity. It is much better. Don't forget to add "const" modifier to block modifications.

Q3:

• I don't like actually both because of encapsulation broken. Once I saw good Java programmer article called something like "Why setters/getters are evil" (Well, I found it easily, there is not so much based on Java itself). This is VERY useful article to change style forever.

Answer 7

Short answer:

Q1: Given the size of your MyAppCtx class, yes, a significant performance hit will take place if the data is dealt with very frequently.

Q2: Minimal, you're passing a pointer.

Q3: Neither, for large objects like that you should use reference semantics and access the data through accessors. Don't worry about function call overhead, with optimizations turned on, the compiler can inline them if they meet various criteria (which I leave up to you to find out).

Long answer: Given functions:

void FuncByValue(MyAppCtx ctx);
void FuncByRef1(MyAppCtx& ctx);
void FuncByRef2(MyAppCtx* ctx);

When passing large objects like your MyApplicationCtx, it's a good idea to use reference semantics (FuncByRef1 & FuncByRef2), passing by reference is identical in performance to passing a pointer, the difference is only the syntax. If you pass the object by value, the object is copy-constructed into the function, such that the argument you pass into FuncByValue is different from the parameter FuncByValue receives. This is where you have to be careful of pointers (if any) contained in an object that was passed by value, because the pointer will have been copied as well, so it's very possible that more than one object will point to one element in memory at a given time, which could lead to memory leaks, corruption, etc.

In general, for objects like your MyAppCtx, I would recommend passing by reference and using accessors as appropriate.

Note, the reason I differentiated between argument and parameter above is that there is a difference between a function argument and a function parameter, it is as follows:

Given (template T is used simply to demonstrate that object type is irrelevent here):

template<typename T>
void MyFunc(T myTobject);

When calling MyFunc, you pass in an argument, eg:

int my_arg = 3;
MyFunc(my_arg);

And MyFunc receives a parameter, eg:

template<typename T>
void MyFunc(T myTobject)
{
  T cloned_param = T(myTobject);
}

In other words, my_arg is an argument, myTobject is a parameter.

Another note, in the above examples, there are essentially three versions of my_arg in memory: the original argument, the copy-constructed parameter myTobject, plus cloned_param which was explicitly copied as well.

Answer 8

There is one single point that has not been dealt in any other of the answers (that focused on your explicit questions more than in the general approach). I agree with @luke in that you should use what is idiomatic: copy constructor and assignment operators are there for a reason. But just for the sake of discusion on the first possibility you presented:

In the first block you propose small functions like:

public:
void setAppLoginInformation(MyApplicationCtx appObj){
    this->loginInfo = appObj.loginInfo; //assume it is correct
}

Now, besides the fact that the parameter should be passed by const reference, there are some other design issues there. You are offering a public operation that promises to change the login information, but the argument you require from your user is a full blown application context.

If you want to provide a method for just setting one of the attributes, I would change the method signature to match the attribute type:

public:
void setAppLoginInformation( loginInformation const & li ); // struct is optional here

This offers the possibility of changing the loginInformation both with a full application context or just with some specific login information object you can build yourself.

If on the other hand you want to disallow changing particular attributes of your class and you want to allow setting the values only from another application context object, then you should use the assignment operator, and if you want to do it in terms of small setter functions (assuming that the compiler provided assigment operator does not suffice), make them private.

With the proposed design you are offering users the possibility of setting each attribute to any value, but you are doing so in a cumbersome, hard to use way.