Good code? Bad code? Horribly horribly ugly code?

[Chris Angelico]

I just wrote this into my MUD. It's beautifully elegant and
disgustingly ugly... at the same time.

else if (!catch
{throw(ip!=gethostbyname("hostname.domain.com")[1][0]);})
location="descriptive location";

This is in a long chain of "else if" checks on an IP address (most of
which are, for instance, prefix checks for "192.168"), and it looks up
the hostname to see if the IP matches it. In the event of DNS failure,
either gethostbyname will return 0, or it'll return something that
doesn't have the proper arrays to index - either of which will throw
an exception, causing the if to fail and the next else to be checked.
If the lookup succeeds and returns something different from the IP,
then the integer 1 will be thrown and the if will fail. If it does
match, though, the integer 0 will be thrown - which is perfectly legal
- and the "!catch {}" will be true, and the location will be set.

Would you, if you saw this code in a production environment, be scared
or delighted? I'm suspecting the former...

ChrisA

My only problem is only that throws are generally rather expensive
compared to an if. :)

> I just wrote this into my MUD. It's beautifully elegant and
> disgustingly ugly... at the same time.

> else if (!catch
> {throw(ip!=gethostbyname("hostname.domain.com")[1][0]);})
> location="descriptive location";

Well.  I would rewrite it using () instead of {}. :)

> !catch(throw(ip!=gethostbyname("hostname.domain.com")[1][0]))

Also, if hostname.domain.com happens to have multiple IP:s the results
could be somewhat random. :)

Anyway, for clarity I would probably avoid that construction.

--
Per

[Chris Angelico]

2012/1/16 Mirar @ Pike  importmöte för mailinglistan
<[hidden email]>:
> My only problem is only that throws are generally rather expensive
> compared to an if. :)
>

Heh! It's called once only, in response to a user command, and only on
one particular parameter, so I'm not too concerned about cost. But
yes, that is definitely a consideration.

ChrisA

[Chris Angelico]

2012/1/16 Per Hedbor () @ Pike (-) importmöte för mailinglistan
<[hidden email]>:
> Well.  I would rewrite it using () instead of {}. :)
>
>> !catch(throw(ip!=gethostbyname("hostname.domain.com")[1][0]))

Hmm, wasn't aware of that. Learn something new every day!

> Also, if hostname.domain.com happens to have multiple IP:s the results
> could be somewhat random. :)
>
> Anyway, for clarity I would probably avoid that construction.

I run the nameserver in question, and that hostname is specifically a
way to reference that IP, so that's safe.

How would you go about this? Ideally without a splash of temporary
variables, and self-contained.

ChrisA

[Martin Stjernholm]

Chris Angelico <[hidden email]> wrote:

> How would you go about this? Ideally without a splash of temporary
> variables, and self-contained.

I'd just write a function.

[Chris Angelico]

On Tue, Jan 17, 2012 at 12:09 AM, Martin Stjernholm <[hidden email]> wrote:
> Chris Angelico <[hidden email]> wrote:
>
>> How would you go about this? Ideally without a splash of temporary
>> variables, and self-contained.
>
> I'd just write a function.

Awww, but that doesn't have the thrill of one-liner composition!

I did think of breaking it out (all it'd need is a couple of
sequential statements, not even a function), but opted for the complex
one-liner - more for fun than for code quality. (See above for why
performance was simply a non-issue. And I can always comment it for
maintenance.)

But yeah, that would be the cleaner option.

ChrisA

Well. I would use the NetUtils module we wrote for ip related things.

There is always the lisp-ish way, I guess:

constant host = "www.opera.com";
constant ip = ({"195.189.143.147"});

Somewhat readable, perhaps:

Array.sum(map(Array.arrayify(gethostbyname(host)),equal,ip));

Minimize readability:

`+(@equal((gethostbyname(host)||({}))[*],ip));

--
Per

[Martin Stjernholm]

Chris Angelico <[hidden email]> wrote:

>> I'd just write a function.
>
> Awww, but that doesn't have the thrill of one-liner composition!
>
> I did think of breaking it out (all it'd need is a couple of
> sequential statements, not even a function), but opted for the complex
> one-liner - more for fun than for code quality. (See above for why
> performance was simply a non-issue. And I can always comment it for
> maintenance.)

Yes I know, it is a bit too easy, obvious, and boring. But oneliners
tend to be more fun to write than to read, and that particular one I
wouldn't like to see in the wild. The most important reason is that I'm
very _very_ skeptic to catches that catch everything without checking
what it is, or logging it somewhere.