Code review comment for lp:~dandrader/unity/lp940612

Revision history for this message
Tim Penhey (thumper) wrote :

Please keep the variable type and variable name separate:
  const CompWindowVector &client_list_stacking
should be
  CompWindowVector const& client_list_stacking
(or
  const CompWindowVector& client_list_stacking
   - but my preference is for the first)

> if (client_list_stacking.size() == 0)

can just be:

  if (client_list_stacking.empty())

> int pos_x = (int) fpos_x;

Please don't use C style casts. It isn't needed here (mostly
certain about that).

The do/while loop misses the first element of the vector if there
is more than one element.

A traditional for loop is more idiomatic and understandable.
Also, can remove the window and result temporaries from outside the loop.
And... if using the iterators, you don't need the empty check first
as the initial check makes sure of that too.

for (auto iter = client_list_stacking.rbegin(),
          end = client_list_stacking.rend();
     iter != end; ++iter)
{
    CompWindow* window = *iter;

    if (pos_x >= window->x() && pos_x <= (window->width() + window->x())
        &&
        pos_y >= window->y() && pos_y <= (window->height() + window->y()))
      return window;
}

return nullptr;

review: Needs Fixing

« Back to merge proposal