Is it bad style of coding ??

Started by
13 comments, last by MadKeithV 15 years, 5 months ago
class Example has operator+= overloads. Example Class keeps a private vector of std::vector<X*> type that it updates with several functions and operator overloads.
Example& operator+=(X* x);
Example& operator+=(X& x);
X is a Class that takes have constructor (not explicit) that takes int.
X* x1 = new X(1);
X* x2 = new X(2);
X x3(3);
X x4 = 4;
Example ex;
ex += x1;
ex += x2;
ex += x3;
ex += x4;
So all this works as expected. But What I want to do is.
ex += 5;
But it requires signature to be
Example& operator+=(const X& x);
But I am storing non-const pointers in the vector. So What I did is I const_cast<X*>(x) it in operator+=(const X&) Now Is that a design flaw ?? cause I am not maintaining the constness. actually I dont use const_cast much thats why I am asking is what I am doing assumed as bad style of coding ??
Advertisement
The number '5' there is a literal number, which means it's temporary value that sits on the stack. As soon as the statement ex += 5 finishes, the reference to it no longer exists. I would re-think whatever you're trying to do.

Also, most people accept overloading operators like += for mathematical operations or string concatenation, but it might be confusing for adding to containers. For clarity, I would just use a normal method name like append(), or push_back() if you'd like to use it alongside STL code. But this is just my opinion :)
const_cast is only acceptable when dealing with immutable libraries that do not accept const values when they could.

In your case this it's ill advised. If you want your Example class to be able to store const objects, change the vector so that it accepts const values. It doesn't attempt to change the values added to it anyways, does it?

Additionally, dereferencing a reference parameter and then staring the address is very dangerous and will cause problems if stack values are passed as hh10k pointed out (In your example x3 and x4).

The cleanest way to do this would be to store copies of the values passed to operator+=. Then you don't have to worry where the parameter was stored originally or if it's const or not.

Last but not least, I too think that using operator+= to append to a container is not intuitive and will confuse a lot of people.
I think using += operator overload for basic types like int, char, char*, std::string etc.. will be easier for the users.
and This is WHAT I NEED TO DO (obviously with minimum unsafe risks)

and secondly the Example class will have several method to update data on the vector and also if some getter operation is done on Example and it returns a const X* it would be a problem for users to handle it so there will be another const problem.

Quote:The number '5' there is a literal number, which means it's temporary value that sits on the stack. As soon as the statement ex += 5 finishes, the reference to it no longer exists. I would re-think whatever you're trying to do.
Hmm I am coding something like this.
Example& Example::operator+=(const X& x){  l.push_back(&x)}


However If I change it to something like

l.push_back(SomeCopyConstructorCall(x))


I think this problem will disappear right ??
Quote:Original post by nlbs
Hmm I am coding something like this.
Example& Example::operator+=(const X& x){  l.push_back(&x)}


This attempts to store the address of 'x' inside 'l'. Therefore, the language requires that 'x' exists and will remain at that address until it is removed from 'l'. This means that values without an address, such as the integer literal 5, cannot be added to 'l', and that values with a limited lifetime (such as variables on the stack) cannot be added to 'l' unless 'l' is also a stack variable that was defined after them, or they are removed from 'l' before their scope is left.

Ultimately, this leads to the question of ownership. Your private vector contains pointers. These pointers point to objects. Who owns these objects?
Example& Example::operator+=(const X& x){
l.push_back(x)
}

This stores a copy of x. No need to manually invoke a copy constructor.
Of course, the type of l must be std::vector<X> then.

Quote:Original post by hh10k
The number '5' there is a literal number, which means it's temporary value that sits on the stack. As soon as the statement ex += 5 finishes, the reference to it no longer exists. I would re-think whatever you're trying to do.
Actually, it's highly likely in the instruction cache, so it's not even on the stack. IIRC "ADD RAX, i64" compiles into a binary instruction with i64 embeded into.
My intension is to do ex += 5.

so what if I do accept const X& x as arguments and pass it as

X* __tmp = new X(x);
l.oush_back(__tmp);

I don't this these dangaling problems would appear.
Yes, you can use l.push_back(new X(x)). Don't forget to delete things after you're done.

But If I code like this a different problem will appear if I do this. although the problem is not so critical or important.

it is when a pointer is supplied its the vector content is linked with the actual content when a reference is supplied the vector content is also linked with the original one.
But when a const reference is supplied its clone is there on vector content.

However one thing I would like to make sure

when an application exists does all Operating System cleans the garbage memory(e.g. new Objects on which delete has not been used) accuired by it automatically ??

This topic is closed to new replies.

Advertisement