Live data from Hacker News

Please, kill your darlings

blog.ikura.co

31–40 of 84 posts

Re: Please, kill your darlings

#31

Earlier quoted context omitted.

I agree. Someone's "darling" code might be a truly innovative or unique way of solving a problem, and providing enough commentary should help avoid the problems the author warns about.

Not to mention it may be a performance optimization.

Which is a good reason, if that optimization is needed.

Re: Please, kill your darlings

#32

I like that type of code, so yes, maybe it's a darling. It's easy to make it more readable by aligning the code with the dots, so that it becomes a pipeline: @sentence = @sentence.split(' ') .map!{|x| x = x[0..0].upcase I definitely prefer that to half a screen page of crappy imperative code, where people over time will add lots of side effects etc. Besides, the middle part is clearly a strawman because @sentence = @…

As pointed out below, they aren't equivalent.

Compare the output of both when run against "foo bar WIBBLE"

Re: Please, kill your darlings

#33
post #14

I'm curious to see how more experienced Rubyists on HN would write this. My stab: @sentence.split(' ').map(&:capitalize).join(' ') More terse and more descriptive (imo)

Yes, that is almost exactly how I would rewrite it. The original doesn't communicate its intent at all. It makes one itching to rewrite it but, it takes some time to take all the "smartness" into account.

The original line raises surprisingly many questions:

- The default for `split` is to split on whitespace. Is it the intent of the author to only split on spaces? (I guess so) What about tabs?

- Why is the author using #map! (with exclamation mark)? I have to admit this put me on the wrong foot for a while. My best guess right now is that the reason is speed; Mutating the array that split produced is (slightly) faster than creating a new one.

- Why is the capitalized string assigned to x again? I can think of no good reason at all. Am I missing something?

- Why is the author using x[0..0] instead of x[0] or x.first? I know why: The difference is that the latter two return nil if x is the empty string; but it's far from obvious and one could easily break the code by trying to improve this.

- Why are the parts of x concatenated with My variant would be this:

    @sentence = @sentence.split.map(&:capitalize).join(' ')
... and I would put it in a one-line method called titleize to better communicate my intent.

Re: Please, kill your darlings

#34
Great post. This is one of my biggest pet peeves with Ruby and languages like it; they encourage developers to "show off" by using the most esoteric features they can find (even better if these features use weird symbols).

Compared to a language like Python that has few neuroses, the same developer writes much less readable code.

The post may have been better if the author included a Python sample that does the same thing:

    long_string = 'ajix mxozl xoap'
    new_string = []
    for word in long_string.split(' '):
        new_string.append("{}{}".format(word[0].upper(), word[1:]))
    new_string = ''.join(new_string)
Admittedly there are a few annoying symbols in here, mostly due to the non-intuitive operation of the join function and the well-meaning but less-than-ideal string formatting syntax (which could've been avoided if we used conventional strong concatenation, and which PEP 0498 attempts to improve). Also admittedly, coders who want to show how smart they are would try to use a list comprehension to do this, which is slightly less readable and imo shouldn't be used over this format without a good reason. But it's still much easier to parse than the Ruby version because everything is explicitly spelled out in the loop, and you generally shouldn't have to consult the language docs to look up 4 different rarely-seen operators.

Remember, debugging is twice as hard as authoring, so if you write the most clever code possible, you are by definition not intelligent enough to debug it. ;)

I understand that people can't be expected to stick to that on their own, so they need languages that promote function, uniformity, and ease of use over showmanship.

The simplicity of the code is one of the main things I look for in interviews. If you could've done something with the conventional, simple language construct that doesn't require someone to go back and refer to the docs, even if it takes more lines, but instead you used the super-arcane construct to prove how well you know the language or to "get it all on one line", I'm going to look on that pretty dubiously. The last thing I want to deal with on my projects is the residue of someone's ego impacting our ability to read their code and get things done quickly and easily.

P.S., in Python, there are a couple of shortcut functions for capitalizing each word in a sentence: str.title() and str.capwords().

Re: Please, kill your darlings

#35

If it takes more cleverness to debug code than to write it, and one writes code that is at the limits of one's own cleverness, then clearly one will not be able to debug it.

Didn't know the original source of this, assumed it was an aphorism. Thanks to a comment above for pointing out that it came from Brian Kernighan.

Re: Please, kill your darlings

#36
I don't mind the implementation of this code, but 99% likely, it's in a method called `capitalize_sentence` or similar. If that is strewn through the middle of your code, it's probably breaking single-responsibility-principle.

Re: Please, kill your darlings

#37
post #14

I'm curious to see how more experienced Rubyists on HN would write this. My stab: @sentence.split(' ').map(&:capitalize).join(' ') More terse and more descriptive (imo)

Different behaviour. capitalize downcases the rest of the each sub-string, the original code did not.

Wow, I just wrote a lengthy reply but I completely missed this. That only adds adds to Ikura's argument though.

Re: Please, kill your darlings

#38

In this thread are literally 300 comments saying "Huh, I don't even Ruby, and I understood it", thereby (in my opinion) completely proving the point . It's exactly because people pull this sort of thing "Hey, it was really easy to understand for me, how about you?" that I have seen developers feel compelled to put clever oneliners in codebases. Clever oneliners that later end up causing problems for whatever unluckly…

Oooooor, its not a 'clever one-liner'. Its just a piece of code. Which, in its native environment e.g. Ruby is what you're expected to understand to be a journeyman of the trade. The code is not written for newbies. It never will be. That's why they're 'newbies' and not 'professionals'.

As I said, until it hits some kind of edge case. Again, I'm not trying very hard here, but this oneliner doesn't give the expected output if your sentence has a string like "æsir".

  irb> @sentence = "\u00e6sir are gods"
  => "æsir are gods"

  irb> @sentence = @sentence.split(' ').map!{|x| x = x[0..0].upcase  "æsir Are Gods"
(Expected output being "Æsir Are Gods")

If you want to understand why this is failing, the code I gave above will make it way simpler. Of course, viewpoints differ – you might claim that troubleshooting it is only a job for "professionals".

Re: Please, kill your darlings

#39
The number of people here defending long 'darlings' (or over-generalizing 'long one-liners'), is both shocking and dimsaying to me. Beyond basic functionality one of any (serious) programmer's _top_ priorities is to maximize the readability of their code for the next (unkown) programmer that touches the source.

We all have large monitors -often rotated 90degrees to portrait. Extra lines are not a bad thing (especially if it helps visually break things into multiple smaller steps), and more work on a single line does not == elegance.

Re: Please, kill your darlings

#40
post #33
post #14

I'm curious to see how more experienced Rubyists on HN would write this. My stab: @sentence.split(' ').map(&:capitalize).join(' ') More terse and more descriptive (imo)

Yes, that is almost exactly how I would rewrite it. The original doesn't communicate its intent at all. It makes one itching to rewrite it but, it takes some time to take all the "smartness" into account. The original line raises surprisingly many questions: - The default for `split` is to split on whitespace. Is it the intent of the author to only split on spaces? (I guess so) What about tabs? - Why is the author us…

One-liners are fine if you know you'll never need to extend the logic later on. Problem is, how often does that stay true?

For instance your 'titleize' method, in order to properly title case any string, ought to support a second option of words to ignore, such as "a, an, the, ...". If you wrote it as a one-liner at first, then you've got to go into your mapper and add conditionals if an ignore list is passed, and you've got more work than if you left it as a more expanded piece of logic.

Again, this is somewhat of a trivial example, but building on the original post's point of "how easily can I understand this later on" can also include "how easily can I extend this later on". Avoiding one-liner cleverness can help as a general principle.

Post reply on HN