request weather data and send them to the wirst #29
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,101 @@ | ||
| #include "openweathermapparser.h" | ||
|
|
||
| OpenWeatherMapParser::OpenWeatherMapParser(QObject *parent) : QObject(parent) | ||
| { | ||
| } | ||
|
|
||
| void OpenWeatherMapParser::prepareData(QJsonObject obj) | ||
| { | ||
| m_weatherId.clear(); | ||
| m_tempMin.clear(); | ||
| m_tempMax.clear(); | ||
| m_dateTempMin.setDate(0, 0, 0); | ||
| m_dateTempMax.setDate(0, 0, 0); | ||
|
|
||
| QDateTime timestamp; | ||
| QJsonArray list = obj.value("list").toArray(); | ||
|
|
||
| for(auto element : list) { | ||
| QJsonObject dayInfo = element.toObject(); | ||
| timestamp.setTime_t(dayInfo.value("dt").toInt()); | ||
|
|
||
| setWeatherId(dayInfo.value("weather").toArray(), timestamp.time()); | ||
|
There was a problem hiding this comment. I see that every method has a different way of determining "what day it is" but ultimately it should be the same day for every method.
I propose that you extract here in this loop once for each element the "day index" which is going to be the index at which any value will be inserted (whether that is a weather id or a max/min temperature in their respective arrays). You could calculate this offset quite easily with https://doc.qt.io/qt-5/qdate.html#daysTo by comparing the date to the first date in the json document. If you provide this day index to all your methods, you should find that your code becomes greatly simplified. Especially your "setM**Temp" functions should have less special cases for empty arrays/new dates etc... They could just insert their value at the right index. This also provides you stronger guarantees that the "weatherId"/"maxTemp"/"minTemp" will be aligned. |
||
| setMinTemp(dayInfo.value("main").toObject(), timestamp.date()); | ||
| setMaxTemp(dayInfo.value("main").toObject(), timestamp.date()); | ||
| } | ||
|
|
||
| setCity(obj.value("city").toObject()); | ||
| } | ||
|
|
||
| void OpenWeatherMapParser::setWeatherId(QJsonArray weather, QTime time) | ||
| { | ||
| QTime elevenoclock, thirteenoclock; | ||
| elevenoclock.setHMS(11,0,0,0); | ||
| thirteenoclock.setHMS(13,0,0,0); | ||
|
|
||
| if(time >= elevenoclock && time <= thirteenoclock) { | ||
| QJsonObject firstEntry = weather.first().toObject(); | ||
| m_weatherId << firstEntry.value("id").toInt(); | ||
| } | ||
| } | ||
|
|
||
| QList<short> OpenWeatherMapParser::getWeatherId() | ||
| { | ||
| return m_weatherId; | ||
| } | ||
|
|
||
| void OpenWeatherMapParser::setMinTemp(const QJsonObject obj, const QDate date) | ||
| { | ||
| if(m_tempMin.isEmpty()) { | ||
| m_tempMin << obj.value("temp_min").toDouble(); | ||
| m_dateTempMin = date; | ||
| return; | ||
| } | ||
|
|
||
| if(m_dateTempMin == date && obj.value("temp_min").toDouble() < m_tempMin[m_tempMin.size() - 1]) { | ||
| m_tempMin.replace(m_tempMin.size() - 1, obj.value("temp_min").toDouble()); | ||
| } else if(m_dateTempMin != date) { | ||
| if(m_tempMin.size() >= 5) return; | ||
|
|
||
| m_tempMin << obj.value("temp_min").toDouble(); | ||
| m_dateTempMin = date; | ||
| } | ||
| } | ||
|
|
||
| QList<short> OpenWeatherMapParser::getTempMin() | ||
| { | ||
| return m_tempMin; | ||
| } | ||
|
|
||
| void OpenWeatherMapParser::setMaxTemp(const QJsonObject obj, const QDate date) | ||
| { | ||
| if(m_tempMax.isEmpty()) { | ||
| m_tempMax << obj.value("temp_max").toDouble(); | ||
|
There was a problem hiding this comment. You often extract this "obj.value("temp_max").toDouble()" could you make it a variable that you only extract once to make the code easier to read ? |
||
| m_dateTempMax = date; | ||
| return; | ||
| } | ||
|
|
||
| if(m_dateTempMax == date && obj.value("temp_max").toDouble() > m_tempMax[m_tempMax.size() - 1]) { | ||
| m_tempMax.replace(m_tempMax.size() - 1, obj.value("temp_max").toDouble()); | ||
|
There was a problem hiding this comment. Something along the line of |
||
| } else if(m_dateTempMax != date) { | ||
| if(m_tempMax.size() >= 5) return; | ||
|
There was a problem hiding this comment. Why 5? If this is a requirement of the protocol, I'd recommend that you enforce this in the "weatherservice" rather than in the parser. Ideally, the parser shouldn't have to care about limitations of the protocol that will transfer its data. There was a problem hiding this comment. For example, it is easy for setWeatherId to ignore this restriction. |
||
|
|
||
| m_tempMax << obj.value("temp_max").toDouble(); | ||
| m_dateTempMax = date; | ||
| } | ||
| } | ||
|
|
||
| QList<short> OpenWeatherMapParser::getTempMax() | ||
| { | ||
| return m_tempMax; | ||
| } | ||
|
|
||
| void OpenWeatherMapParser::setCity(const QJsonObject obj) | ||
| { | ||
| m_city = obj.value("name").toString().trimmed(); | ||
| } | ||
|
|
||
| QString OpenWeatherMapParser::getCity() | ||
| { | ||
| return m_city; | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,37 @@ | ||
| #ifndef OPENWEATHERMAPPARSER_H | ||
| #define OPENWEATHERMAPPARSER_H | ||
|
|
||
| #include <QObject> | ||
| #include <QJsonObject> | ||
| #include <QList> | ||
| #include <QJsonArray> | ||
| #include <QDateTime> | ||
|
|
||
| class OpenWeatherMapParser : public QObject | ||
|
There was a problem hiding this comment. Since the methods are all stateless you don't really need a class here (an object of type OpenWeatherMapParser doesn't carry any ownership/lifetime semantic) |
||
| { | ||
| Q_OBJECT | ||
|
|
||
| public: | ||
| OpenWeatherMapParser(QObject *parent = nullptr); | ||
|
|
||
| void prepareData(QJsonObject obj); | ||
| QList<short> getWeatherId(); | ||
| QList<short> getTempMin(); | ||
| QList<short> getTempMax(); | ||
| QString getCity(); | ||
|
|
||
| private: | ||
| void setWeatherId(const QJsonArray weather, const QTime time); | ||
| void setMinTemp(const QJsonObject obj, const QDate date); | ||
| void setMaxTemp(const QJsonObject obj, const QDate date); | ||
| void setCity(const QJsonObject obj); | ||
|
|
||
| QList<short> m_weatherId; | ||
| QList<short> m_tempMin; | ||
| QList<short> m_tempMax; | ||
| QDate m_dateTempMin; | ||
| QDate m_dateTempMax; | ||
| QString m_city; | ||
| }; | ||
|
|
||
| #endif // OPENWEATHERMAPPARSER_H | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't understand why you have one m_wmp that you keep updating. Could you just create a new wmp here and give rootObj to its constructor?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You also wouldn't have to clear fields like you do in your prepareData method