match? returns nil not boolean

[sposin]

Using browser 2.1.0

pry(#<#<Class:0x007f91633768a0>>)> browser
=> #<Browser::Edge:0x007f915c3bbb00 @accept_language=[#<Browser::AcceptLanguage:0x007f915c3bb600 @part="en-US", @quality=1.0>, #<Browser::AcceptLanguage:0x007f915c3bb470 @part="en;q=0.5", @quality=0.5>], @ua="Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/46.0.2486.0 Safari/537.36 Edge/13.10586">
pry(#<#<Class:0x007f91633768a0>>)> browser.safari?
=> false

pry(#<#<Class:0x007f91633768a0>>)> browser
=> #<Browser::InternetExplorer:0x007f9165c94250 @accept_language=[#<Browser::AcceptLanguage:0x007f9162f7bf98 @part="en-US", @quality=1.0>, #<Browser::AcceptLanguage:0x007f9162f7bf70 @part="en;q=0.5", @quality=0.5>], @ua="Mozilla/5.0 (Windows NT 10.0; WOW64; Trident/7.0; rv:11.0) like Gecko">
pry(#<#<Class:0x007f91633768a0>>)> browser.safari?
=> nil

When the browser is InternetExplorer a check for safari? returns nil instead of false. It seems to work fine with Chrome and Edge.

Question, why on the various .match? methods do you use regex which returns nil rather than .include?

[fnando]

So, predicate methods returning nil values were mentioned before (like #245). Ruby doesn't really have an opinion on this. The community does.

Since nil is a falsy value just like false itself, I don't think we should coerce values. Just don't type check on your end and you should be fine. If you do need boolean values, you can always !!value on your side too.

About using String#match? vs String#include?, can you point me to places where we can replace the former with the latter? I'll do some benchmarks to see if it's faster, and if so I can change all references.

[sshaw]

If you do need boolean values, you can always !!value on your side too.

It's not so much about "needing" boolean values as it is a consistent API:

rails [3.2.21] (rails32-test-app)$ Browser.new("").device.public_methods(false).select { |n| n.to_s.end_with?("?") }.map { |n| p "#{n}: #{b.device.public_send(n).inspect}" }
"tablet?: false"
"mobile?: nil"
"ipad?: false"
"unknown?: true"
"ipod_touch?: false"
"ipod?: false"
"iphone?: false"
"ps3?: false"
"playstation3?: false"
"ps4?: false"
"playstation4?: false"
"psp?: false"
"playstation_vita?: false"
"vita?: false"
"psp_vita?: false"
"kindle?: false"
"kindle_fire?: false"
"nintendo_wii?: false"
"wii?: false"
"nintendo_wiiu?: false"
"wiiu?: false"
"blackberry_playbook?: false"
"playbook?: false"
"surface?: false"
"tv?: false"
"silk?: nil"
"xbox?: nil"
"xbox_360?: false"
"xbox_one?: false"
"playstation?: false"
"nintendo?: false"
"console?: false"

In this case 91% of the methods return true or false and the other 9% return true or nil.

Happy bug hunting! 🐛 🐞 :neckbeard:

[sshaw]

P.S. You should update the docs or at least define your interpretation of Boolean.

[LeEnno]

This just bit me. It's unexpected behavior. The downsides have been pointed out and I struggle to see any upside. I'd appreciate ensuring the API always returns a bool for all those ? methods.