Code Review Stack Exchange is a question and answer site for peer programmer code reviews. Join them; it only takes a minute:

Sign up
Here's how it works:
  1. Anybody can ask a question
  2. Anybody can answer
  3. The best answers are voted up and rise to the top

I'm working through some exercises to sharpen my Ruby skills. The following code is how I solved this particular question. I'd like to know what other developers think of this solution. I'm self taught, which means: I'm not getting any feedback as I learn, so I just want to make sure that I'm learning the right way to do things. And I'd like to get a gauge of what professional developers think of my code.

Ranking System
[1-terrible] [2-not good] [3-ok] [4-good] [5-very good]

  1. Is the answer code elegant? (score and feedback)
  2. What tips can you suggest to improve the answer?
  3. How's the readability of the code? (score and feedback)
  4. Can you suggest a simpler and alternative answer?


class Book
  def title

  def title=(title)
  special_words = %w(and in the of a an)
  formatted_title = []

    @title = title.split.each_with_index do |w,i|
      when i == 0
        formatted_title << w.capitalize

      when i > 0 && !special_words.include?(w)
        formatted_title << w.capitalize

      when special_words.include?(w)
        formatted_title << w

  @title = formatted_title.join(" ")


describe Book do

  before do
    @book =

  describe 'title' do
    it 'should capitalize the first letter' do
      @book.title = "inferno"
      @book.title.should == "Inferno"

    it 'should capitalize every word' do
      @book.title = "stuart little"
      @book.title.should == "Stuart Little"

    describe 'should capitalize every word except...' do
      describe 'articles' do
        specify 'the' do
          @book.title = "alexander the great"
          @book.title.should == "Alexander the Great"

        specify 'a' do
          @book.title = "to kill a mockingbird"
          @book.title.should == "To Kill a Mockingbird"

        specify 'an' do
          @book.title = "to eat an apple a day"
          @book.title.should == "To Eat an Apple a Day"

      specify 'conjunctions' do
        @book.title = "war and peace"
        @book.title.should == "War and Peace"

      specify 'prepositions' do
        @book.title = "love in the time of cholera"
        @book.title.should == "Love in the Time of Cholera"

    describe 'should always capitalize...' do
      specify 'I' do
        @book.title = "what i wish i knew when i was 20"
        @book.title.should == "What I Wish I Knew When I Was 20"

      specify 'the first word' do
        @book.title = "the man in the iron mask"
        @book.title.should == "The Man in the Iron Mask"
share|improve this question
i'd say specs -- 4, code -- 3. i'd answer more fully but I don't have anything to add to Phillip's solution, which I like a lot other than the word "titlieze" – Jonah Mar 4 '14 at 3:23
@Jonah Thanks for your input. I agree about "titlieze" – Fab Mar 4 '14 at 3:25
I use this tool for my blog to auto-capitalize titles, but if you look at the source code, you can see how they wrote it in javascript following the Chicago Manual of Style rules for title capitalization. – user57018 Oct 28 '14 at 12:05
up vote 11 down vote accepted

Your code is readable, but it doesn't feel very "rubyish" to me. I think that's due to needing to setup and track some extra variables and track the index of the word in question -- having to case it depending on the index, etc.

It also bugs me a little bit that title= is doing so much. This is how I would have written it:

class Book
  def title

  def title=(title)
    @title = titlieze(title)

  def titlieze(title)
    stop_words = %w(and in the of a an){|w| stop_words.include?(w) ? w : w.capitalize}.join(' ')


I used stop_words instead of special_words since that's a common naming scheme for search applications (ie. words you ignore when searching). I'd be tempted to move that into a constant or some other configuration, but it works fine for this.

Rspec output:

$ rspec -f d foo_spec.rb

    should capitalize the first letter
    should capitalize every word
    should capitalize every word except...
    should always capitalize...
      the first word

Finished in 0.00261 seconds
9 examples, 0 failures
share|improve this answer
It does? Did you change your test from when it was on stackoverflow? It's passing on my system using Ruby 2. – Philip Hallstrom Mar 3 '14 at 20:39
Hey thanks. Sorry about that, I made a mistake copying your code. It works well and is very nice. Thank you for your feedback. +1 – Fab Mar 3 '14 at 20:42

Your indentation is a bit inconsistent, making the code hard to read.

This short method

def title

… is commonly written using meta-programming.

The title= method could be improved by shortening it. There is too much reassignment going on: making an empty formatted_title, appending each word to it, then setting @title, but — just kidding! — we still have to re-join the words into a string! Better to do the job right the first time.

I also think that the code would be easier to understand by collapsing the three cases into two. Then you could easily state exactly when a word should be capitalized.

class Book
  attr_reader :title

  # Define this constant array just once
  @@SPECIAL_WORDS = %w(and in the of a an)

  def title=(title)
    @title = do |w,i|
        when i == 0 || !@@SPECIAL_WORDS.include?(w)
          # Capitalize the first word and all subsequent non-special words
    end.join(' ')

Consider leaving words that already have internal capitalization unchanged (e.g. "iPhone").

Also consider that you might already have a String.titleize() if you're using ActiveSupport.

share|improve this answer
Whats the reason for creating a class variable constant? Are you just assuming that the special words may need to available to other methods on the Book class? – Fab Mar 3 '14 at 20:50
Just to emphasize that it never changes. I don't feel strongly about that particular piece of advice. – 200_success Mar 3 '14 at 20:56
So you're saying a constant within the method itself may suffice? – Fab Mar 3 '14 at 20:58
Sure, that could work too. – 200_success Mar 3 '14 at 22:59

Though I am not a professional developer, I will venture one suggestion: learn how to use String methods and regexes to fullest advantage. Sometimes it is necessary to use split, partition, etc. to convert a string to an array of strings (possibly single-character strings), manipulate the array elements, then rejoin them into a string, but there is a lot you can do by working on the string directly. Here, for example, you can use String#gsub with a block:

str = "to eat an apple a day" 
sw = %w[and in the of a an]

str.capitalize.gsub( /\S+/ ) { |w| sw.include?(w) ? w : w.capitalize }
  #=> "To Eat an Apple a Day"

Edit: initially I had ...gsub( /\w+/ ).... I am grateful to @feed_me_code for the suggestion to use \S rather than \w.

share|improve this answer
+1 @Cary your suggestion: "learn how to use String methods and regexes to fullest advantage" is great. I wanted to add that instead of \w+ boundary.. Consider \S Any non-whitespace character. This takes into account strings with punctuation. Consider this: str = "the girl's voyage back to earth" Thanks again for your advice. – Fab Mar 6 '14 at 19:43
Good point, feed. Will edit. – Cary Swoveland Mar 7 '14 at 6:55

protected by Jamal Mar 7 '15 at 20:18

Thank you for your interest in this question. Because it has attracted low-quality or spam answers that had to be removed, posting an answer now requires 10 reputation on this site (the association bonus does not count).

Would you like to answer one of these unanswered questions instead?

Not the answer you're looking for? Browse other questions tagged or ask your own question.