current community

your communities

more stack exchange communities

company blog
Stack Exchange Inbox Reputation and Badges
tour help
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. Join them; it only takes a minute:

Sign up
Here's how it works:
  1. Anybody can ask a question
  2. Anybody can answer
  3. The best answers are voted up and rise to the top
up vote 2 down vote favorite

BitPat here is a wrapper around a bitset.

class BitPat
{
private:
    boost::dynamic_bitset<> data;
public:    
    bool BitPat::operator[](size_t coord) const
    {
        if(coord < length)
            return data[coord];
        else
            throw std::out_of_range(std::string("Pattern coord out of range")
                + " (" + std::to_string(coord%_width) + ", " + std::to_string(coord/_width) + ") "
                + "[" + std::to_string(coord) + "]");
    }

    // vs:

    bool BitPat::operator[](size_t coord) const
    {
        if(coord >= _length){
            throw std::out_of_range(std::string("Pattern coord out of range")
                + " (" + std::to_string(coord%_width) + ", " + std::to_string(coord/_width) + ") "
                + "[" + std::to_string(coord) + "]");
        }
        return data[coord];
    }
};

I have additional getters and setters where I'd implement bounds-checking like this. It generates a lot of boilerplate code though.
As a first step maybe I should define some little formatting functions, but most importantly I'm curious which version of the if statements is more conventional.
Not sure if omitting return statements like in the first case is good practice either.

share|improve this question
up vote 2 down vote accepted

It doesn't really matter which direction you do the checking in - it's a matter of personal preference. Both are valid, semantically identical, and will almost certainly perform identically.

One thing you can do to avoid having to rewrite all of this stuff is to have a single function that does bounds-checking:

size_t boundsChecked(size_t coord) const {
    // whichever order of this you prefer
    if (coord >= _length) {
        throw std::out_of_range(...);
    }
    return coord;
}

And then defer to that function everywhere you want to add bounds checking:

bool operator[](size_t coord) const
{    
    return data[boundsChecked(coord)];
}
share|improve this answer
    
I like that method, gives the needed extensibility – MatrixAndrew Nov 3 '15 at 21:50
    
Actually, while they are semantically identical they are unlikely to perform equally well: Remember that modern CPUs are pipelined, and a branch-misprediction is really costly. – Deduplicator Nov 4 '15 at 14:48
1  
@Deduplicator Why would one be more likely to mispredict than the other? We're checking the same thing. – Barry Nov 4 '15 at 15:03
    
You could look for "static branch-prediction". But I don't know how the compiler decides to generate the code, so I cannot say whether the if-statement will do a forward-branch for its body or not. – Deduplicator Nov 4 '15 at 15:07
up vote 1 down vote

You know, your class is a golden opportunity to let private inheritance shine.

All those functions where you don't want to add any checking, just use a using-declaration to make them public.

The others, reimplement with delegation after additional checking, using a dedicate check_or_throw-function like Barry recommends.

Regarding the normal way to do things, that's first checking the preconditions, then doing the main work.
And where in your first snippet are you omitting any return-statement? I hope you are running with full warnings (-Wall -Wextra -pedantic), and the compiler should not complain.

gcc and clang have __builtin_expect, which you can use to give the compiler a hint.
Also see: http://stackoverflow.com/questions/1440570/likely-unlikely-equivalent-for-msvc

share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.