Warning: string literal in condition – how to best check whether a value is within a range

[Michael_Schwarze]

Hi,

I’d like to implement a guard clause to check whether a value is within a certain range but it gives me a warning: »string literal in condition«:

···

~~~
def billable?(activity)
   return false if activity[:project_nbr] == '900'..'999'
   # …
   true
end
~~~

I’ve looked this up ([1], [2], [3]) and can follow the examples given there and why

~~~
if input == "N" || "n"
~~~

has to be

~~~
if input == "N“ || or input "n"
~~~

But I can’t really get my head around what’s wrong with my above mentioned code.

Anyway, I’ve changed the guard clause to

~~~
return false if activity[:project_nbr] >= '900' and activity[:project_nbr] <= '999'
~~~

and it works, but are there any other approaches to check whether a value is within a certain range and what’s wrong with my first approach?

Many thanks for any ideas on this!

Cheers,
Michael

[1] http://stackoverflow.com/questions/20867709/warning-string-literal-in-condition
[2] http://stackoverflow.com/questions/10896248/ruby-warning-string-literal-in-condition
[3] http://stackoverflow.com/questions/21556673/what-does-string-literal-in-condition-mean

[Bryce_Kerley]

are there any other approaches to check whether a value is within a certain range and what’s wrong with my first approach?

~~~
def billable?(activity)
   return false if activity[:project_nbr] == '900'..'999'
   # …
   true
end
~~~

This is checking to see if `activity[:project_nbr]` is equal to the given range, which it presumably isn’t, since a `String` is not a `Range`.

~~~
if input == "N" || "n"
~~~

This evaluates the `input == “N”` half into a boolean first, and then ORs it with the string `”n”`, which is truthy, and as such the whole expression is always truthy.

~~~
return false if activity[:project_nbr] >= '900' and activity[:project_nbr] <= '999'
~~~

You can use the `Range#===` or “threequals” method to do the comparison:

("900".."999") === "123" #=> false
("900".."999") === "923" #=> true

Threequals is what `case` uses, and is generally treated as a specific “matching” operation. `Class#===` is true if the given object is a member of the class, `Regexp#===` is truthy if the given string is matched by the regular expression, and `Range#===` is true if the given object is an element of the range.

See Class: Range — Documentation for core (3.0.2)

···

On 8 Oct 2016, at 12:08, Michael Schwarze wrote:

[Robert_K1 (Robert K.)]

I’d like to implement a guard clause to check whether a value is within a certain range but it gives me a warning: »string literal in condition«:

~~~
def billable?(activity)
   return false if activity[:project_nbr] == '900'..'999'
   # …
   true
end
~~~

First of all you use the wrong test: that should read:

return false if ('900'..'999').include? activity[:project_nbr]
return false if ('900'..'999') === activity[:project_nbr]

String ranges are tricky because they can contain arbitrary many
elements and this is also reflected in the difference between the
range check and the two condition check:

irb(main):011:0> ('900'..'999').include? '9899'
=> false
irb(main):012:0> ('900'..'999').to_a.include? '9899'
=> false
irb(main):013:0> '900' <= '9899' && '999' >= '9899'
=> true

Why don't you just do an integer check?

return false if (900..999) === Integer(activity[:project_nbr])

I’ve looked this up ([1], [2], [3]) and can follow the examples given there and why

~~~
if input == "N" || "n"
~~~

has to be

~~~
if input == "N“ || or input "n"
~~~

That does not work - you have two consecutive binary operators.

But I can’t really get my head around what’s wrong with my above mentioned code.

Anyway, I’ve changed the guard clause to

~~~
return false if activity[:project_nbr] >= '900' and activity[:project_nbr] <= '999'
~~~

and it works, but are there any other approaches to check whether a value is within a certain range and what’s wrong with my first approach?

See above.

Kind regards

robert

···

On Sat, Oct 8, 2016 at 6:08 PM, Michael Schwarze <michael@schwarze-web.de> wrote:

--
[guy, jim, charlie].each {|him| remember.him do |as, often| as.you_can
- without end}
http://blog.rubybestpractices.com/

[Michael_Schwarze]

Hi Bryce,

Thank you!

…a String is not a Range.

Well, you are right; the warning directed me into the wrong direction, that’s why my search on StackOverflow hasn’t really helped me, either.

You can use the Range#=== or “threequals” method to do the comparison:

("900".."999") === "123" #=> false
("900".."999") === "923" #=> true

Ok, the threequals looks far more idiomatic, but it’s still a tricky one:

···

Am 08.10.2016 um 18:36 schrieb Bryce Kerley <bkerley@brycekerley.net>:

~~~
("900"..“999") === "923" #=> true
"923" === ("900"..“999“) #=> false
~~~

Anyway, „threequals“ directed me into the right direction [1]; thanks again!

Cheers,
Michael

[1] http://gilesbowkett.blogspot.de/2007/11/what-is-threequals.html

[Michael_Schwarze]

Hi Robert,

First of all you use the wrong test: that should read:

return false if ('900'..'999').include? activity[:project_nbr]

Ok, that's great; now I can see why the range has to be on the left side:

return false if ('900'..'999') === activity[:project_nbr]

Now it makes perfect sense to me why the threequals has to be this way round and not the other!

Why don't you just do an integer check?

return false if (900..999) === Integer(activity[:project_nbr])

Makes sense, although I prefer your first approach (.include?).

Many thanks for opening my eyes (again)!

Cheers,
Michael

···

Am 08.10.2016 um 18:38 schrieb Robert Klemme <shortcutter@googlemail.com>:

[Robert_K1 (Robert K.)]

Why don't you just do an integer check?

return false if (900..999) === Integer(activity[:project_nbr])

Makes sense, although I prefer your first approach (.include?).

The point is not whether to use === or #include? but that the test
uses integers!

Many thanks for opening my eyes (again)!

You're welcome!

Kind regards

robert

···

On Sat, Oct 8, 2016 at 11:24 PM, Michael Schwarze <michael@schwarze-web.de> wrote:

Am 08.10.2016 um 18:38 schrieb Robert Klemme <shortcutter@googlemail.com>:

--
[guy, jim, charlie].each {|him| remember.him do |as, often| as.you_can
- without end}
http://blog.rubybestpractices.com/

[Michael_Schwarze]

Hi Robert,

···

Am 09.10.2016 um 12:12 schrieb Robert Klemme <shortcutter@googlemail.com>:

The point is not whether to use === or #include? but that the test uses integers!

I think I haven’t really understood you, but as you’ve pointed this out again, I’ll give it a try:

As the business rule in this case states that projects with a number ranging from '900' to '999' are not billable, at first glance testing them accordingly made sense and testing them as integers felt a little bit awkward to me:

~~~
  ('900'..'999').include? '991' #=> true
  ('900'..’999').include? 99 #=> false
  ('900'..’999').include? 999 #=> false
  ('900'..'999').include? '99' #=> false
  ('900'..'999').include? '9919' #=> false
  ('900'..'999').include? '99 9' #=> false
  ('900'..'999').include? '999 9' #=> false
  ('900'..'999').include? ' 999' #=> false
  ('900'..'999').include? '9 9' #=> false
~~~

At second glance it’s not really clear how to handle ’99 9', ' 999', and '9 9', for instance. Testing them as strings will return them as not billable (#=>false) without further notice. ' 999' might actually be meant as '999' but it might be something different, too. Testing them as integers leads in this cases ('9 9', …) to an ArgumentError due to an invalid value which I can handle appropriately: at least I can log them now, for instance. Without this it might get really tricky to trace any differences in the calculation based on this tests.

Therefore, I’ve to admit that at second glance testing the range as integers is an improvement to my code and I’ll implement it accordingly.

Did I get your point here or am I still missing something?

Many thanks!

Regards,
Michael

[Robert_K1 (Robert K.)]

The point is not whether to use === or #include? but that the test uses integers!

I think I haven’t really understood you, but as you’ve pointed this out again, I’ll give it a try:

As the business rule in this case states that projects with a number ranging from '900' to '999' are not billable, at first glance testing them accordingly made sense and testing them as integers felt a little bit awkward to me:

Contrary to what you state you did not test as integers in the code below.

~~~
  ('900'..'999').include? '991' #=> true
  ('900'..’999').include? 99 #=> false
  ('900'..’999').include? 999 #=> false
  ('900'..'999').include? '99' #=> false
  ('900'..'999').include? '9919' #=> false
  ('900'..'999').include? '99 9' #=> false
  ('900'..'999').include? '999 9' #=> false
  ('900'..'999').include? ' 999' #=> false
  ('900'..'999').include? '9 9' #=> false
~~~

Please look at the example I gave.

At second glance it’s not really clear how to handle ’99 9', ' 999', and '9 9', for instance. Testing them as strings will return them as not billable (#=>false) without further notice. ' 999' might actually be meant as '999' but it might be something different, too. Testing them as integers leads in this cases ('9 9', …) to an ArgumentError due to an invalid value which I can handle appropriately: at least I can log them now, for instance. Without this it might get really tricky to trace any differences in the calculation based on this tests.

Right.

Therefore, I’ve to admit that at second glance testing the range as integers is an improvement to my code and I’ll implement it accordingly.

Did I get your point here or am I still missing something?

Yes. I think you would do yourself a favor if you read a bit more carefully.

Kind regards

robert

···

On Sun, Oct 9, 2016 at 5:48 PM, Michael Schwarze <michael@schwarze-web.de> wrote:

Am 09.10.2016 um 12:12 schrieb Robert Klemme <shortcutter@googlemail.com>:

--
[guy, jim, charlie].each {|him| remember.him do |as, often| as.you_can
- without end}
http://blog.rubybestpractices.com/