Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Question about if a = b.first snippet in README #1469

Closed
DeeDeeG opened this issue Oct 13, 2020 · 2 comments
Closed

Question about if a = b.first snippet in README #1469

DeeDeeG opened this issue Oct 13, 2020 · 2 comments

Comments

@DeeDeeG
Copy link

DeeDeeG commented Oct 13, 2020

Hello,

I'm maintaining a project that uses geocoder, and we've apparently copy-pasted a snippet from README.md some time ago. I'm also a bit of a beginner with Ruby, admittedly. The snippet doesn't make a lot of sense to me, and I'm wondering if this is intentional, or there's a special purpose to the code pattern? Any explanation you can give is appreciated.

in README.md there is the following snippet (https://github.com/alexreisner/geocoder/tree/v1.6.4#custom-result-handling)

reverse_geocoded_by :latitude, :longitude do |obj,results|
  if geo = results.first
    obj.city    = geo.city
    obj.zipcode = geo.postal_code
    obj.country = geo.country_code
  end
end
after_validation :reverse_geocode

Particularly this line is confusing me:

  if geo = results.first

As I understand it, this attempts to assign results.first to the (potentially newly created) geo object. And if this works, proceeds to do the content of the if/end block.

My questions are:

  • Is this meant to be a double-equals "equality check"? (if a = b --> if a == b?)
    • That would make more intuitive sense to me than what appears to be an "attempt to assign, and then check that the assignment worked" sort of pattern.
  • Why attempt to assign to geo and do an if statement on the same line? This happens to fail the default Rubocop linting.
  • Is this done for any special reason other than being more compact than assigning then checking if geo is non-empty?
    • This seems better: geo = results.first followed by (separately, on the next line) if geo # [then do stuff]
  • I am a beginner with Ruby, but I'm alarmed that, in irb, assigning from an "undefined local variable or method" gives a NameError. The if statement here is either true or there is a program error, right?
    • Why check with an if statement at all, if the alternative situation isn't skipping the content of the if/end block, but rather erroring out of the program?

I may be deeply misunderstanding this, but a project I work on has this snippet copy-pasted in, and we're trying to enable Rubocop. This snippet has me scratching my head a bit.

Sorry to take your time, but any explanation would be much appreciated as I try to figure out what this code is doing. Thanks for considering.

Best regards,

- DeeDeeG

P.S. this line appears to date all the way back to April 2011: 54329e5 (exact line) Not sure if that context helps, but I thought I mention that anyway.

DeeDeeG referenced this issue in RefugeRestrooms/refugerestrooms Oct 13, 2020
The condition was actually supposed to be `if geo = results.first`,
because that's not always obvious the intention and fails lint, I'm just
doing a truthy check for it which should be the same.
@alexreisner
Copy link
Owner

Interesting question. As far as I know, this is standard Ruby style. I've been using it for more than a decade. Possibly I've gotten old and my style has become dated.

The statement will return false if results.first is nil (if there's a program error it won't return at all, an exception will be raised). The line says: if you can assign a non-nil (truthy) value to geo, then execute the code that follows. One reason I like this syntax is it also denotes a block within which the geo variable is relevant. If I were going to use geo outside the if block, I'd probably do the assignment separately, prior to the if statement.

Put another way, the line in question checks whether results.firstcontains a result, and then uses an abbreviation (geo) while extracting data from that result, rather than repeating results.first.

Does that help?

@DeeDeeG
Copy link
Author

DeeDeeG commented Oct 17, 2020

Hi @alexreisner, thanks for the answer.

Yes, that does help.

It was also pointed out/explained to me in another thread that results is "defined by the parameter to the block" (do |obj,results|). So results won't be undefined, and attempting the assignment geo = results.first will not, in and of itself, trigger an error. (Unless something else pretty weird is happening at the same time to make the app not behave like it should, which is beyond the question I meant to ask; I only meant to ask about the "weird case" of results being undefined, and I can now see why that won't happen under normal circumstances.)

Between your answer and the comment I just linked, I'm able to see why the snippet works. Thanks again for taking the time.

(Whether to follow Rubocop in this case, I'll just keep that as being a decision over at the repo I'm helping to maintain.)

@DeeDeeG DeeDeeG closed this as completed Oct 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants