How can I simplify or improve readability in complex logic statements?

Question

So I have this monster of a logic statement:

(((ncode[i]+key[ikey])>90 && (ncode[i]+key[ikey])<97) || (ncode[i] + key[ikey])>122)

I was always told that if you have logic statements that require a new line you might be doing something wrong. I understand that I could probably refactor it into a new method but it does not necessarily make sense to do that for what I am working on. What can I do to make it more readable?

Readable for who? You? Me? The Pope? One line, two lines, three lines, bored? You like 80 columns, i like 100 columns. You like really short names I like descriptive names. You prefer quiche and I'll take whatever's in the vending machine. — Captain Obvlious, Apr 16 '16 at 22:58
Always write code as if the maintainer is a psychopath that knows where you live :-P — Fraser G, Apr 16 '16 at 23:04

score 2 · Answer 1 · answered Apr 16 '16 at 22:59

2

I'd simply use a temporary variable to clean things up:

auto code = ncode[i] + key[ikey];
if ( (code > 90 && code < 97) || (code > 122) )

Most compilers will optimize the temporary away anyway.

answered Apr 16 '16 at 22:59

Unimportant

2,076
14
19

Yeah and now you've just polluted the scope and an unnecessary variable. This contradicts other good practices. Is this good or is this bad? Does it matter? Why not just go full Gump and us ea `for` statement. `(for(auto code = 1; code > 90 && code < 97 || code > 122;) {}` – Captain Obvlious Apr 16 '16 at 23:04
Why not `{ ...code... }` ? I don't think creating unnecessary variables is a bad thing if it makes code more readable/maintainable. Pollution can be handled with a `{ }` block and if you don't make humoungously large functions it wont matter much anyway because it's local. For performance- most compilers will optimize it away anyway and if it does matter focus on the 10% of the program that takes 90% of the execution time but keep the rest readable. – Unimportant Apr 16 '16 at 23:10
I'm just pointing things out and asking questions. If you're going to go all the way to adding a local variable use a lamba which reduces your expression to `if(ConditionMet(code)) [}`. But only if you're _really_ looking for readability. – Captain Obvlious Apr 16 '16 at 23:14
I don't event consider not giving two shits about performance until my code is written, works properly, and is ready for profiling. Why are you even mentioning it – Captain Obvlious Apr 16 '16 at 23:19
Because i'm also just trying to point things out and answer. No worries. – Unimportant Apr 16 '16 at 23:21
It is far better to use predicates, as they can be used in other cases, whereas directly coding the condition everywhere can lead to errors in transcription, and violates the one definition rule. – dgsomerton Apr 17 '16 at 00:01

score 0 · Answer 2 · answered Apr 16 '16 at 23:02

Assuming the values in ncode and key are both integers (or floating point numbers), could you not assign their summed value to a new variable? Turn the line into:

int result = ncode[i] + key[iKey];
if( (result > 90 && result < 97) || result > 122)
    //Logic

It becomes two lines but it does increase the readability in my opinion.

score 0 · Answer 3 · answered Apr 16 '16 at 23:10

I would use a const temp variable to replace the three duplicated (ncode[i]+key[ikey]) for both performance and readability. Don't repeat yourself.

const auto temp = ncode[i] + key[ikey];
if ( ((90<temp) && (temp<97)) || (122<temp) )

Also, use only < in this case. Therefore, the first two comparison becomes "temp is between 90 and 97. Imagine draw a line with 90, 97 and 122 on the line, the "good" temp range on the line relating to those three points.

Yakk - Adam Nevraumont · Answer 4 · 2016-04-17T00:41:31.280

template<class Lower>
auto is_gt(Lower&&l){
  return [=](auto&&x){ return l<x; };
}
template<class Higher>
auto is_lt(Higher&&l){
  return [=](auto&&x){ return x<l; };
}
template<class L, class R>
auto also(L&& l, R&& r){
  return [=](auto&&x){return l(x)&&r(x); };
}
template<class L, class R>
auto either(L&& l, R&& r){
  return [=](auto&&x){return l(x)||r(x); };
}
template<class Lower, class Upper>
auto between(Lower&& l, Upper&& u){
  return also(is_gt(l), is_lt(u));
}

are some toys.

Then:

if (either(between(90,97), is_gt(122))(ncode[i]+key[ikey])) {

This can be made a bit more slick. If we define a predicate to be a function object returning bool that supports && and || and ~ overloading, and have our above toys return predicates, we get:

if (( between(90,97) || is_gt(122))(ncode[i]+key[ikey])) {

getting rid of the either prefix noise.

For more efficiency in non-int cases, capture the bounds by move.

template<class F>
struct predicate_t:F{
  predicate_t(F const& f):F(f){}
  template<class F2>
  friend auto operator&&(predicate const& f1,predicate<F2> const& f2){
    auto r=[=](auto&&...args){
      return f1(args...)&&f2(args...);
    };
    return predicate_t<decltype(r)>(r);
  }
  template<class F2>
  friend auto operator||(predicate const& f1,predicate<F2> const& f2){
    auto r=[=](auto&&...args){
      return f1(args...)||f2(args...);
    };
    return predicate_t<decltype(r)>(r);
  }

  template<class F2>
  friend auto operator!(predicate const& f1){
    auto r=[=](auto&&...args){
      return !f1(args...);
    };
    return predicate_t<decltype(r)>(r);
  }
};
template<class F>
predicate_t<std::decay_t<F>>
predicate(F&&f){
  return std::forward<F>(f);
}

With each of the above toy functions returning return predicate(lambda); instead of lambda.

This code is simpler and shorree at the point of use, should have zero run time overhead, but is a different style of coding than conventional C++.

How can I simplify or improve readability in complex logic statements?

4 Answers4