Fetching weather data from the OpenWeatherMap API

Posted on

Problem

I am trying to fetch weather data from the OpenWeatherMap API. The responded JSON will be deserialized into a object which has the same structure of the JSON, then we need to transform this object into another class to meet our needs.

Following is the class which the JSON returned by OpenWeatherMap will be deserialized:

public class OpenWeatherMapResponse implements Serializable {

    private static final long serialVersionUID = 8841521123253407726L;

    private City             city;
    private int              cod;
    private Item[]           list;

    // lots of accessors

    public class static Item {
        // members of inner class Item
    }

    public class static Weather {
        // members of inner class Weather
    }
}

The following is a snippet of the second class. This class has a constructor which takes a OpenWeatherMapResponse and use the value in the response to construct the object will actually be used by my system:

public class CityWeather implements Serializable {

    private static final long serialVersionUID = -4883661697465418806L;

    private static final String ISO8601_FORMAT = "yyyy-MM-dd HH:mm:ss";

    private double            temp;
    private double            minTemp;
    private double            maxTemp;
    private long              weatherId;
    private String            weatherName;
    private String            weatherDesc;     

    public CityWeather() {}

    public CityWeather(OpenWeatherMapResponse openWeatherResponse) {

        Item[] list = openWeatherResponse.getList();
        Weather weather = findWeather(list);  // Get the first element of the Weather list

        // OpenWeatherMap doesn't provide API for fetching a specific day's forecast, so we have to build a day's weather data by assembling ourselves.
        Date tomorrow = getDateOfTomorrow();            
        double tempMax = 0.0, tempMin = 99.0;
        List<Double> temps = new ArrayList<Double>();
        for (Item item : list) {
            Date dt = null;
            if((dt = parseDtTxt(item.getDt_txt())) == null) {
                continue;
            }

            // We only use weather data in recent 24 hours.
            if (dt.after(tomorrow)) {
                break;
            }

            Main main = item.getMain();                
            temps.add(main.getTemp());
            tempMax = Math.max(tempMax, main.getTemp_max());
            tempMin = Math.min(tempMin, main.getTemp_min());
        }

        this.weatherId = weather.getId();
        this.weatherName = WeatherType.fromId((int) weather.getId()).name();
        this.weatherDesc = weather.getDescription();
        this.temp = calculateMean(temps);
        this.maxTemp = tempMax;
        this.minTemp = tempMin;
    }

    // lots of accessors

    // Parse the datetime string
    private Date parseDtTxt(String dtTxt) {
        try {
            return new SimpleDateFormat(ISO8601_FORMAT).parse(dtTxt);
        } catch (ParseException ex) {            
            return null;
        }
    }

    private Date getDateOfTomorrow() {        
        Calendar temp = Calendar.getInstance();
        temp.add(Calendar.DATE, 1);
        return temp.getTime();
    }

    private Weather findWeather(Item[] list) {        
        for (Item item : list) {
            if (item.getWeather() !=null && item.getWeather().length > 0) {
                return item.getWeather()[0];
            }
        }
        return null;
    }

    private double calculateMean(List<Double> numbers) {        
        if (isEmpty(numbers)) return 0;

        double sum = 0;
        for (double d : numbers) {
            sum += d;
        }
        return sum/numbers.size();
    }
}

As you can see, although this CityWeather looks like a data object, there is quite a little logic in this class to initialize its attributes.

Is this a good practice? If not, how should I modify my code?

Solution

I would suggest splitting this into two classes, one being your CityWeather POJO and a separate class for performing the logic currently in CityWeather. The primary reason for this is to separate the business logic here (the parsing of the response from OpenWeatherMap) and your representation of the response (CityWeather). In doing so, whichever class you have that is currently invoking the call to OpenWeatherMap should then pass the OpenWeatherMapResponse response that is returned from the call to this new class, which we’ll call OpenWeatherMapParser. OpenWeatherMapParser should take this response, and instantiate CityWeather objects based on the logic you currently have in CityWeather by calling it’s setter methods

Leave a Reply

Your email address will not be published. Required fields are marked *