Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

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. I appreciate all your feedback.

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?

My Ruby Code

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 = Book.new

  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 at 3:23
@Jonah Thanks for your input. I agree about "titlieze" –  feed_me_code Mar 4 at 3:25

3 Answers 3

up vote 8 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)
    title.capitalize.split.map{|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 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 –  feed_me_code Mar 3 at 20:42
How does it know to always capitalize the first word (regardless) if it is a stop word? –  feed_me_code Mar 3 at 20:44
Actually I see .. It's because you call it as: title.capitalize... –  feed_me_code Mar 3 at 20:54

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 = title.split.each_with_index.map 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? –  feed_me_code Mar 3 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 at 20:56
So you're saying a constant within the method itself may suffice? –  feed_me_code Mar 3 at 20:58
Sure, that could work too. –  200_success Mar 3 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. –  feed_me_code Mar 6 at 19:43
Good point, feed. Will edit. –  Cary Swoveland Mar 7 at 6:55

Your Answer


By posting your answer, you agree to the privacy policy and terms of service.

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