Student J
Student J

Reputation: 203

Delete from where?

Below I have written a piece of code that's supposed to count the number of occurrences of characters in a string and display the result in a hash. My idea was to get rid of that letter in the string right after I've counted it so that we don't put it into the hash more than once.

My original code was this:

def letter_count_spec(str)
letter_count = Hash.new #create hash
letter = str.split('') #separate letters
letter.each{ |e|
    if( /[A-Za-z0-9]/.match(e)  )
        occurances = letter.count{ |x| x==e}
        letter_count[e] = occurances
        letter.delete(e) #delete letter 

    end
}        

return letter_count
end

letter_count_spec("cat")

result: => {"c"=>1, "t"=>1}

I lose "a"!

So I tried this:

def letter_count_spec(str)
letter_count = Hash.new #create hash
letter = str.split('') #separate letters
letter.each{ |e|
    if( /[A-Za-z0-9]/.match(e)  )
        occurances = letter.count{ |x| x==e}
        letter_count[e] = occurances
    end
}        
letter.each{ |e|
    letter.delete(e) #delete letter
}
    return letter_count
end

letter_count_spec("cat")

result => {"a"=>1, "c"=>1, "t"=>1}

Why would I need to go through the array again to delete?

Upvotes: 0

Views: 65

Answers (1)

Arie Xiao
Arie Xiao

Reputation: 14082

The modification of collection during iteration may cause problems, which is stated in the comment.

The word counting algorithm normally involves a hash to keep track of the word count, and an iterator to go through the content. You don't need to modify the original collection. This is an O(n) solution, given that the hash has O(1) complexity on update in general case. However, the count and delete approach in your post has O(n^2) complexity (if it works).

def letter_count_spec(str)
  letter_count = Hash.new(0) # create hash, and use 0 as the default value
  letter = str.split('')     # separate letters
  letter.each do |e|
    if /[A-Za-z0-9]/.match(e)
      letter_count[e] += 1   # increment count
    end
  end
  letter_count
end

BTW, in Ruby, it's a convention to use do ... end for multiple lines block, unless {} is required in some circumstances.

Upvotes: 1

Related Questions