# Nested trial and error in if-else condition

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?

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


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---

• Ha! Your first answer was what I had before refactoring! – Victor Feb 8 '14 at 18:33
• 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? – Uri Agassi Feb 8 '14 at 19:10