array modified in the loop causes bug with the output ruby

Question

I have a code that places anagrams into an array of arrays. (which contain anagrams) but somewhere i made a bug and the first values do not output as arrays but just as strings

I am using the << operator to push one array into the other the code is not that complicated but i cannot find a bug

def combine_anagrams(words)
    indexes = []
    anagrams = []

    words.each{|word| 
    if(word.is_a? String )
        first_word = word.downcase.chars.sort.join
        words.each{|second_word| 
            if(second_word.is_a? String)
                if(first_word == second_word.downcase.chars.sort.join)
                    indexes << words.index(second_word)
                end
            end
        } 

        indexes.each{|index| anagrams << words[index] }
        words.reject!.with_index {|el, idx| indexes.include?(idx)}

        words << anagrams # i replaced words with an array all_anagrams
        indexes = []
        anagrams = []
    end
    }

    return words
end

puts combine_anagrams([ 'cars','for', 'potatoes', 'racs', 'four','scar', 'creams', 'scream'] ).inspect

outputs

["for", "four", ["cars", "racs", "scar"], ["potatoes"], ["creams", "scream"]]

if i switch the order of "cars" and "for" in the input i get

["cars", "racs", "scar", ["for"], ["potatoes"], ["four"], ["creams", "scream"]]

Whats going on here

Sorry for the messy code im just begging to learn ruby

I created an additional variable all_anagrams = [] to store the array of all anagrams when i output the array onto the sreen i get all the values except the "for" and "four" for some reason those never get send to all_anagrams probably because i shorten the array when i am in the loop and those values get skipped over? However i dont know how to deal with this problem.

the output of all_anagrams is

[["cars", "racs", "scar"], ["potatoes"], ["creams", "scream"]]

Your indentation is absolutely horrible; I can't follow your code. — lc.
– lc., Commented Oct 15, 2012 at 6:33
sorry let me fix that i am used to eclipse but i dont have eclipse on this because its a virtual drive of ubuntu. I dont know a good program for it that will do indentations :( — Xitcod13
– Xitcod13, Commented Oct 15, 2012 at 6:38
Try the tab key or the space bar. I know we're all lazy but there comes a point where readability eventually trumps laziness. — lc.
– lc., Commented Oct 15, 2012 at 6:40
You're modifying the words array as you iterate over it - not a good plan. You may be interested in group_by — Frederick Cheung
– Frederick Cheung, Commented Oct 15, 2012 at 7:13

saihgala · Accepted Answer · 2012-10-15 08:01:29Z

1

What you need is introduce a new array to store anagrams before you blank it, lets call it valid_anagrams. Right now you're pushing that in words. And as Fredrick pointed out you're modifying words while iterating over it. Its not good and to avoid that you keep a clone of words called words_clone and reject items from it instead. Following code should work -

def combine_anagrams(words)
    indexes, anagrams, valid_anagrams = [], [], []
    words_clone = words.clone # creating a clone of words

    words.each do |word|
            if(word.is_a? String )
                 first_word = word.downcase.chars.sort.join
                words.each do |second_word|
                        if(second_word.is_a? String)
                            if(first_word == second_word.downcase.chars.sort.join)
                                    indexes << words.index(second_word)
                            end
                        end
                end

                indexes.each{|index| anagrams << words[index] }

                # reject from words_cloned instead of words
                words_clone.reject!.with_index {|el, idx| indexes.include?(idx)}

                # insert anagrams into valid_anagrams array. In your code you inserted it in words array
                valid_anagrams << anagrams unless valid_anagrams.include?(anagrams)
                indexes, anagrams = [], []
            end
    end

    # return valid_anagrams array
    return valid_anagrams
end

answered Oct 15, 2012 at 8:01

saihgala

5,7943 gold badges37 silver badges31 bronze badges

Sign up to request clarification or add additional context in comments.

1 Comment

Xitcod13 Over a year ago

it works! awesome! and beautiful indentation may i add :P Im new to ruby so your syntax helps me learn. If you are given an array of strings you dont need the if(second_word.is_a? String) statements. I can take them out now and the code will still work for all cases??

Collectives™ on Stack Overflow

array modified in the loop causes bug with the output ruby

1 Answer 1

1 Comment

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

1 Comment

Your Answer

Sign up or log in

Post as a guest

Related