Hacker Newsnew | past | comments | ask | show | jobs | submitlogin

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 << instead of with +? For speed? Is the author not aware of String#capitalize ?

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.


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.


> 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?

Write the simple version first, wait for the need to extend the logic arise naturally, factor out the function and extend.


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.


  titleize = ->(sentence) { sentence.split(' ').map!{|x| x = x[0..0].upcase << x[1..-1]}.join(' ') }

  titleize.call("my lovely lovely lovely horse")

  ["a clause", "another clause"].map(&titleize)
:'


pretty experienced rubyist here, would use your version instead of what's in the blog unless benchmarking indicated a penalty that was going to hurt in some egregious way.


Man, my pet hate with Ruby is all the weird extra punctuation characters.

Here's the Perl version:

    $_ = join ' ', map ucfirst, split / /;




Guidelines | FAQ | Lists | API | Security | Legal | Apply to YC | Contact

Search: