Hacker News Reader

Hacker News

Comment by jsaxton86 | Hacker News Reader

Parent Full thread

jsaxton86

A much simpler solution is to comment your code:

# Invoke curl in silent mode (-s), pipe the output to grep

# and use an extended regex (-E) to only show the resulting

# digits (-o: only print matching text, not the whole line):

curl -s checkip.dyndns.org | grep -Eo '[0-9\.]+'

View on HN

The comment is needless noise that the reader must check for correctness and then determine whether it or the code is the intended behaviour when they differ. The same is unfortunately true of many comments.

BTW, you've probably a bug in your regexp because you like to needlessly and wrongly escape a dot without knowing the regexp syntax you're using. Assuming you don't intend

    $ grep -Eo '[\.]' <<<\\
    \
    $

that is. :-)

(curl's -s should be accompanied by -S IMHO, and it's brain-damaged in not having that behaviour under the one option.)

I don't like commenting the how and what; that's what the code is for. Comments are for "why".

How and what are good to include when describing the function, but not for specific statements of code.

I agree with ralph that these comments are annoying because I have to waste time reading them (what if they contain something important?) but they offer me nothing (I already know these flags).

If you still prefer putting these hints in comments, I would rephrase your comments in a way that they are easily identifiable as useless, so I can stop reading them right away. For example:

# -s flag: puts curl in silent mode.

# -E flag: puts grep in extended regex mode.

# -o flag: prints only matching text.

curl -s thingy.thing | grep -Eo '[0-9.]+'

An additional win: comments are slightly more future proof. When the comments inevitably become outdated, you'll now see:

# -E flag: puts grep in extendex regex mode.

curl -s thingy.thing | egrep -o '[0-9.]+'

which is better because the comment is now merely irrelevant instead of actively wrong.

Or even better:

  # Get my IP address by checking dyndns.org
  curl -s checkip.dyndns.org | grep -Eo '[0-9\.]+'

When reading the script, you want to know what it does, not what each character in it does.

Using long flags actually hinders that by adding clutter; you have to parse a much longer line to know what it does. Adding a functional commend and then using the clean short flags is IMHO a much better way.

Or, even better:

  # Get my IP address by checking ifconfig.me
  curl -s ifconfig.me

The less work you have to do, the better :)

NOTE: If you visit ifconfig.me with your browser you get an html page full of text, but with the curl useragent it just returns the ip-address

That also introduces a dependency on ifconfig.me.

If your script contains four or five such 'less work you have to do', I bet it breaks within a year.

All of the above scripts had external dependencies.

Obviously that's a consideration, but this script has removed a dependency on checkip.dyndns.org. You could make the case that that's more likely to stay up, which is a valid decision.

Alternatively, you could create a "whereami" that returns the ip, and use that in your scripts. If ifconfig.me goes down for good, you have to change the url in one place. I'm sure you'll cope.

Reply on news.ycombinator.com