Take the 2-minute tour ×
Programmers Stack Exchange is a question and answer site for professional programmers interested in conceptual questions about software development. It's 100% free, no registration required.

Which is a better option?
It's not always that when you have something creative your code is going to look ugly.
But at times it does go a bit ugly.
e.g.

if ( (object1(0)==object2(0) &&
     (object1(1)==object2(1) &&
     (object1(2)==object2(2) &&
     (object1(3)==object2(3) )
    retval = true;
else
    retval = false;

is simple and readable

bool retValue = (object1(0)==object2(0)) && 
                (object1(1)==object2(1)) && 
                (object1(2)==object2(2)) && 
                (object1(3)==object2(3));

but having something like this will make some newbies scratch their heads.

So what do I go for? including simple code everywhere might sometime hamper my performance.
What I could think of was commenting wherever necessary but at times you get too curious to know what is actually happening.

share|improve this question
12  
I think I understood the question better before I looked at the example provided. –  sq33G Mar 29 '12 at 10:58
24  
both snippets are complex and unreadable –  BЈовић Mar 29 '12 at 11:01
10  
Is it just me, or are the examples mixed up? –  scrwtp Mar 29 '12 at 11:01
2  
...the edit has made the first example incorrect? lots of ()'s getting lost there. In any case, hasn't the idea of boolean conditions returning boolean values been discussed in some form or another in the past? Why yes. Was that your question? –  sq33G Mar 29 '12 at 11:14
1  
You don't have to make your code 'noob' friendly. I'll go with elegance and simplicity. –  kadaj Mar 30 '12 at 6:39

8 Answers 8

up vote 34 down vote accepted

Your second version IS far more simple and readable, and much better in every way. It's not creative or complex, but perfectly normal, straightforward code.

The only way in which it might confuse newbies is that it requires them to understand that complex boolean expressions are A) still expressions like any other and B) can be used wherever a boolean value is required, rather than just inside an if clause.

But this is something newbies really need to understand, so you should never let the possibility of someone not yet understanding it influence your code.

share|improve this answer
    
about simplicity people do find both the eg complex. –  Shirish11 Mar 29 '12 at 11:36
    
much better in every way ... except when number of objects to compare raises to 100000 ;) –  BЈовић Mar 29 '12 at 12:08
    
@VJovic: the versions I answered for both enumerated individual comparisons, so that does not make a difference. –  Michael Borgwardt Mar 29 '12 at 12:36
    
+1 on the "this is something newbies really need to understand" -- if you don't have boolean logic down, then how can you understand more complicated concepts? –  gahooa May 24 '12 at 21:56

I don't see what's creative about either example at all, you're mostly just changing whitespace. If you think it's (potentially) unclear, try clarifying your intent, eg:

bool sameForEvery(int begin, int end)
{
  for (int i = begin; i <= end; ++i)
  {
    if (object1(i) != object2(i)) return false;
  }
  return true;
}

bool allthesame = sameForEvery(0, 3);

in general, understanding the implementation is easier if you know what the code is trying to achieve, and if the function (and variables) are well-named, you don't need to read the implementation to have a reasonable idea what the caller is doing.


Edit: went back and read the original version of the question.

Are you suggesting that manually unrolling this loop is a complex optimisation that might confuse people?

share|improve this answer
    
the for loop is easily readable but the second eg might cause some problems for some who are not aware of it. –  Shirish11 Mar 29 '12 at 11:32
3  
it really shouldn't. If you feel the need to clarify it, I'd rename bool retValue to bool allthesame or similar. As I say, it's more important IMO to be clear about what you're doing (and why), and accept people may have to give some thought to how. –  Useless Mar 29 '12 at 12:06
1  
+1. A named function like this is much more quickly understandable. Actually, reading sameForEvery made me go back and notice that the original example was checking exactly that. Self-documenting function and variable names make code much more readable (not to mention that a function is reusable). –  Nathan Long Mar 29 '12 at 18:34

including simple code everywhere might sometime hamper my performance.

Don't prematurely optimize. Readable code is likely maintainable code. It is easier to optimize maintainable code, than to maintain unreadable (but fast) code.

That being said, both of your examples are less than readable. iterating through (as in @Useless's) answer, might help.

share|improve this answer

Respect to above mentioned example, both are readable and understandable perfectly. In general we should balance both.

share|improve this answer

Edit: The first revision of the question for which I have provided the answer below was changed. Since the question has already been well answered, I won't update this answer.

Your first approach is extendable to accommodate for more occurrences. If in the future your business would require to accommodate 50 objects for example, you must go with the first approach and even abstract the count by providing a variable limit for the loop. If this is not the case, option 2 is better because it is much simpler, contains less variables, and more concise.

Side notes:

1-What if one of the objects is null? Is this possible? One should consider this case.

2-Why did you not initialize the result before the loop in method 1? It is a good practice to do that.*

share|improve this answer
2  
-1: so wrong in every way –  sparkleshy Mar 29 '12 at 11:33
2  
@sparkleshy: you did note the first example changed completely after the answer was posted? –  scrwtp Mar 29 '12 at 11:36
    
@scrwtp: yes :) –  sparkleshy Mar 29 '12 at 12:01
    
note answer is relevant only for rev 1 of the question; change made in rev 2 rendered answer senseless –  gnat Mar 29 '12 at 12:09
3  
@sparkleshy, I don't consider 50 && to be OK. The question I posted the answer for, had a for loop with array items simulating and breaking when the first false condition is detected so as to simulate the &&. This is what I meant by 'first approach'. I guess you compared the current version of the question with my answer. The current answer is not valid because the question has changed. –  Emmad Kareem Mar 29 '12 at 13:38

I think that both your examples are simple and readable. In general, I follow the Occam's razor or KISS principle: do not use a concept that is more difficult to understand unless you have some real gain from it.

In other words, the problem you are trying to solve is probably complex enough, do not make your solution even more complex. I would add that sometimes a simple solution requires a deeper analysis and much more creativity than a complex one.

I think these principles are particularly important if

  1. You are working in a team: other programmers are going to have to read your code. They all know you are a good programmer already, no need to show off, just make it simple and readable.
  2. You are developing something you have to deliver in time and according to some quality requirement: do not add extra complexity that you will have to manage yourself sooner or later. Customers won't read your source code.

On the other hand, if you are coding for your own fun and you want to try out new stuff, then do what you like better.

Just my 2 cents.

share|improve this answer

When doing any sort of optimization - whether trying to write it all in one line (optimizing my time), or use a shortcut to do the work (optimizing the compiler's time, or maybe the end user's time) - I try to balance the gain against the loss of time on the part of the reader who will come along later and try to understand my code.

One way to try to judge the cost/benefit is to add a comment to the optimized code that will make it as clear as the readable version. If it's too hard to explain well in a comment, it might be too costly.

share|improve this answer

Others have covered the "what's good/bad"-- here's an additional point that may have been missed:

Newbies scratching their heads is good.

By "newbie", I mean new programmer. Not "new addition to team".

Whenever a newbie scratches his head, he learns something. He may have to ask someone else, but he'll learn in the end. The example you've given is readable to any intermediate programmer, and will teach the fact that "the conditions inside ifs can be used directly to make bools--thus conditional statements are bools". Pretty useful thing to know--but AFAIK never really explained in books/courses.

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.