Capitalize all words in sentence string in Ruby

Question

I have created the following code:

def LetterCapitalize(str)
 str = str.split(" ").each {|word| word.capitalize!}
 str = str.join(" ")
 return str
end

This takes in a basic string and capitalizes the first letter of all of the words. It looks kind of awkward to reassign things to str twice. What is wrong with my style here, if anything at all?

Answer 1

• Your function name should be in snake_case, not PascalCase. (Thanks @rui.)
• It seems like it would make more sense to use map than each here.
• Ruby doesn't need explicit return.
• You might even want to extend the base string class.

Here's my rendition:

class String
 def capitalize_words
 this.split.map { |x| x.capitalize }.join(" ")
 # or: .map(&:capitalize)
 end
end
"this IS a Test".capitalize_words # => "This Is A Test"

• tokland: +1 I'd just add a note about what's wrong with the re-using of variables and that would be it. Also, methods_with_underscore is the idiomatic method naming.
• rui: - you should use snake_case. - please avoid extending core classes, it's not good practice and can get out of control easily. - split uses space as default delimiter string.split.map(&:capitalize).join(' ')
• Schism: @rui thanks, my ruby is a bit rusty. as for extending the base class, I'd argue that capitalize_words is descriptive and general enough, so per this SO question it seems okay... could you point me to a resource that explains why it's poor practice?
• RubberDuck: This is a decent read on the topic. devblog.avdi.org/2008/02/23/…
• rui: @Schism monkeypatches in core classes are available globally, any other gem can monkeypatch your method or use your (possibly different) version and stop working as expected (and you don't usually test external dependencies). Debugging that is painful. This is trivial example, but that's codereview so I think it's worth taking into consideration more general view on the questions here.

Answer 2

Schism's answer touches on the important points: use snake_case, implicit return, and use map rather than modifying the string instances in-place with a bang-method (!).

However, if the string contains many kinds of whitespace - linebreaks, tabs; not just " " - it may not work as intended.

You could consider using gsub in order to maintain any and all original whitespace:

def capitalize_words(string)
 string.gsub(/\S+/, &:capitalize)
end

This will capitalize words separated by any kind of whitespace, and preserve that whitespace in the resulting string.

The regular expression will match 1 or more consecutive non-whitespace characters (i.e. it'll match individual words), which are then passed to an implicit block that calls capitalize on the word, replacing it in the string.

E.g.

input = "here's a STRING with\n\ta newline aNd a tab character"
puts capitalize_words(input)

will print

Here's A String With
 A Newline And A Tab Character

(StackExchange's system seems to replace the tab character with 4 spaces, but run the code yourself, and it'll remain a \t)

As for monkey patching the String class... up to you. I'd consider it for something like this, but I wouldn't do it immediately. The functionality is generic enough that it'd make sense as an addition to String, but, IMO, the real question is whether its usage is wide-spread enough. If the functionality is only used in a few places, then don't start messing with basic classes.

• Cary Swoveland: Great answer! I wasn't aware you could write String#gsub with a block (that lends itself to that) that way, but it makes perfect sense. If a reader doesn't understand what Flambino means by an "implicit block", string.gsub(/\S+/, &:capitalize) is equivalent to string.gsub(/\S+/) { |str| str.capitalize } (in the same way that, say, arr.map(&:to_i) is equivalent to arr.map { |str| str.to_i }.
• Flambino: @CarySwoveland Thanks for helping clarify that! I honestly wasn't sure what to call the &:some_method shortcut. I'm sure there's a proper term for it that I've just forgotten. "Implicit block" seemed reasonable though, but it's a pretty unwieldy term. If you know a better word for it, let me know :)