Style follows semantics

re: Style follows semantics

Not Oliver — Wed, 03 Nov 2010 14:22:27 GMT

Oliver: "I used to think "shorter is always better", but using breakpoints in VS made me change my mind."

I still think that in general "shorter is always better" (at least for simplicity in terms of semantic elements, not characters). Using breakpoints in VS made me convinced that VS doesn't do breakpoints very well. :-)

re: Style follows semantics

Pete Wilson — Tue, 06 Apr 2010 22:35:00 GMT

Personally I don't like seeing tests or assignments of literal true/false values when it can be avoided.

I'd probably do

bool success = NetworkAvailable();

if (success)

success = LogUserOn();

re: Style follows semantics

Bent Rasmussen — Mon, 08 Mar 2010 09:41:49 GMT

I observed a colleague doing a side-effecting expression a week or so ago and tried to explain the command-query separation principle and how he could make more explicit that there was a side-effect by not chaining side-effecting calls together in an expression as if the expression really was pure. It was a pretty tough sell...

re: Style follows semantics

RobS — Wed, 17 Feb 2010 19:34:27 GMT

C# is a high-level language and should be about readability rather than brevity.

Conversely, being overly verbose is just as bad a writing cryptic code.

In this case, virtually nothing is gained by combining these lines and you lose a little bit of readability.

In these cases, I would always keep them as separate cases or, as mentioned, assign the results to separate variables then compare in an IF statement

BTW I like the variable names in helping to make self-documenting code. When lines get too cryptic, you tend to lose that too.

re: Style follows semantics

Allon Guralnek — Tue, 16 Feb 2010 19:02:04 GMT

Kris really nailed it in my opinion, but I don't agree that both approaches make the original question pointless, rather that they make the LogUserOn example pointless.

re: Style follows semantics

rousso — Tue, 16 Feb 2010 09:43:04 GMT

I always prefer the shortest way to express something in code. So I tend to prefer skipping the control flow statements if I can do the exact same thing with a direct assignment.

Nevertheless I see other semantic issues in the examples you are presenting here, which would make the argument quite different had they been addressed:

I would implement NetworkAvailable() method most likely as a NetworkAvailable property (because availability is a property of a network that indicates its status). I also would test the property inside LogOnUser() method which would return false if network was unavailable or even better it should throw an exception (because lack of network availability is an exceptional situation).

To be more precise if I needed to express these things in code I would structure the whole thing to end up looking like:

if (!Network.IsAvailable)

throw new Network.UnavailableException();

return User.Logon();

or (if i didn't want to throw an exception)

return Network.IsAvailable ? User.TryLogOn() : false;

which is shorter than the control flow statement but still expresses the notion that attempting a login while network is unavailable is pointless. Also naming the LogOn method as TryLogOn() denotes that the method is not going to throw an exception.

So bottom line i would go for the shortest expressive statement foobar = foo() && bar(); but I would also take under consideration the semantics of foo() and bar() and and go for the shortest && least cryptic code by utilizing programming constructs that suite best the nature of the expressed elements.

re: Style follows semantics

Filini — Thu, 04 Feb 2010 14:23:53 GMT

I tend to agree with the idea that communicating the semantics of the program fragment is important, and trying to "compress" 2 lines into 1, just because you prefer that style, risks to hide the flow you are writing.

If we talk about the "real-word" scenario of login, then I suppose the requirements of that code might be something like: "create a method which logs a user in the system, if the network is available, and returns the status of the login".

What if some other action needs to be taken, when the connection is not available?

Let's change the requirements to: "create a method which logs a user in the system, if the network is available. If the network is not available, check your client for a cached profile, to work off-line. the method returns the status of the login (real or cached)".

Then our code would become:

bool result;

if( NetworkAvailable() )

result = LogInUser();

else

result = ExistsCachedProfile();

How would you change it?

bool result = ( NetworkAvailable() ) ? LogInUser() : ExistsCachedProfile() ;

bool result = ( NetworkAvailable() && LogInUser() ) || ExistsCachedProfile() ;

For me, the second "style" is not so nice/readable anymore.

It all depends on what your methods do (check a value / modify the system), as Eric states in the first paragraph. The two fragments:

bool result = NetworkAvailable() && LogInUser();

bool result = NetworkAvailable() && IsLoggedUser();

are the same code fragment, but they clearly perform different algorithms (the first one modifies the system). This might become misleading, for the code maintenance.

re: Style follows semantics

Kris Vandermotten — Wed, 03 Feb 2010 11:33:16 GMT

I mostly agree with James Hart. A method should do what its name implies, and return its result (if any). If it can't do what its name implies, it should report it as an exception.

So that leaves two options (for this example):

1) LonOnUser(...) always returns an IPrincipal. You can call .Identy.IsAuthenticated to test whether the principal was authenticated.

2) LogOnUser() returns an IPrincipal where .Identity.IsAuthenticated is always thrue, and throws if it can't do that.

Note that the method would need a rename in both cases.

Both approaches make the original question pointless.

Summarry: a method that returns a boolean is a function meant to compute the boolean. A method meant to do something else ("called for its side effect") should not return a boolean to indicate success or failure.

(The only exception to this is bool methods with out parameters named TryX, such as TryParse methods.) Consequently, if functions return boolean, they can be combined with the && operator with no maintainability problem at all.

re: Style follows semantics

Wes — Wed, 03 Feb 2010 07:10:20 GMT

Upon reading this I immediately searched for CQS in the comments and am glad to see it show up. Seeing as the state of being "logged in" to a system is something that can change and is what we are really modeling here, it totally makes sense to call a logon procedure and then check if the login was sucessful via a query routine.

CQS leads to reuse as I imagine we wouldn't want to attempt logging in a second time just to check if the user is authenticated somewhere else. Also, I think it would be beneficial to know if a user is no longer logged on without logging them in, so we would have to add another query routine anyways, so we might as well make LogOn void to avoid duplication.

I guess what we come back to the semantics of separating commands from queries, in which case the separated semantics lead to a more _readable_ style(syntax) where Foo() && Bar() can always be compacted to one line, thanks to CQS.

re: Style follows semantics

nog — Wed, 03 Feb 2010 06:04:19 GMT

In terms of code readability a method named LogUserOn() shouldn't return a bool.