Code suggestions by Resharper making code less readable?

Question

While trying to get to all green, i got the following suggestion by Resharper.

Original code:

    static public string ToNonNullString(this XmlAttribute attr)     {         if (attr != null)             return attr.Value;         else             return string.Empty;     } 

Suggestion: remove redundant 'else' resulting in following:

    static public string ToNonNullString(this XmlAttribute attr)     {         if (attr != null)             return attr.Value;         return string.Empty;     } 

To me, the suggested version seems less readable than the original. Does Resharper suggestion reflect the definition of good maintainable code?

Answer 1

Technically Resharper is correct in that the "else" is unnecessary, I prefer the former version though as the intent is more obvious.

Having said that, I'd rather go with:

return attr != null ? attr.Value : string.Empty; 

Answer 2

Ah, code aesthetics. Holy war time. (ducks)

I'd go with either a ?: expression:

return attr != null ? attr.Value : String.Empty 

or invert the if and remove the line break to produce a guard clause:

if (attr == null) return String.Empty;  return attr.Value;