macOS How do you decide between elegance and readability

[Sydde]

I just glanced at a fragment of code

				setScanner = [self objectEnumerator];
while ( thing = [setScanner nextObject] )
  {
      if ( NSOrderedSame == [(NSString *)aValue caseInsensitiveCompare:thing] )
          break;
  }

and thought, "well, I could re-write that loop in one line

while ( ( thing = [setScanner nextObject] )&&( NSOrderedSame != [(NSString *)aValue caseInsensitiveCompare:thing] ) ) ;

AFAICT, these two forms compile to essentially the exact same object code. For me, "break" is usually a rather unattractive statement, yet at the same time, an empty loop is really no more appealing. So it got me to wondering, at what point does one choose one form over the other? I realize businesses typically have coding guidelines that leave the programmer little discretion, but from a personal standpoint, what is preferable?

When writing method calls, do you personally prefer to nest them

[[[aThing aProperty] itsProperty] aMethod];

or break them up

thingProperty = [aThing aProperty];
otherObject = [thingProperty itsProperty];
[otherObject aMethod];

and how strict do company coding guidelines tend to be in such cases?

Not being a professional, I tend to decide method nesting based on need. If I will be using an object reference again, I will grab a copy of it so that the accessor method does not get called repeatedly, but sometimes reliable nested methods are just quicker to write.

(Edit) fixed the second loop example

 

[lee1210]

Can you describe what the result of the first example should be? Having an iterator at the position in a collection of a particular string (case insensitive)? And then what? This looks to me like self is an NS(Ordered?Mutable?)Set, so it implements Fast Enumeration. That says to me that this should be:

for(NSString *toCompare in self) {
  if(NSOrderedSame == [toCompare caseInsensitiveCompare:(NSString *)aValue]) {
    break;
  }
}

I don't know if casting aValue is totally necessary, or if so, it might be good to have an NSString * that it's assigned to before if it's used more than a few times, to avoid clutter. The purpose of this still escapes me, and fast enumeration destroys any state of the enumeration so it may not be effective.

If you're on 10.4 or below, i guess i'd prefer the first version because the longest line is about 80 characters, while the longest line in the second is 120. Yes, we have big monitors, but I still prefer to keep lines shorter if possible.

As for nesting message passes... sometimes memory management prevents this, so you don't have a choice. But when you do have a choice... if you are never going to use the intermediate values again, I suppose I'd opt for the more terse version if the purpose/method names/etc. make it clear what's going on. My tune changes if the line grows too long, as noted above, though. I guess this doesn't speak well for consistency, but I think clarity should be strived for beyond religious adherence to a particular style.

This is pretty subjective, though, so expect a wide variety of responses.

-Lee

 

[Sydde]

The purpose of the loop was to scan through a set (NSCountedSet, to be specific, of which self is a subclass) and return "thing" as either the object that matches "aValue" or nil (meaning "aValue" was not found in the set).

I write almost exclusively for 10.4+, though I run 10.5

 

[mfram]

If you are writing this code for your own use and won't share it with anyone, then it doesn't matter which method you choose. But if this code will be maintained by someone else in the future, then you should make it as straight-forward and readable as possible.

Even if the code is just for you, it might be a good idea to make it so that it can be more easily modified in the future. In your example, what if you all of a sudden needed to check for two different strings instead of just one string? Modifying the code with the broken out while loop will be a lot easier than messing with the "fancy" version.

As a software professional that works in a team environment, I can say it's hard enough to try and maintain someone else's code. But trying to maintain "clever" code can be a nightmare.

 

[lee1210]

So if something matching aValue is found it's returned (but the Set version, not aValue itself, in case they are different cases?), if not, nil is returned?

setScanner = [self objectEnumerator];
NSString *retVal = nil;
while ( thing = [setScanner nextObject] && retVal == nil)
  {
      if ( NSOrderedSame == [(NSString *)aValue caseInsensitiveCompare:thing] ) {
          retVal = thing;
      }
  }

I guess i'd prefer that to break, and you have your return value stored.

-Lee

 

[Sydde]

So if something matching aValue is found it's returned (but the Set version, not aValue itself, in case they are different cases?), if not, nil is returned?

setScanner = [self objectEnumerator];
NSString *retVal = nil;
while ( thing = [setScanner nextObject] && retVal == nil)
  {
      if ( NSOrderedSame == [(NSString *)aValue caseInsensitiveCompare:thing] ) {
          retVal = thing;
      }
  }

I guess i'd prefer that to break, and you have your return value stored.

-Lee

But "thing" is the return value if it matches "aValue".

 

[lee1210]

But "thing" is the return value if it matches "aValue".

Is that different than what I have there?

-Lee

 

[larkost]

My usual rule-of-thumb goes about like this: if I stopped on this project right now, and did not come back to it for a year, would I understand what the line was doing immediately? If I can answer yes, then the line is probably allright, if not then taking it apart and commenting more is probably needed.

The other thing that often breaks this up in production code is troubleshooting code (I use macro that change based on compile setting to make sure this they don't affect production code), and error checking.

 

[Sydde]

Is that different than what I have there?

-Lee

Only in terms of style.

 

[jpyc7]

In the given code with 2 versions, I think the first version (more readable) is definitely better. As larkost noted, the first version lets you insert code. A contrived example could be that you might want to capitalize the string object before returning its reference.

Just like computers' caches, humans read more often than write so the general rule is to strive for readability instead of elegance. Another guideline is to avoid optimizing too early (potentially making code more elegant) since that effort is often wasted as one re-designs the code.

 

[mslide]

I tend to think more explicit lines of code is better than trying to condense things into one or 2 lines. As for what I would do in this specific example (not that I necessarily think that any of the given choices are unreadable), I would do something like what lee suggested except I tend to not like to assign variables inside loop or if conditionals. I tend to leave the conditionals to do just that. Again, it may result in more lines of code, but it is very clear.

 

[Sydde]

Apple designed NSEnumerator to be used that way, with the assignment taking place in the loop test. Personally, as concise as it is, I do feel like it encourages bad programming style.