2

I'm trying to get all the anagrams for one word with Ruby but my code doesn't work, I only get three results for the string 'ant'. Any help would be much appreciated.

class Anagram
    attr_reader :word
  def initialize(word)
    @word = word.downcase
  end

  def anagram_maker
    @word_bank = []
    index = @word.length
    minus_one = index - 1

    while (index * minus_one) != 0
      anagram = @word.split('').shuffle.join
      @word_bank << anagram
      index -= 1
    end
    @word_bank = @word_bank.uniq
  end

  def display
    anagram_maker
    if @word_bank.count > 1
      @word_bank.each do |anagram|
        puts anagram
      end
    else
      puts "Not enough letters for an anagram"
    end
  end

end

Not sure what else to try here.

toro2k
  • 19,020
  • 7
  • 64
  • 71
John
  • 443
  • 8
  • 19

2 Answers2

8

Your code is quite un-idiomatic Ruby.

Computing anagrams of a string is a matter of computing the permutations of the characters of the string, and Ruby makes this task quite easy. An example reproducing your Anagram class is:

class Anagram
  def initialize(word)
    @word = word
  end

  def display        
    # In Ruby 2.0 @word.chars will return an array, no need for `to_a`.
    @word.chars.to_a.permutation.map(&:join).uniq.each do |anagram|
      puts anagram
    end
  end
end

anagram = Anagram.new('ant')
anagram.display

# Output
# ant
# atn
# nat
# nta
# tan
# tna

To answer your question: you get only three anagrams because the while loop inside the anagram_maker method is executed three times (the length of the string). Moreover I guess that just shuffling characters is not the proper way of generating permutations, see "Algorithm to generate anagrams" for more information about implementing an anagram algorithm.

Community
  • 1
  • 1
toro2k
  • 19,020
  • 7
  • 64
  • 71
  • What does `map(&:join)` mean? – Zero Fiber Dec 11 '13 at 12:36
  • @SampritiPanda See [this question](http://stackoverflow.com/questions/1217088/what-does-ampersand-colon-pretzel-colon-mean-in-ruby). – toro2k Dec 11 '13 at 12:40
  • @Sampriti That will join the elements of the array (created from .map) back together – John Dec 11 '13 at 12:40
  • The permutations line gives a syntax error. It should be `@word.chars.to_a.permutation` – Zero Fiber Dec 11 '13 at 12:40
  • @toro2k thanks so much for the help, I don't think my brain is functioning properly today – John Dec 11 '13 at 12:41
  • @SampritiPanda Thanks, I've tested only on Ruby 2.0. – toro2k Dec 11 '13 at 12:43
  • 1
    I would note that it may be necessary to call .uniq I.E. `@word.chars.permutation.map(&:join).uniq` this way duplicate permutations are removed. – Sean Larkin Dec 11 '13 at 12:44
  • @SeanLarkin Yor're pretty right, updated the answer, thank you. – toro2k Dec 11 '13 at 12:50
  • `Array#permutations` does not exist as a method in Ruby 1.9.3 or 2.0.0. And I believe you don't need to call `uniq` until you start creating anagrams that are shorter than the given set of characters. – vgoff Dec 11 '13 at 13:37
  • 1
    @vgoff `Array#permutation` (no s) exists. The `uniq` call is needed when there are duplicate chars. – steenslag Dec 11 '13 at 14:43
  • 1
    Yes, I know, but as it was the code wouldn't work. It needed to be edited to work. And it was reported, and we were told "It works on 2.0.0" when in fact it did not. And as far as unique happening, depends on what you mean by unique, as it may be perfectly valid to have the same literal word repeated when you can create it via different uses of the same letter as long as that same letter happens more than once in the collection of words. (Think Scrabble). – vgoff Dec 11 '13 at 15:59
  • 3
    As the SO is obviously new to Ruby, you should have explained `@word.chars.to_a.permutation.map(&:join).uniq.each`. That would have made the answer much more useful to the SO and avoided many of the followup questions and answers that could have been anticipated. The best way of doing that, imo, is to break it down with an example, e.g., for `@word` equal to `"ant"`, `e = @word.chars => ["a", "n", "t"]`, `ee = e.permutation => #`, `eee = ee.map(&:join) => ["ant", "atn", "nat", "nta", "tan", "tna"]` and then explain `uniq` and `&:join`. – Cary Swoveland Dec 11 '13 at 17:27
2

Also if you wanted a revision of your own code so that you can learn in context of your own writing style you could try this:

class Anagram
    attr_reader :word, :combinations
  def initialize(word)
    @word = word.downcase
    @combinations = (1..@word.length).inject(:*)
  end

  def anagram_maker
    @word_bank = []
    index = @word.length
    minus_one = index - 1

    while @word_bank.uniq.count < @combinations
      anagram = @word.split('').shuffle.join
      @word_bank << anagram
    end
    @word_bank = @word_bank.uniq
  end

  def display
    anagram_maker
    if @word_bank.count > 1
      @word_bank.each do |anagram|
        puts anagram
      end
    else
      puts "Not enough letters for an anagram"
    end
  end
end

This is far less efficient than the aforementioned but relevant none-the-less.

Sean Larkin
  • 6,290
  • 1
  • 28
  • 43