From: "Jesús Gabriel y Galán" Date: 2010-03-31T19:11:44+09:00 Subject: Re: RubyDictionary - First Try On Wed, Mar 31, 2010 at 11:55 AM, Max Schmidt wrote: > Hello and thank you for this fast widespread answer! > >>Your implementation allows to insert two pairs with the same key > This totally was my intention, because one word can be translated in > many different ways. Associative Arrays are called "Hashs" in Ruby, > aren't they? Yes, but the term Dictionary is a common technical term with that meaning, that's why I was asking... > > 1 - >>def initialize(entries=nil) >>    @entries = entries || []   # or Array.new if you prefer >>end > Nice idea, I never would consider a boolean operator as a expression, > which returns anything else than "true" or "false" > > 2 - >>def []=(word1, word2) > Don't you think that this definition is rather confusing, if Dictionary > is NOT an associative array? I have to admit that I am still not > familiarized with these method-like operators... Yes, you are right. That syntactic sugar makes more sense when it's an associative array, not in your case. > 3 - >>Why don't you allow one letter words? > Because strings with one letter aren't any words, they are characters. > My aim was to develop a dictionary where you can organize "real-life" > words "a" is a word :-) > >>if the string doesn't match you get a > >>irb(main):160:0> d.insert_words "a", "b" >>NoMethodError: undefined method `[]' for nil:NilClass >>  from (irb):94:in `is_word?' > > Ok, fixed it like this: > >>match = word.match(/[a-zA-Z]{2,15}/); >>if (match!=nil && match[0].length == word.length) then true > >>def self.is_word? word >>  word.match(/\A[:alpha:][:alpha:]+\z/) #you can remove the first >>[:alpha:] to allow 1-letter words >>end > > I wanted to limit the word's length to 15 as you can see in my regular > expression. Are the "\A" (start position) and "\z" (end position) > necessary? It's a way of saying that the full string should match your restrictions. It avoids having to check that the match is equal to the length of the original string: def self.is_word? word word.match(/\A[:alpha]{2,15}\z/) end should do what you want. > 5 - > >> this way, even if the arrays have different size, you store as much as >> you can. If you still want the check you can add it. > > Hmm, I want the length-check remain. > >> def insert_array(words1, words2) >>   words1.zip(words2).each {|first,second| insert_words(first,second) >> if first && second} >> end > > Why do you prefer to a implementation which produces a temporaly array? > > 6 - > > 7 - > >> This one doesn't make sense for the public interface of a dictionary. >> If you implement remove as above, you don't need it anymore. > > This was meant to be a time-saver. You can output the Dictionary by > typing "o/output", find the index of the entry you want to delete and > then apply it to the "r/remove" - command > > 8 - > > Very cool! > > 9 - > >> Why do you need the index as a return value? > for "remove" - I first searched for the word pair and if found I removed > the pair by calling remove_at(found_index) > >> @entries.find {|entry| entry == [word1,word2]} > I slowly realize that I will have to clearly go through the Array > methods this afternoon. > > 10 - > >>@entries.each_with_index {|entry, i| s << sprintf("%3d %15s | >> %15s\n", i, entry.word1 , entry.word2) > > This means it is actually never recommended to use >>@entries.length.times {|i| ... } > to iterate through an array? It's usually not recommended. If you really, really need the index each_with_index handles that. > > DictionaryEntry > > 11 - >> you might want to dup the words to > avoid aliasing > Do you mean to avoid exceptions when the words are frozen? If yes, I > would throw an Exception if either of the two words is frozen. No, what I mean is that the client can modify the String object after inserting it in the dictionary: d = Dictionary.new w1 = "hello" w2 = "world" d.insert_words(w1,w2) w1 << "something crazy which maybe should be in the dictionary at this point" The string referenced by w1 is modified after being inserted. With your solution, you store the same object, and so the dictionary is modified by this external action. The frozen check is just an optimization, because if the string is frozen it's not going to be modified by the client, and so you are safe just using it. > 12 - >>Enumerable doesn't provide the method [] > Ok, I was convinced that Enumerables behave like Arrays > >>I'm not sure you want to keep the equality comparison with an >>Enumerable > ... then I should replace the Enumberable with i.e. an Array like this? I don't know, depends on what your use case is. If it's just for convenience, I think you should drop it, because it's giving those objects a behaviour that they shouldn't have. >>case other >> when Array >>  @word1 == other[0] && @word2 == other[1] >> when DictionaryEntry >>  @word1 == other.word1 && @word2 == other.word2 >> else >>  false >>end Another idea: if want to see the dictionary as a set of things, a common thing is to include Enumerable and provide an each method, so that the clients of the dictionary can use the cool methods Enumerable provides. In your case: class Dictionary include Enumerable def each (&blk) @entries.each &blk end end that way people could do: d = Dictionary.new #populate it somehow d.find {|entry| entry.word1 == "hello"} and all the rest of the cool stuff Enumerable provides. Jesus.