0

I have a simple app that calls an API and returns weather data. A user is able to search for a city and the current temperature is returned. I have a problem though, when the search field is empty or a city that isn't recognised I get an error undefined method[]' for nil:NilClass`. Here is my code:

forecasts_controller.rb

class ForecastsController < ApplicationController
  def current_weather
    @token = Rails.application.credentials.openweather_key
    @city = params[:q]
    if @city == nil
      @forecast = ""
    else
      @forecast = OpenWeatherApi.new(@city, @token).my_location_forecast
    end
  end
end

services/open_weather_api.rb

class OpenWeatherApi
  include HTTParty
  base_uri "http://api.openweathermap.org"

  def initialize(city, appid)
    @options = { query: { q: city, APPID: appid } }
  end

  def my_location_forecast
    self.class.get("/data/2.5/weather", @options)
  end
end

current_weather.html.erb

<%= form_tag(current_weather_forecasts_path, method: :get) do %>
  <%= text_field_tag(:q) %>
  <%= submit_tag("Search") %>
<% end %><br>

<p>Current temperature: <%= @forecast['main']['temp'].to_i - 273 %>°C</p>

Obviously the code ['main']['temp'].to_i - 273 can't be called on nil, but how do I prevent @forecast from being nil when nothing is passed in the form or when the API doesn't recognise the city?

Steve
  • 418
  • 1
  • 4
  • 16
  • Why not wrap that line in `<% if @forecast.present? %> ... <% end %>`? Also, defaulting `@forecast` to `""` is not idiomatic, it's better to just define it as `nil` (or just don't define it at all, because instance variables are nil by default anyway). – max pleaner Jun 06 '19 at 17:58

4 Answers4

1

You might try...

if @city.nil?
  @forecast = {}

To ensure @forecast always responds as a hash, And then in your view you can use dig which will let you drill down into a hash even for nodes that are not present...

<p>Current temperature: <%= @forecast.dig('main', 'temp').to_i - 273 %>°C</p>

But better might be

<% if @forecast.present? %>
  <p>Current temperature: <%= @forecast.dig('main', 'temp').to_i - 273 %>°C</p>
<% else %>
  <p>You need to select a city!</p>
<% end %>
SteveTurczyn
  • 36,057
  • 6
  • 41
  • 53
  • Thanks! Problem with this is that it shows `You need to select a city!` when the page is loaded at first. – Steve Jun 06 '19 at 21:34
  • It also returns `Current temperature: -273°C` if a city is entered that the API doesn't recognise. – Steve Jun 06 '19 at 21:39
  • Look at what the API returns for the condition of unrecognized or blank city. It should be enough info for you to wrap an error paragraph in an if block. – SteveTurczyn Jun 06 '19 at 22:11
1

You can simply use the Safe Navigation Operator (&) before each method to prevent this.

<p>Current temperature: <%= @forecast['main']['temp']&.to_i - 273 %>°C</p>

Refer to this question and answers to learn more about it.

Basically, it prevents undefined method for nil:NilClass from happening. if a value is empty/nil.

Jake
  • 1,086
  • 12
  • 38
  • Except the undefined method is referring to the square brackets, not the `to_i` – SteveTurczyn Jun 06 '19 at 22:07
  • @SteveTurczyn Ah yes, you're right. I'm not sure how to apply it to `@forcast` then. Would it be directly before or after the object? Is it even possible in this case? – Jake Jun 07 '19 at 17:44
  • 1
    Yes, you can `try` for brackets in rails... `[:a, :b, :c].try(:[], 1)` or in this case `@forecast.try(:[], 'main')` and chained would be `@forecast.try(:[], 'main').try(:[], 'temp')` but an interesting weirdness is that `nil.to_i` returns `0` but `nil.try(:to_i)` returns `nil`. – SteveTurczyn Jun 07 '19 at 22:11
  • 1
    I had been searching a couple days for proper use of this. I just needed the ampersand, not the '.to_i' part. ' Like `

    Current city: <%= @forecast['main']['city']& %>°

    `
    – Chnikki Aug 26 '19 at 21:04
  • @Chnikki `.to_i` is "to integer" which is specific to the OP's question. Use `&` before a method to allow it to pass when pulling from a parameter that is empty. – Jake Aug 26 '19 at 21:20
0

Taking advantage of the last comment you can wrap this logic in a view object and do something like this as an option:

class ForecastsController < ApplicationController
  def current_weather
    @city = params[:q]

    @temperature = ForecastView.new(city).temperature
  end
end

class ForecastView
  DESCRIPTIVE_NAME_HERE = 273

  def initialize(city)
    @city = city
    @token = Rails.application.credentials.openweather_key
  end

  def temperature
    forecast.dig('main', 'temp').to_i - DESCRIPTIVE_NAME_HERE
  end

  private

  attr_reader :city, :token

  def forecast
    return {} if city.blank?

    OpenWeatherApi.new(city, token).my_location_forecast
  end
end

<p>Current temperature: <%= @temperature %>°C</p>
0

you can try

<p>Current temperature: <%= @forecast['main']['temp'].to_i - 273 if @forcast['main']['temp'].present? %>°C</p>
Kamal Pandey
  • 1,508
  • 1
  • 8
  • 12