-
Notifications
You must be signed in to change notification settings - Fork 9
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
request weather data and send them to the wirst #29
base: master
Are you sure you want to change the base?
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,77 @@ | ||
#include "openweathermapparser.h" | ||
|
||
OpenWeatherMapParser::OpenWeatherMapParser(QObject *parent) : QObject(parent) | ||
{ | ||
} | ||
|
||
QList<short> OpenWeatherMapParser::getWeatherId(QJsonArray list) | ||
{ | ||
QList<short> id; | ||
QDateTime timestamp; | ||
|
||
for(int i = 0; i < 5; i++) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I recommend using list.size() like in the other methods or you risk accessing undefined values. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think range-based loops should work on QJsonArray https://en.wikipedia.org/wiki/C%2B%2B11#Range-based_for_loop |
||
QJsonObject dayInfo = list.takeAt(i).toObject(); | ||
QJsonArray dayWeather = dayInfo.value("weather").toArray(); | ||
QJsonObject firstEntry = dayWeather.takeAt(0).toObject(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. QJsonArray has a |
||
id << firstEntry.value("id").toInt(); | ||
} | ||
|
||
return id; | ||
} | ||
|
||
QList<short> OpenWeatherMapParser::getTempMin(QJsonArray list) | ||
{ | ||
QList<short> tempMin; | ||
QDateTime timestamp; | ||
QJsonObject dayInfo = list.at(0).toObject(); | ||
timestamp.setTime_t(dayInfo.value("dt").toInt()); | ||
QString actDate = timestamp.toString("dd.MM.yyyy"); | ||
QJsonObject dayMain = dayInfo.value("main").toObject(); | ||
short int tmp = dayMain.value("temp_min").toDouble(); | ||
|
||
for(int i = 1; i < list.size(); i++) { | ||
dayInfo = list.at(i).toObject(); | ||
timestamp.setTime_t(dayInfo.value("dt").toInt()); | ||
dayMain = dayInfo.value("main").toObject(); | ||
if(actDate == timestamp.toString("dd.MM.yyyy") && dayMain.value("temp_min").toDouble() < tmp) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. a string comparison is always quite an expensive operation (and also a bit risky as a simple space or something is always a risk), it's usually the wrong way to approach a problem. What if you'd compare QDates instead https://doc.qt.io/qt-5/qdatetime.html#date ? |
||
tmp = dayMain.value("temp_min").toDouble(); | ||
} else if(actDate != timestamp.toString("dd.MM.yyyy")) { | ||
tempMin << tmp; | ||
actDate = timestamp.toString("dd.MM.yyyy"); | ||
tmp = dayMain.value("temp_min").toDouble(); | ||
} | ||
} | ||
|
||
return tempMin; | ||
} | ||
|
||
QList<short> OpenWeatherMapParser::getTempMax(QJsonArray list) | ||
{ | ||
QList<short> tempMax; | ||
QDateTime timestamp; | ||
QJsonObject dayInfo = list.at(0).toObject(); | ||
timestamp.setTime_t(dayInfo.value("dt").toInt()); | ||
QString actDate = timestamp.toString("dd.MM.yyyy"); | ||
QJsonObject dayMain = dayInfo.value("main").toObject(); | ||
short int tmp = dayMain.value("temp_max").toDouble(); | ||
|
||
for(int i = 1; i < list.size(); i++) { | ||
dayInfo = list.at(i).toObject(); | ||
timestamp.setTime_t(dayInfo.value("dt").toInt()); | ||
dayMain = dayInfo.value("main").toObject(); | ||
if(actDate == timestamp.toString("dd.MM.yyyy") && dayMain.value("temp_max").toDouble() > tmp) { | ||
tmp = dayMain.value("temp_max").toDouble(); | ||
} else if(actDate != timestamp.toString("dd.MM.yyyy")) { | ||
tempMax << tmp; | ||
actDate = timestamp.toString("dd.MM.yyyy"); | ||
tmp = dayMain.value("temp_max").toDouble(); | ||
} | ||
} | ||
|
||
return tempMax; | ||
} | ||
|
||
QString OpenWeatherMapParser::getCity(QJsonObject obj) | ||
{ | ||
return obj.value("name").toString().trimmed(); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,23 @@ | ||
#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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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); | ||
|
||
QList<short> getWeatherId(QJsonArray list); | ||
QList<short> getTempMin(QJsonArray list); | ||
QList<short> getTempMax(QJsonArray list); | ||
QString getCity(const QJsonObject obj); | ||
}; | ||
|
||
#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.
Given that this class does a lot, it would be helpful to name this into something more specific, maybe onOwmReplyFinished?
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.
or maybe you could use a lambda and inline the slot in owmRequest. I think that's what the cool kids would do these days