Idiomatic Ruby #8

Closed
arronmabrey opened this Issue Mar 19, 2013 · 1 comment

Comments

Projects
None yet
2 participants
@arronmabrey
Contributor

arronmabrey commented Mar 19, 2013

https://github.com/segmentio/analytics-ruby/blob/master/lib/analytics-ruby/client.rb#L118-L124

Generally in ruby most developers don't do much explicit type checking. It's not a big deal but I had to crack open the source to see that it required a String. The error message wasn't even clear about that.
A better approach might be to call the method .to_s on the user_id and coerce it into a string inside that method.

The use case is a simple rails example.

u = User.find 53
Analytics.identify(user_id: u.id, traits:{email: u.email,twitter: u.twitter_name, username: u.username})
# => ArgumentError: Must supply a non-empty user_id

The id in that case is a number so it failed.

So it forces me to call the method like this with to_s on the u.id.

...
Analytics.identify(user_id: u.id.to_s,   ...)
# => true

@calvinfo calvinfo closed this in 6b06dad Mar 19, 2013

@calvinfo

This comment has been minimized.

Show comment
Hide comment
@calvinfo

calvinfo Mar 19, 2013

Member

Good call, I just updated it to use to_s internally. You're totally right - we shouldn't have missed it in the first place!

Member

calvinfo commented Mar 19, 2013

Good call, I just updated it to use to_s internally. You're totally right - we shouldn't have missed it in the first place!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment