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;}
    }
  }
}

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.

int i = 1, j = 7;

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++).

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

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.

Can't bring myself to do it.

Why?

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

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.)

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.

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.

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.