Nested trial and error in if-else condition

Question

Using Rails 3.2, I have the following:

g = Geocoder.search(address)
# if no address found, geocode with latitude and longitude
if g.empty?
 g = Geocoder.search(latitude, longitude)
 if g.empty?
 log.info "Geocode failed for #{latitude}, #{longitude}"
 end
else
 ---save---
end

If the first geocoding with address fails, it will try latitude and longitude. If still fails, then it logs error, else it will save.

Is this a good way to write this code?

Answer 1

Some notes:

• g: use meaningful names.
• Don't mix code with parentheses and without.
• Check the pattern Object#presence.
• # if no address found, ...: declarative code makes this kind of comments unnecessary.

I'd write:

results = Geocoder.search(address).presence || Geocoder.search(latitude, longitude)
if results.present?
 # save
else
 log.info("Geocode failed for addr='#{search}' and coords=#{latitude}/#{longitude}")
end

Answer 2

Nesting your ifs is unnecessary, and flatting them will look better:

g = Geocoder.search(address)
# if no address found, geocode with latitude and longitude
if g.empty?
 g = Geocoder.search(latitude, longitude)
end
if g.empty?
 log.info "Geocode failed for #{latitude}, #{longitude}"
 end
else
 ---save---
end

You can even go one-liners to make the code look more succinct:

g = Geocoder.search(address)
# if no address found, geocode with latitude and longitude
g = Geocoder.search(latitude, longitude) if g.empty?
log.info "Geocode failed for #{latitude}, #{longitude}" && return if g.empty?
---save---

• Victor: Ha! Your first answer was what I had before refactoring!
• Uri Agassi: Nested ifs are considered a smell, just imagine how your code look if had two more fallbacks - each fallback will nest another if, and it would be hard to read the flow of the code - does it save? does it log?