Refactoring is fun, especially on a slow day. The "Introducing an Explaining Variable"[1] refactoring is:
if (version < 8 && ua ~ "IE") ...
is better written as
IE8 = (version < 8 && ua ~ "IE");
if (IE8) ...
This allows variable IE8 to be (1) reused later on in the code, (2) is self documenting and (3) sets a precedent for extending it easily by future devs:
IE8 = (version < 8 && ua ~= "IE");
mobile = (ua ~= "mobile")
if (IE8 || mobile) ...
Another quick one is Guard Clauses[2].
function {
if (...) {
...
} else {
...
}
}
where either block is long, is better written as
function {
if (...) {
return ...;
}
...
}
This site http://sourcemaking.com/refactoring is worth a read even if you have been programming for years; especially if you have been programming for years and haven't developed good habits.
I like functions that return a value to be written in the guard-clause style. [In C++, that's probably better for efficiency since the compiler optimizer doesn't have to remove temporaries by converting the code to guard-clause style.]
However, I'm not convinced that void functions are better written in the guard-clause style. I think the intent of the code is better expressed with nested ifs. Of course, try to factor the ifs so that they're readable.
I'm saying this is more readable if the "do something part" is less than about 15 lines. Thoughts?
void do_something(input) {
if (!input.already_done()) {
// do something.
}
}
Those guard clauses is something that I've got more used to doing since using ReSharper as it is something it is fairly agressive in suggesting (and will do the refactoring for you).
It's not something I'd previously considered doing as it seems a bit unnatural at first, but reducing nesting does improve readability over dogmatically sticking to "single return point".
