Clean Code

[leeor_net]

So... I've been working on OPHD again and I started building the new UI code. After sleeping on it for a night I realized that I was falling into an old trap:

Code: [Select]

void MainReportsUiState::onMouseDown(EventHandler::MouseButton button, int x, int y)
{
    if (button == EventHandler::BUTTON_LEFT)
    {
        for (Panel& panel : Panels)
        {
            if (isPointInRect(MOUSE_COORDS, panel.Rect))
            {
                panel.Selected = true;
            }
            else
            {
                panel.Selected = false;
            }
        }
    }
}

Could be much more effectively written as:

Code: [Select]

void MainReportsUiState::onMouseDown(EventHandler::MouseButton button, int x, int y)
{
    if (button == EventHandler::BUTTON_LEFT)
    {
        for (Panel& panel : Panels)
        {
            panel.Selected = isPointInRect(MOUSE_COORDS, panel.Rect);
        }
    }
}

And of course for those who hate the curly braces:

Code: [Select]

void MainReportsUiState::onMouseDown(EventHandler::MouseButton button, int x, int y)
{
    if (button == EventHandler::BUTTON_LEFT)
        for (Panel& panel : Panels)
            panel.Selected = isPointInRect(MOUSE_COORDS, panel.Rect);
}

And for the extreme minimalist:

Code: [Select]

void MainReportsUiState::onMouseDown(EventHandler::MouseButton button, int x, int y)
{
    if (button == EventHandler::BUTTON_LEFT) for (Panel& panel : Panels) panel.Selected = isPointInRect(MOUSE_COORDS, panel.Rect);
}

[Hooman]

A classic 


And for those who don't hate curly braces, but do want more vertical density:

Code: cpp [Select]

void MainReportsUiState::onMouseDown(EventHandler::MouseButton button, int x, int y) {
    if (button == EventHandler::BUTTON_LEFT) {
        for (Panel& panel : Panels) {
            panel.Selected = isPointInRect(MOUSE_COORDS, panel.Rect);
        }
    }
}

The curly braces do help guard against common mistakes, such as revisiting code a long time later to add or remove a line, and thus breaking a loop or if statement because they only apply to the next statement. And also, indentation might lie, but curly braces don't.

[leeor_net]

That's why I enforce the braces even for something like if(expression) { return; } -- helps to avoid humiliating mistakes like Apple's goto fail bug... which aside from the very obvious mistake is also genuinely bad code altogether.