6
\$\begingroup\$

I'm teaching myself Ruby by rewriting PHP solutions, such as this one. The program should match words in a sentence with a dictionary of synonyms and then make combinations with them.

dict = {
    'hello' => ['hey', 'what\'s up'],
    'name' => ['nomenclature', 'epithet']
}

str = 'hello, my name is'

class SentenceMaker
    def initialize str, dict
        @thesaurus = []
        @template = str.gsub(/\w+/) {|match|
            if (dict[match])
                @thesaurus.push dict[match]
                '%%%d$s' % @thesaurus.length
            else
                match
            end
        }
    end

    def each
        @thesaurus.first.product(*@thesaurus.slice(1..-1)).each {|repl|
            yield @template % repl
        }
    end
end

SentenceMaker.new(str, dict).each {|sentence|
    puts sentence
}

Example output

hey, my nomenclature is
hey, my epithet is
what's up, my nomenclature is
what's up, my epithet is

Does this make enough use of the language expressions, or did I miss a few things?

\$\endgroup\$
5
\$\begingroup\$

Overall, it's pretty good. My comments are:

  • Use do/end for multiline blocks.
  • Typical Ruby style is a 2-space indent.
  • I think @thesaurus.drop(1) looks a bit better than @thesaurus.slice(1..-1)
  • match is not a good local variable name, word is more descriptive
  • Template is too complicated: you can just use multiple %s placeholders with an array of Strings.

I'm not commenting too much on the algorithm, since you weren't asking, but it seems a bit complicated. I'd be tempted to try simplifying it a bit.

Here's your code with the above changes applied:

dict = {
  'hello' => ['hey', 'what\'s up'],
  'name' => ['nomenclature', 'epithet'],
}

str = 'hello, my name is'

class SentenceMaker
  def initialize str, dict
    @thesaurus = []
    @template = str.gsub(/\w+/) do |word|
      if (dict[word])
        @thesaurus.push dict[word]
        '%s'
      else
        word
      end
    end
  end

  def each
    @thesaurus.first.product(*@thesaurus.drop(1)).each do |repl|
      yield @template % repl
    end
  end
end

SentenceMaker.new(str, dict).each {|sentence|
  puts sentence
}
\$\endgroup\$
  • \$\begingroup\$ Good point about the template; it was a left-over artefact from the previous simplification round :) out of curiosity, what's better about do .. end versus braces? \$\endgroup\$ – Jack Jul 19 '15 at 6:42
  • \$\begingroup\$ Also, I would be interested to see what you would do to the algorithm :) \$\endgroup\$ – Jack Jul 19 '15 at 7:32
  • 1
    \$\begingroup\$ @Jack It is just a convention, curly braces for single lines, do end for multilines blocks. \$\endgroup\$ – Caridorc Jul 19 '15 at 20:39
  • 1
    \$\begingroup\$ I would also add to use double quotes when your string has a single quote in it to avoid needing to escape the apostrophe/single quote. Also I would organize the class at the top, the running code at the bottom. (the local variable assignments right above the SentenceMaker.new line.) Keeps things organized in a more logical way, rather than splitting what is running with a class definition in the middle. \$\endgroup\$ – vgoff Jul 23 '15 at 22:18

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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