From: Robert Klemme <shortcutter@...>
Date: 2011-09-06T20:51:20+09:00
Subject: Re: Conditional expression and return

On Tue, Sep 6, 2011 at 11:23 AM, Stefano Crocco <stefano.crocco@alice.it> wrote:
> On Tuesday 06 September 2011 18:11:47 Philipp Altmann wrote:
>> Hi guys,
>>
>> this is my Code:
>>
>>   def getRegex(string)
>>     if @HashMapWithAllRegex.has_key?(string) ?
>> (@HashMapWithAllRegex.fetch(string) : raise(ArgumentError, "This regex
>> is not specified")
>>     end
>>   end
>>
>> Error Message:
>>
>> SyntaxError:
>> /Users/Philipp/Documents/projects/objcdoc/src/RegexProvider.rb:19:
>> syntax error, unexpected tIVAR
>>
>>     if @HashMapWithAllRegex.has_key?(string) ? return
>> @HashMapWithAllRegex.fetch(string : raise(ArgumentError, "This regex is
>> not specified")
>>                                                                          ^
>>   require at org/jruby/RubyKernel.java:1038
>>    (root) at
>> /Users/Philipp/Documents/projects/objcdoc/src/RegexProvider.rb:6
>>   require at org/jruby/RubyKernel.java:1038
>>    (root) at /Users/Philipp/Documents/projects/objcdoc/src/Main.rb:1
>>
>>
>> Why will this don't work? I think in Java will this construct work :-).
>
> Well, ruby is not java. At any rate, why do you use the ternary operator
> together with an if? You only need one of the two:
>
> def getRegexp(string)
>  if @HashMapWithAllRegex.has_key?(string)
>    @HashMapWithAllRegex.fetch(string)
>  else raise(ArgumentError, "This regex is not specified")
>  end
> end
>
> or
>
> def getRegexp(string)
>  # The following should be on one line
>  @HashMapWithAllRegex.has_key?(string) ? @HashMapWithAllRegex.fetch(string) :
> raise(ArgumentError, "This regex is not specified")
> end
>
> I'd very much prefer the former. Better yet (supposing that
> @HashMapWithAllRegex is a hash), you can do:
>
> def getRegexp(string)
>  @HashMapWithAllRegex.fetch(string) do |k|
>    raise(ArgumentError, "This regex is not specified")
>  end
> end

Also, although "getRegexp" is idiomatic in Java, in Ruby this would
simply be "regexp".  We also prefer lowercase variable names with
underscores and we typically also do not encode the type in the
variable name.  And it is a good idea to include the key in the error
message so you know what key was actually missing.  So, this is how I
would do a more Rubyesque version

def regexp(str)
  @regexps.fetch str do
    raise ArgumentError, "No regexp for key %p" % str
  end
end

or even

def regexp(str)
  @regexps[str] or raise ArgumentError, "No regexp for key %p" % str
end

You can also incorporate that into the Hash and make the getter merely
a delegating method:

def initialize
  @regexps = Hash.new {|h,str| raise ArgumentError, "No regexp for key
%p" % str}
end

def regexp(str)
  @regexps[str]
end

Kind regards

robert

-- 
remember.guy do |as, often| as.you_can - without end
http://blog.rubybestpractices.com/