Jump to content

  • Log In with Google      Sign In   
  • Create Account

Passing vector by reference


Old topic!
Guest, the last post of this topic is over 60 days old and at this point you may not reply in this topic. If you wish to continue this conversation start a new topic.

  • You cannot reply to this topic
12 replies to this topic

#1 Tallkotten   Members   -  Reputation: 288

Like
0Likes
Like

Posted 01 August 2012 - 03:46 AM

Hello!

I've got a brief grip on this one and I've solved it half-way.

I want to pass a vector to a function by reference so that i don't need to update the vector the function has stored to know all the latest changes in the vector.

This function should do just fine:
void setVector(vector<Classname*> &vectorTemp);

the function from the class where i get the vector looks like this:
vector<Classname>* getVector()
{
return vector;
}

however i am not able to send that vector directly to the function for some reason, this code is invalid:
pointerToClass->setVector(pointerToAnotherClass->getVector());

Al-thought if i do this before i send it, everything works:
vector<Classname*> vectorToSend = pointerToClass->getVector();
pointerToClass->setVector(vectorToSend);

but that results in that i have to constantly update the "vectorToSend" with any new information from the "main" vector and then resend it to the class. How do it work around this? Can't i just send it directly without that middle-step?

How this made any sense, i'm super tired!

Sponsor:

#2 BitMaster   Crossbones+   -  Reputation: 4442

Like
0Likes
Like

Posted 01 August 2012 - 04:01 AM

Neither of the two versions should be compiling. getVector() returns a pointer to the vector (so you would need return &vector; in the first place unless your vector was a pointer - which it should not have to be and in general would indicate a design problem).
setVector() instead expects a reference but the assignment "vector<Classname*> vectorToSend = pointerToClass->getVector();" should not compile.
pointerToClass->setVector(pointerToAnotherClass->getVector());
should compile fine if getVector() returned a reference or setVector accepted a pointer. Of course
pointerToClass->setVector(*pointerToAnotherClass->getVector());
would work without further change but I'm not a fan of using pointers and references interchangeably unless there is a good semantic reason for it (which I cannot see here).

On a sidenote, saying "for some reason, this code is invalid" is not helpful. The compiler (or linker) produces an error message which should have been posted.

Edited by BitMaster, 01 August 2012 - 04:03 AM.


#3 Tallkotten   Members   -  Reputation: 288

Like
0Likes
Like

Posted 01 August 2012 - 04:32 AM

Neither of the two versions should be compiling. getVector() returns a pointer to the vector (so you would need return &vector; in the first place unless your vector was a pointer - which it should not have to be and in general would indicate a design problem).
setVector() instead expects a reference but the assignment "vector<Classname*> vectorToSend = pointerToClass->getVector();" should not compile.

pointerToClass->setVector(pointerToAnotherClass->getVector());
should compile fine if getVector() returned a reference or setVector accepted a pointer. Of course
pointerToClass->setVector(*pointerToAnotherClass->getVector());
would work without further change but I'm not a fan of using pointers and references interchangeably unless there is a good semantic reason for it (which I cannot see here).

On a sidenote, saying "for some reason, this code is invalid" is not helpful. The compiler (or linker) produces an error message which should have been posted.


Must have written something wrong then. Because I've got a somewhat working code now that i compiling but not entirely doing what i want. And the reason i didn't post the error is because i am not currently coding. But the error basically states that what i try to compare isn't in the same form, one is reference and one is pointer or vice verse.

Thanks for your code, will test it later! What would you use here then? if not pointers

#4 patrrr   Members   -  Reputation: 1053

Like
2Likes
Like

Posted 01 August 2012 - 05:51 AM

void setVector(vector<Classname*> &vectorTemp);

You probably want: (notice the const)
void setVector(const vector<Classname*> &vectorTemp);

the function from the class where i get the vector looks like this:

vector<Classname>* getVector()
{
return vector;
}

And here, you probably want
vector<Classname*> &getVector()
{
return vector;
}

or

const vector<Classname*> &getVector() const
{
return vector;
}

or preferably both.

Al-thought if i do this before i send it, everything works:

vector<Classname*> vectorToSend = pointerToClass->getVector();
pointerToClass->setVector(vectorToSend);

You have to do it this way because you can't take a non-const reference to a temporary. Your getVector returns a temporary copy of your vectory (assuming the place where you put the * was a typo), and to be able to send it directly to a function through a reference requires the reference to be const.

Edited by patrrr, 01 August 2012 - 05:54 AM.


#5 Tallkotten   Members   -  Reputation: 288

Like
0Likes
Like

Posted 02 August 2012 - 03:46 AM

helpful text!


Thank you guys so much!


But patrr may you continue to explain why it has to be const? And what does const really do, i mean what is it really other that making something constant? What does it allow me to do?

#6 samoth   Crossbones+   -  Reputation: 5038

Like
0Likes
Like

Posted 02 August 2012 - 05:43 AM

And what does const really do, i mean what is it really other that making something constant? What does it allow me to do?

The const qualifier does mainly two things:
1. It is a promise (not more than that) to the compiler that you will not modify the value. You can still cast the const away and modify the value anyway, but the compiler will assume that you keep your promise and might make optimizations based this, so cheating is unwise.
2. It allows you to bind a reference to a temporary, and extends the lifetime of that temporary to the lifetime of the const reference. Otherwise, if you were allowed to bind to a temporary (say, returning a local variable by reference) then whatever you alias does not exist any more by the time you use it, which is bad mojo. You would be reading and possibly modifying some more or less arbitrary data on the stack which might be overwritten by or overwrite other structures without your knowledge. Bad situation.

The newer C++11 rvalue references do a somewhat similar thing as in (2.), they allow you to "scavenge" a temporary by reference, omitting a copy. This works due to the fact that the temporary would die immediately after the return statement anyway. Thus, since it's already certain that the temporary is gone, nobody will notice that it's "missing", so another object can "steal" it with not side effects.

Edited by samoth, 02 August 2012 - 05:45 AM.


#7 rip-off   Moderators   -  Reputation: 8764

Like
2Likes
Like

Posted 02 August 2012 - 09:18 AM

Const is about restricting what you can do. The idea is that if a given section of code should not modify a variable, by passing it as const you will not accidentally do so.

Const is hard, because it cascades from the point of declaration to any other points of use. However, this makes such changes more obvious. If you want to change some code from modifying a value to not modifying it, the compiler will help you track down any corner cases you forgot.

#8 Tallkotten   Members   -  Reputation: 288

Like
0Likes
Like

Posted 03 August 2012 - 02:28 AM

Thank you!

Should i use this as much as possible? From basically every get function and everywhere where i wont modify the value it receives?

Would this solve the warning "Warning: taking value of temporary". I've got that one in some places

#9 rip-off   Moderators   -  Reputation: 8764

Like
2Likes
Like

Posted 03 August 2012 - 09:03 AM

Should i use this as much as possible?

I think doing so is a good idea. However, trying to migrate an established code base is a non-trivial task, it might be better in such cases to leave well enough alone and apply the rule at the start of your next project.


Would this solve the warning "Warning: taking value of temporary". I've got that one in some places

Does the warning actually say "Warning: taking address of a temporary"? No, this won't solve that, you need to change the code to not produce this warning. This is a serious warning - it indicates actual bugs in your code.

Ideally, your code would compile cleanly without warnings. Again, retroactively cleaning up an established code base would take time. However, warnings almost always indicate valid problems, so I would advise you to try to do this if possible. It would be higher priority than trying to enforce "const correctness" in your code.


Try to configure your toolchain to have the highest warning level possible, and set it so that warnings are treated as errors. Sometimes having the warning level at the very top can produce some false positives that might be better suppressed (e.g. forcing value to bool 'true' or 'false' (performance warning)), but given that you are posting in For Beginners I would encourage you to not suppress any warnings. Also, don't use unsafe casts to fix warnings.

The only real fix for warnings is to understand what they mean and really fix the code. Suppressing the warning, or adding in some voodoo code that makes the warning go away is probably more dangerous than leaving the code as is, where at least you're aware there are potential bugs when you compile it.

#10 Tallkotten   Members   -  Reputation: 288

Like
0Likes
Like

Posted 03 August 2012 - 09:43 AM


Should i use this as much as possible?

I think doing so is a good idea. However, trying to migrate an established code base is a non-trivial task, it might be better in such cases to leave well enough alone and apply the rule at the start of your next project.


Would this solve the warning "Warning: taking value of temporary". I've got that one in some places

Does the warning actually say "Warning: taking address of a temporary"? No, this won't solve that, you need to change the code to not produce this warning. This is a serious warning - it indicates actual bugs in your code.

Ideally, your code would compile cleanly without warnings. Again, retroactively cleaning up an established code base would take time. However, warnings almost always indicate valid problems, so I would advise you to try to do this if possible. It would be higher priority than trying to enforce "const correctness" in your code.


Try to configure your toolchain to have the highest warning level possible, and set it so that warnings are treated as errors. Sometimes having the warning level at the very top can produce some false positives that might be better suppressed (e.g. forcing value to bool 'true' or 'false' (performance warning)), but given that you are posting in For Beginners I would encourage you to not suppress any warnings. Also, don't use unsafe casts to fix warnings.

The only real fix for warnings is to understand what they mean and really fix the code. Suppressing the warning, or adding in some voodoo code that makes the warning go away is probably more dangerous than leaving the code as is, where at least you're aware there are potential bugs when you compile it.


Ok, thank you. I'll look into that.

This code
iMngr->collision(&camera, myCursor, &getTileCollision(x, y), player, map->getMap(), enemyVector, currentInterface, 0);
gets the warning. I am guessing that the value that is returned by getTileCollision is temporary. Only places with a call to a function gets this warning.

I've also got this warning at one place
warning: extended initializer lists only available with -std=c++0x or -std=gnu++0x

This is the code:
textColor = {0, 0, 0};

I tried to solve that before but it created more problems that it solved. But again i'll look into these warning and fix them.

#11 rip-off   Moderators   -  Reputation: 8764

Like
1Likes
Like

Posted 03 August 2012 - 10:26 AM

The first line of code is not legal. The fix is to store the temporary:
CollisionType collision = getTileCollision(x, y)
iMngr->collision(&camera, myCursor, &collision, player, map->getMap(), enemyVector, currentInterface, 0);

Pre C++11, you can only initialise variables in this manner, not assign to them. I'm not familiar enough with C++11 initialiser lists to comment on whether it is legal in it. I would guess you'd need to write the appropriate assignment operator.
textColor = {0, 0, 0};
To fix it, use a constructor call, declare a temporary or make a factory function (useful if you do not control the type):
textColor = Color(0, 0, 0);

// or
Color temp = { 0, 0, 0 }
textColor = temp;

// or
Color make_colour(int r, int g, int b) {
    Color color = { r, g, b };
    return color;
}

textColor = make_color(0, 0, 0);


#12 Tallkotten   Members   -  Reputation: 288

Like
0Likes
Like

Posted 03 August 2012 - 12:51 PM

Thank you very much. Really this helped me so much!

The reason why i didn't resolve the "taking address of temporary" before was that i thought that if i used more code it would take up more CPU power. It wont?

Edited by Tallkotten, 03 August 2012 - 12:57 PM.


#13 rip-off   Moderators   -  Reputation: 8764

Like
1Likes
Like

Posted 06 August 2012 - 06:17 PM

The reason why i didn't resolve the "taking address of temporary" before was that i thought that if i used more code it would take up more CPU power. It wont?

Lines of code does not 100% map to CPU usage. In any case, you probably don't want to trade performance for crashes.

Here are some performance rules of thumb:
  • Is the game/program running too slow on the target hardware? If so, continue on below. If not, then you don't need to optimise for the time being. Make the game better instead!
  • 80% of the time in your program is consumed in 20% of the code. The 20% of code is called the "bottleneck". Any time spent optimising the other 80% of the program is poorly spent.
  • Programmers are notoriously bad at guessing where these "bottlenecks" actually are. This means that even programmers who know not to optimise the 80% of their program can often end up doing so. Instead of guessing, start measuring. Learn to use a profiler.
  • When removing known bottlenecks, apply algorithmic optimisations first. In the case of collision detection, are you doing spatial partitioning?
  • Finally, if you have determined where the bottleneck lies and exhausted any algorithmic optimisations, you might then consider tricks on the level of individual lines of code like that. You need to be able to measure the performance difference with the previous implementation to ensure you are actually making a difference, and that the difference is positive!





Old topic!
Guest, the last post of this topic is over 60 days old and at this point you may not reply in this topic. If you wish to continue this conversation start a new topic.



PARTNERS