CODEWITHSUNDEEP
c++
Cameron Atkinson
Cameron Atkinson

Reputation: 101

Is it considered better practice to change an object with a member function or return it and assign it?

First of all, sorry if the question is worded poorly, I wasn't sure how best to phrase it. Also I should note that the code I am writing is designed to be used by others, so readability and ease of use are key.

Let's say I have a class, vec3, that contains a member function called normalize. I see two options for how to operate this function. I can use the function to normalize the data contained within the object itself, or I can create a new normalized vec3 within the function and return it.

Now obviously both of these functions would have their uses, but what I am wondering is if there is a standard practice for this sort of thing. Obviously returning a new object is more flexible as I can always assign the new object to the object:

v = v.normalize();

vec3 vec3::normalize(){
    vec3 r = *this;
    double s = sqrt(r.x*r.x + r.y*r.y + r.z*r.z);
    if(s<1e-8){
        r.x=0.0; r.y=0.0; r.z=0.0;
    }
    else{
        x /= s;
        y /= s;
        z /= s;        
    }
    return r;

}

Now perhaps modern compilers might optimize this out, but if I will almost always be using this function in this way, it seems more efficient to just change the object within the function.

v.normalize()

void vec3::normalize(){
    double s = sqrt(x*x + y*y + z*z);
    if(s<1e-8){
        x=0.0; y=0.0; z=0.0;
    }
    else{
        x /= s;
        y /= s;
        z /= s;        
    }
}

Now I could create two different functions, one to return a value and one to change the value, but doing this for every function seems like overkill, and I wouldn't be sure how to differentiate well between them for the users.

Now obviously a lot of these questions could come down to simply the personal preferences of the programmer, but if there is any standard practice for this kind of problem I would really appreciate some tips.

Thanks.

Upvotes: 2

Views: 132

Answers (2)

Nicol Bolas
Nicol Bolas

Reputation: 473856

Very broadly speaking, if a function is a member function, and it is named in such a way as to suggest an operation which mutates the object, then it probably does/should. vector::clear does not return an empty vector; it clears the current vector.

If there is a normalize function on a vector type, I would say that people will reasonably assume that it is normalizing the vector itself.

Now, that's hardly universal. Indeed, it's not even universally true of just the standard library. basic_string::substr does not convert the string into a subset of itself; it returns a new string which is a subset of the original.

So it will generally be dealer's choice. But that doesn't mean that readers of your class will not make assumptions, and the general assumption will be that member functions which seem like they mutate the object probably do. a = a.normalize() looks a bit weird.

By contrast, if normalize is a non-member function, it is reasonable to assume that it is returning a normalized vector, not normalizing the vector in-situ. The reason being that, if it mutated in-situ, you wouldn't be able to do something like this:

normalize(a + b);

People will generally assume that this will work. But if its mutating, it doesn't; it will return a reference to a temporary, which will be destroyed at the end of the expression.

By contrast, (a + b).normalize(), while technically possible, looks really bizarre and things like that are almost always a code smell. Unless you consume that vector later in the same expression, you will get a reference to a destroyed object.

That's bad. And experienced C++ programmers know that in pretty much every case, code that looks like this (calling a member function on a temporary) is doing this bad thing.

So if you're going for a mutating member function, there are two things to keep in mind: making (a + b).normalize() fail to compile, and return *this at the end, so that users can chain functions as needed. For example:

vec3 &vec3::normalize() & //Prevents the use of this function on prvalues.
{
    double s = sqrt(x*x + y*y + z*z);
    if(s<1e-8){
        x=0.0; y=0.0; z=0.0;
    }
    else{
        x /= s;
        y /= s;
        z /= s;        
    }
    return *this;
}

Upvotes: 2

Vikas Awadhiya
Vikas Awadhiya

Reputation: 308

What should be the interface of a class? It is totally depends on class and what it is representing.
For example if i consider your function vec3::normalize(), i would say normalize() should not modify object, because it is not taking any input from user and this normalization can be done in constructor itself.

Now consider a situation if someone call normalize() function more then once, then what will happen? and would you like to handle this situation? It means normalize() function should return a new object rather than modifying object.

Let's consider QRect class of Qt, it offers two functions

QRect QRect::normalized() const It performs, "Returns a normalized rectangle; i.e., a rectangle that has a non-negative width and height. If width() < 0 the function swaps the left and right corners, and it swaps the top and bottom corners if height() < 0." and this method return a new object.

second method void QRect::adjust(int dx1, int dy1, int dx2, int dy2) and it performs, "Adds dx1, dy1, dx2 and dy2 respectively to the existing coordinates of the rectangle."

So from these two functions, i can conclude if i want to perform some logic on object and wants to keep the change then i shall go for function which modify the object, and if i want to perform some logic on object and don't want any modification in it (object state should not change) then i shall go from function which return new object.

Upvotes: 0

Related Questions