[Swan-dev] style issue

[D. Hugh Redelmeier]

This is about a matter of taste.  I'd like to understand why others
don't agree with me.  I'm not just jumping in to "improve" the code.

I like compact code.  It means that more functionality can be seen at 
once.

That's one reason that I like C better than COBOL (there are many others).

But this can be taken to far: I find APL pretty hard to read.  Maybe it is 
just that I haven't had enough exposure to internalize the set of 
operators.

Anyway, I find this:

bool selector_subnet_in(const ip_selector *lhs, const ip_selector *rhs)
{
	ip_subnet lhs_subnet = selector_subnet(*lhs);
	ip_subnet rhs_subnet = selector_subnet(*rhs);
	return subnet_in_subnet(lhs_subnet, rhs_subnet);
}

less readable than:

bool selector_subnet_in(const ip_selector *lhs, const ip_selector *rhs)
{
	return subnet_in_subnet(selector_subnet(*lhs), selector_subnet(*rhs));
}

- shorter (fewer characters, fewer lines)

- fewer variables

- sometimes creating an intermediate variable helps the reader understand
  what a subexpression means.  I don't see that applies in these cases.

It may be that steppng through the first with gdb is more
comprehensible than stepping through the second.  But how often are we
stepping through code with gdb?


Here's another example:

bool selector_subnet_is_address(const ip_selector *selector, const
ip_address *address)
{
	if (address_is_unset(address) || selector_is_unset(selector)) {
		return false;
	}
        ip_subnet subnet = selector_subnet(*selector);
	return subnet_eq_address(subnet, *address);
}

This looks simpler to me:

bool selector_subnet_is_address(const ip_selector *selector, const
ip_address *address)
{
	return	!address_is_unset(address) &&
		!selector_is_unset(selector) &&
		subnet_eq_address(selector_subnet(*selector), *address);
}

- it is shorter

- there is no control-flow to analyze

- there's no "false" to understand

- a logical conjunction is easier to understand than a disjunction plus
  a conjunction created by control flow.

- a variable is eliminated

- most of all: you can "read" the meaning of the function directly
  rather than deriving it through the application of predicate
  transformers.

- one reason I use an algebraic language rather than assembly code is
  so that I can write things like this concisely.

What do you think?