Jump to content

  • Log In with Google      Sign In   
  • Create Account


VS2010 problem with multiple declaration in for loop


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
9 replies to this topic

#1 Khatharr   Crossbones+   -  Reputation: 2934

Like
0Likes
Like

Posted 13 January 2013 - 02:34 AM

Ehh... I'm pretty sure I'm doing this right.
 
I just got a new hard drive so I took the opportunity to upgrade to Win7 and VS2010 (since I can get them free through my school).
 
I'm trying to update some code with the C++x00 stuff in VS2010 and I came across this for loop that uses an 'erase' on an iterator. That rules out using for-each, but I noticed that I have the variables declared outside the loop and I want to bring them in. (I had put them outside before because they made the line way too long, but with 'auto' and the zooming feature it's managable now.)
 
Anyway, I tried putting the declaration of both the start and end iterators in the for(), but I got a compile error.
 
Is this thing busted (again)? I've subbed in simple int's and I stil have the same problem:
void VirtualKey::unbind(const Binding& binding) {
  auto iter = m_bindings.begin();
  auto end = m_bindings.end();
  for(int i = 1, int j = 7; iter != end; ++iter) {
    if((iter->device == binding.device) && (iter->diKey == binding.diKey)) {
      iter = m_bindings.erase(iter);
      if(iter == end) {break;}
    }
  }
}
 
I'm getting:
1>ClCompile:
1>  cl_VirtualKey.cpp
1>c:\users\khatharr\desktop\khatengine\khatengine\cl_virtualkey.cpp(22): error C2062: type 'int' unexpected
1>c:\users\khatharr\desktop\khatengine\khatengine\cl_virtualkey.cpp(22): error C2143: syntax error : missing ';' before ')'
1>c:\users\khatharr\desktop\khatengine\khatengine\cl_virtualkey.cpp(22): error C2143: syntax error : missing ';' before ')'
1>c:\users\khatharr\desktop\khatengine\khatengine\cl_virtualkey.cpp(22): error C2143: syntax error : missing ';' before '{'
1>c:\users\khatharr\desktop\khatengine\khatengine\cl_virtualkey.cpp(25): error C2043: illegal break
1>
1>Build FAILED.

Edit - Nevermind, I'm an idiot. You have to chain multiple declarations off the same type statement, so it would have to be:
int i = 1, j = 7;

Which should work for the auto's as well in this case.

Edited by Khatharr, 13 January 2013 - 02:40 AM.

void hurrrrrrrr() {__asm sub [ebp+4],5;}

There are ten kinds of people in this world: those who understand binary and those who don't.

Sponsor:

#2 Trienco   Crossbones+   -  Reputation: 2085

Like
1Likes
Like

Posted 13 January 2013 - 10:31 AM

You still have the problem of increasing your iterator twice when you erase an object.

 

Common ways are either if/else (if -> erase, else -> increment) or erasing a tmp iterator (auto tmp = iter++).


f@dzhttp://festini.device-zero.de

#3 phantom   Moderators   -  Reputation: 6891

Like
3Likes
Like

Posted 13 January 2013 - 11:32 AM

You could always use the Std lib Algorithm's to side step the mistakes smile.png

 

m_bindings.erase(std::remove_if(std::begin(m_bindings), std::end(m_bindings), 
			        [&binding](Binding &b) { return (b.device == binding.device) && (b.diKey == binding.diKey); })
		, std::end(m_bindings));



 



#4 Khatharr   Crossbones+   -  Reputation: 2934

Like
0Likes
Like

Posted 16 January 2013 - 06:51 AM

You still have the problem of increasing your iterator twice when you erase an object.
 
Common ways are either if/else (if -> erase, else -> increment) or erasing a tmp iterator (auto tmp = iter++).

 
Ah, derp. So it'd have to be:
 

void VirtualKey::unbind(const Binding& binding) {
  for(auto iter = m_bindings.begin(), end = m_bindings.end(); iter != end; ) {
    if((iter->device == binding.device) && (iter->diKey == binding.diKey)) {
      iter = m_bindings.erase(iter);
    }
    else {
      ++iter;
    }
  }
}

 

So I still can't nice-looking syntax with erase. -.-

 

 

 

 

 

You could always use the Std lib Algorithm's to side step the mistakes smile.png

m_bindings.erase(std::remove_if(std::begin(m_bindings), std::end(m_bindings), [&binding](Binding &b) { return (b.device == binding.device) && (b.diKey == binding.diKey); })
		, std::end(m_bindings));

 

Can't bring myself to do it.


void hurrrrrrrr() {__asm sub [ebp+4],5;}

There are ten kinds of people in this world: those who understand binary and those who don't.

#5 Rattrap   Members   -  Reputation: 1561

Like
2Likes
Like

Posted 16 January 2013 - 07:04 AM

Can't bring myself to do it.

 

Why?

 

This code is basically self documenting and much easier to read.



#6 Khatharr   Crossbones+   -  Reputation: 2934

Like
0Likes
Like

Posted 16 January 2013 - 05:11 PM

Nesting braces within parenthesis makes my eyes water. It's just one of those things I'm weird about.

 

I appreciate that algorithm syntax is better than it used to be, but I still just don't like it. (I'll eventually end up doing it, but I probably just need time to adapt. It took me quite a while with C++ before I started using STL.)


void hurrrrrrrr() {__asm sub [ebp+4],5;}

There are ten kinds of people in this world: those who understand binary and those who don't.

#7 sednihp   Members   -  Reputation: 241

Like
0Likes
Like

Posted 17 January 2013 - 10:01 AM

Bear in mind that when you do erase it removes the element from the vector and then moves every element behind it up one place. If your vector is of any kind of size this rearranging of memory will be a lot slower then the STL way, which moves all the elements to be erased to the end of the vector and then erases everything at the end.



#8 rip-off   Moderators   -  Reputation: 8049

Like
2Likes
Like

Posted 17 January 2013 - 11:59 AM

When it is packed into one line it can get very dense. If you separate it out it becomes a bit more reasonable:

template<typename Container, typename Predicate>
void remove_and_erase_if(Container &continer, Predicate predicate) {
    auto begin = std::begin(container);
    auto end = std::end(container);
    auto i = std::remove_if(begin, end, predicate);
    container.erase(i, end);
}
 
auto predicate = [&binding](Binding &b) {
    return (b.device == binding.device) && (b.diKey == binding.diKey);
}
remove_and_erase_if(bindings, predicate);

In particular, that helper function can be reused wherever.

 

Consider "swap and pop", if the order is irrelevant and erasing from the middle of the container is expensive.



#9 SiCrane   Moderators   -  Reputation: 9495

Like
1Likes
Like

Posted 17 January 2013 - 12:10 PM

If you swap and pop is an option, then you might also consider std::partition() with erase() as well.

#10 Khatharr   Crossbones+   -  Reputation: 2934

Like
0Likes
Like

Posted 18 January 2013 - 04:23 AM

I'll look into it more. I never thought of partition() for this, but I guess it's similar to remove_if().

 

I'm sort of surprised that there's not already an STL algorithm for the remove_if+erase combo. Seems like something that would come up pretty often.

 

Unbinds aren't performance critical and a single VitualKey should (should, lol) only have a handful of bindings at most, but I ought to practice a better strategy for this.

 

Thanks, all.


Edited by Khatharr, 18 January 2013 - 04:32 AM.

void hurrrrrrrr() {__asm sub [ebp+4],5;}

There are ten kinds of people in this world: those who understand binary and those who don't.




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