Pobieranie kursu EUR

0

Dzień dobry. Chciałem do swojego programu dodać funkcje pobierania aktualnego kursu euro. Funkcja odpowiedzialna za to jest wywoływana w domyślnym konstruktorze klasy Euro. Niestety funkcja nie działa tak jak powinna tzn. Po analizie programu, wygląda to jakby program pomijał linijki kodu odpowiedzialne za pobranie i zapisanie kursu euro. Poniżej moja klasa:

Currencies.h

#ifndef CURRENCIES_H
#define CURRENCIES_H

#include <QObject>
#include <QFile>
#include <QTextStream>
#include <QtDebug>
#include <QList>
#include <QProcess>

#include <QRegularExpression>
#include <QSettings>
#include <QtNetwork/QNetworkAccessManager>
#include <QtNetwork/QNetworkReply>
#include <QtNetwork/QNetworkRequest>
#include <QtXml/QtXml>
#include <QDebug>
#include <string.h>

#include <QFile>
#include <QByteArray>
#include <QJsonObject>
#include <QJsonDocument>
#include <QObject>
#include <algorithm>

#include <QSettings>



#include "Connections.h"

namespace Currency {

  class Euro : public QObject
  {
      Q_OBJECT

  private:

      double euroRate;
      double finalPriceEuro;
      QString const urlEuroString;
      Connection::Status connectStatus;
      QSettings settings;

  public:
      Euro();
      ~Euro();

      double GetEuroRate() const;
      void SetEuroRate(double);

      void SetFinalPriceEuro(double);
      double GetFinalPriceEuro() const;

      QString GetEuroUrl() const;

      void SaveEuroRate();
      double LoadEuroRate() const;

      void DownloadEuroRate();


  };
 }

Currencies.cpp

#include"Currencies.h"

//Euro
Currency::Euro::Euro():
    euroRate(0.0),
    finalPriceEuro(0.0),
    urlEuroString("http://api.nbp.pl/api/exchangerates/rates/a/eur/?format=json")
{
    DownloadEuroRate();

}

Currency::Euro::~Euro()
{

}

double Currency::Euro::GetEuroRate() const{
    return euroRate;
}

void Currency::Euro::SetEuroRate(double rateValue){
    euroRate = rateValue;
}

QString Currency::Euro::GetEuroUrl() const{
    return urlEuroString;
}

void Currency::Euro::SetFinalPriceEuro(double price)
{
    finalPriceEuro = price;
}

double Currency::Euro::GetFinalPriceEuro() const
{
    return finalPriceEuro;
}

void Currency::Euro::SaveEuroRate(){

    QSettings settings;
    settings.beginGroup("Currency");
    settings.setValue("euroRate",GetEuroRate());
    settings.endGroup();

}

double Currency::Euro::LoadEuroRate() const
{
    QSettings settings;
    settings.beginGroup("Currency");
    return settings.value("euroRate").toDouble();
    settings.endGroup();
}

void Currency::Euro::DownloadEuroRate()
{

    QNetworkAccessManager* manager = new QNetworkAccessManager(this);

    connect(manager,&QNetworkAccessManager::finished,this,[&](QNetworkReply* reply)
    {

        if(reply->bytesAvailable())
        {
            connectStatus.SetConnectionStatus(true);
            qDebug()<<"Nawiazano polaczenie";

        }else
        {
            connectStatus.SetConnectionStatus(false);
        }

        if(connectStatus.GetConnectionStatus())
        {
            QString  stringData = reply->readAll();
            QFile file("temp.json");
            QTextStream stream(&file);
            if(file.open(QIODevice::WriteOnly|QIODevice::Text))
            {
                stream<<stringData;
            }

            file.close();


            QJsonDocument doc = QJsonDocument::fromJson(stringData.toUtf8());

            QJsonObject rootObj = doc.object();

            QJsonValue rates = rootObj.value("rates");

            if (rates.type() == QJsonValue::Array) {

                QJsonArray ratesArray = rates.toArray();

                for (int i = 0; i < ratesArray.count(); i++) {

                    QJsonValue ratesChild = ratesArray.at(i);

                    if (ratesChild.type() == QJsonValue::Object) {

                        QJsonObject ratesObj = ratesChild.toObject();

                        QJsonValue midValue = ratesObj.value("mid");
                        SetEuroRate(midValue.toDouble());
                        SaveEuroRate();

                    }
                }
            }
        }
        else
        {
            SetEuroRate(LoadEuroRate());
            SaveEuroRate();
            qDebug()<<"Brak nowego kursu euro";
        }


       });

    manager->get(QNetworkRequest(QUrl(GetEuroUrl())));


}

mainwindow.cpp

MainWindow::MainWindow(QWidget *parent)
    : QMainWindow(parent)
    , ui(new Ui::MainWindow)
{

    ui->setupUi(this);

    Currency::Euro* euroPtr = new Currency::Euro();
    Currency::Dollar* dollarPtr = new Currency::Dollar();
    Currency::Pound* poundPtr = new Currency::Pound();


    delete euroPtr;
    delete dollarPtr;
    delete poundPtr;
}

Niestety nie mogę znaleźć błędu. Wydaje mi się, że jest coś nie tak z funkcją DownloadEuroRate() ale nie wiem co. Jeżeli ktoś ma jakiś pomysł jak to rozwiązać, będę wdzięczny za każdą wskazówkę. Pozdrawiam !

5

A co ci mowi debuger? Moim zdanie nie powinno strzelac do serwisu zewnetrznego z konstruktora.

3
S4t napisał(a):

... Moim zdanie nie powinno strzelac do serwisu zewnetrznego z konstruktora.

Dokładnie tak!
Oraz nie powinno się ignorować łez kompilatora, przecież ci krzyczy że nic po return się nie wykona.
Oraz nie powinno się pchać zmiennych roboczych do poł klasy.

Wg mnie powinieneś się ograniczyć do:

class AnyValut:public QObject
{
    Q_OBJECT
    private:
    QString name;
    double rate;
    public:
    AnyValut(const QString &name,double rate):name(name),rate(rate) {}

    double GetRate()const { return rate; }
    double ToPln(double valut)const { return valut/rate; }
    double FromPln(double pln)const { return pln*rate; }
    QString GetName()const { return name; }

    class Builder:public QObject
    {
      static AnyValut download(const QString &name)
      {
        ...
        if(bad) throw ...
      }
    }
};
1

Przyznam z podziwem, że nie widziałem jeszcze tak brzydko napisanego connecta.

Zacznę od tego, że ta lambda jest zdecydowanie za długa i powinna być napisana jako osobny slot. A i chętnie bym ją samą podzielił na mniejsze funkcje: saveFile(), processJSON().

Dodatkowo, jako że nie używasz nigdzie poza przypisaniem i porównaniem connectStatus, to nie ma w ogóle potrzeby ustawiania go tylko po to, żeby zaraz go odczytać:

    if(reply->bytesAvailable())
        {
            QString  stringData = reply->readAll();
            QFile file("temp.json");
            QTextStream stream(&file);
            // i tak dalej [...]
        }

2
Mikołaj Nawrocki napisał(a):

Dzień dobry. Chciałem do swojego programu dodać funkcje pobierania aktualnego kursu euro.

Nie chciałbym mieć tego programu u siebie.

Jak na pół roku / rok wysiłku, to słabo
Temat na dziesiatki sposóbów przetrwawiony, na 4p, na githubie gotowce, w każdym języku.
edit: debuger powinien w twoich rękach "ino błyskać" (za @S4t )

xfin napisał(a):

Przyznam z podziwem, że nie widziałem jeszcze tak brzydko napisanego connecta.

Rożnica pomiędzy właścicielem IDE a programistą (to od fotografów: jak kupiłeś lustrzankę, to jesteś właścicielem lustrzanki, ale jeszcze nie fotografem)
Surowe pointery, antyoptymalne podejście (proszenie się o ban na firewallu) itd ...

Od budowania struktur danych w dialekcie C++ od GUI ja osobiscie uciekam jak diabeł od święconej wody. Wyłącznie standard. Klasy GUI niech będą, bo muszą przy GUI., ale tylko tyle.
Pominę że w C# / Javie to 10x mniej linii

1
ZrobieDobrze napisał(a):

Pominę że w C# / Javie to 10x mniej linii

nie wiadomo do czego to przypiąć i jaki cel tym stwierdzeniem chcesz osiągnąć ?

  1. To, że jest 10x mniej linii, to znaczy, że powinien zmienić język ?
  2. To, że jest 10x mniej linii, to znaczy, że C# i Java jest lepsza niż C++ ?
  3. To, że jest 10x mniej linii, to znaczy, że C# i Java staną się łatwiejsze ?
  4. To, że jest 10x mniej linii, to znaczy, że C++ jest trudny ?

Od budowania struktur danych w dialekcie C++ od GUI ja osobiscie uciekam

czemu ? Jaka jest różnica między strukturami danych w C# a w C++ ?

w C++ i w C# wystarczy

struct Exchange
{
   QString currency; //waluta
   double exchangeRate; //kurs wymiany
   double purchase; //kupno
   double sale; //sprzedaż
}

ja nie doświadczony, więc w tym momencie zadaję pytanie - czy jest jakiś problem techniczny ze strukturami, że od nich uciekasz ?

0
zkubinski napisał(a):
ZrobieDobrze napisał(a):
  1. To, że jest 10x mniej linii, to znaczy, że C# i Java jest lepsza niż C++ ?

Kod integracyjny, libki / serwisy wymieniające / utrwalające dane itd 1000x efektywniej się pisze w językach z refleksją (nie wnikając w szczegóły) - jak język tego nie posiada, trzeba walić dużo nieprzenośnego i nieczystego kodu "boilerplate"
Pięc linijek klasy definiującej daną i dwie linijki wywołania blioteki (np RESTowej)

  1. To, że jest 10x mniej linii, to znaczy, że C++ jest trudny ?>

Opinie o łatwości / trudnosci C++ są mocno wyrobione i nie musze tłumaczyć (przy czym inne mają którzy liznęli, inne którzy znają głęboko)

zkubinski napisał(a):
ZrobieDobrze napisał(a):

Od budowania struktur danych w dialekcie C++ od GUI ja osobiscie uciekam

czemu ? Jaka jest różnica między strukturami danych w C# a w C++ ?

Najzwyczajniej nie zrozumiałeś zdania. Przeczytaj jeszcze raz

w C++ i w C# wystarczy

struct Exchange
{
   QString currency; //waluta
   double exchangeRate; //kurs wymiany
   double purchase; //kupno
   double sale; //sprzedaż
}

ja nie doświadczony, więc w tym momencie zadaję pytanie - czy jest jakiś problem techniczny ze strukturami, że od nich uciekasz ?

O ile przeczytasz i zrozumiesz zd. powyżej - kołatanie się literki Q w strukturach, kodzie jak najbardziej ogólnym, w nadziei uniwersalnym, jest jak wrzód na d...
ale to zakładając, że zrozumiesz.

Nie tylko że wymaga build dependencji od Qt każdego przyszłego projektu, ale na runtime wprowadza syf, np odmienne standardy alokowania / zwalniania / odśmiecania/ whatever (QString jeszcze w wcale / w zalążku, ale potem zaczynaja się prawdziwe schody)

0
ZrobieDobrze napisał(a):

O ile przeczytasz i zrozumiesz zd. powyżej - kołatanie się literki Q w strukturach, kodzie jak najbardziej ogólnym, w nadziei uniwersalnym, jest jak wrzód na d...
ale to zakładając, że zrozumiesz.

zamiast QString, można użyć std::string lub bardziej hard i będzie 100% przenośne char *

Nie tylko że wymaga build dependencji od Qt każdego przyszłego projektu, ale na runtime wprowadza syf, np odmienne standardy alokowania / zwalniania / odśmiecania/ whatever (QString jeszcze w wcale / w zalążku, ale potem zaczynaja się prawdziwe schody)

a czemu zakładasz, że jego kod będzie używany np w C# albo Java ? Bo tego domyślam się po tym co piszesz. Jeżeli aplikacja powstaje w Qt, to jaki jest sens pisać klasę, strukturę czy cokolwiek innego aby można było to użyć w innej technologii ? Już prościej wziąć kod i przepisać to pod daną technologię, niż robić to dla wygodnictwa innych. Już twoje założenia są z d...py. Przynajmniej tak to odbieram. A tak na marginesie, czemu zakładasz, że jego program zawojuje świat ? Może pisze coś dla siebie ? Tego nie wiemy

0

Jesteś pewien, że te wszystkie includy są potrzebne?

0

chyba sobie zrobię z tego kopiuj-wklej na przyszłość. Oprócz uwag kolegów:
statyczna analiza bioraca pod uwagę qt
https://github.com/KDE/clazy
https://clang.llvm.org/extra/clang-tidy/
https://cppcheck.sourceforge.io/
i najwazniejsze czytaj co kompilator wyrzuca! Ustaw sobie warningi jako errory.

1

Przeanalizowałem sprawę, więc wg mnie potrzebujesz coś na kształt:

#include <iostream>
#include <vector>
#include <string>
#include <sstream>
#include <unordered_map>
using namespace std;

struct Valut
{
    public:
    enum class Code:uint8_t
	{
		PLN,
        EUR,
        USD,
        CAD,
        CHF,
    };
    private:
    static vector<pair<Valut::Code,string>> &data()
    {
    	static vector<pair<Valut::Code,string>> data
		{
			{Valut::Code::EUR,"EUR"},
			{Valut::Code::USD,"USD"},
			{Valut::Code::CAD,"CAD"},
			{Valut::Code::CHF,"CHF"},
		};
		return data;
	}
	static unordered_map<Valut::Code,string> makeTo()
	{
		unordered_map<Valut::Code,string> map;
		for(auto item:data()) map[item.first]=item.second;
		return map;
	}
	static unordered_map<string,Valut::Code> makeFrom()
	{
		unordered_map<string,Valut::Code> map;
		for(auto item:data()) map[item.second]=item.first;
		return map;
	}
	static unordered_map<Valut::Code,double> makeRates()
	{
		unordered_map<Valut::Code,double> map;
		for(auto item:data()) map[item.first]=0;
		return map;
	}
	static unordered_map<Valut::Code,double> &rates()
	{
		static unordered_map<Valut::Code,double> map(makeRates());
		return map;
	}
	static void setRate(Valut::Code valut,double rate) { if(rates().find(valut)!=end(rates())) rates()[valut]=rate; }
	public:
	static string toString(Valut::Code value)
	{
		static unordered_map<Valut::Code,string> map(makeTo());
		return map[value];
	}
	static Valut::Code fromString(string code)
	{
		static unordered_map<string,Valut::Code> map(makeFrom());
		return map[code];
	}
	static double rate(Valut::Code valut) { return valut!=Valut::Code::PLN?rates()[valut]:1; }
	static double convertFrom(Valut::Code valut,double value) { return value*rate(valut); }
	static double convertTo(Valut::Code valut,double value) { return value/rate(valut); }
	static double convert(Valut::Code from,Valut::Code to,double value) { return convertTo(to,convertFrom(from,value)); }
	static void loadSettings(const vector<pair<string,double>> &settings)
	{
		for(auto item:settings) setRate(fromString(item.first),item.second);
	}
	static void saveSettings(vector<pair<string,double>> &settings)
	{
		settings.clear();
		for(auto item:rates()) settings.push_back(make_pair(toString(item.first),item.second));
	}
};

int main()
{
	const vector<pair<string,double>> settings
	{
		{"EUR",4.6886},
		{"USD",4.3973},
		{"CAD",3.2481},
		{"CHF",4.7439},
		{"GBP",5.4490}, // tego nie notujemy
	};
	Valut::loadSettings(settings);
	cout<<Valut::convertFrom(Valut::fromString("EUR"),100)<<endl;
	cout<<Valut::convertTo(Valut::fromString("EUR"),Valut::convertFrom(Valut::fromString("EUR"),100))<<endl;
	cout<<Valut::convert(Valut::fromString("EUR"),Valut::fromString("USD"),100)<<endl;
	cout<<Valut::convert(Valut::fromString("USD"),Valut::fromString("EUR"),100)<<endl;

    cout<<Valut::convertFrom(Valut::Code::EUR,100)<<endl;
	cout<<Valut::convertTo(Valut::Code::EUR,Valut::convertFrom(Valut::Code::EUR,100))<<endl;
	cout<<Valut::convert(Valut::Code::EUR,Valut::Code::USD,100)<<endl;
	cout<<Valut::convert(Valut::Code::USD,Valut::Code::EUR,100)<<endl;
	return 0;
}
0

Na wstępie chciałbym podziękować wszystkim za odpowiedź na postawione przeze mnie pytanie. Zacznę od początku:

  1. Jaka jest estetyka mojej klasy każdy widzi. Jednym słowem szału nie robi. W tym miejscu chciałbym dopowiedzieć coś, o czym zapomniałem. Klasa Currencies to klasa robocza. Oznacza to w bardzo dużym skrócie, że cały czas są do niej dodawane i usuwane niektóre funkcje. Dopiero po upewnieniu się, że wszystko działa jak należy przechodzę do refaktoryzacji. Dotyczy to wszystkich elementów klasy w tym wyżej omawianej funkcji DownloadEuroRate(). Czy mogłem uporządkować klasę przed dodaniem jej do posta? Jak najbardziej, ale nie zrobiłem tego, dlatego że zależało mi na poprawieniu działania funkcji DownloadEuroRate().

  2. Czy wszystkie pliki nagłówkowe, które zawiera moja klasa są potrzebne? Absolutnie nie. Jak wspomniałem wyżej są one pozostałością po funkcjach, które zostały usunięte, ale includy zostały w razie gdyby były potrzebne.

Nie zostaje mi nic innego, jak usiąść i sprawdzić wszystko debuggerem. Jak będę coś miał to natychmiast wrzucę. Na tą chwilę dziękuję wszystkim serdecznie za udział w dyskusji i pozdrawiam!

0

Sprawdziłem debuggerem co się dzieje w funkcji DownloadEuroRate() i okazuje się, że reply->bytesAvailable() zwraca 0. Z czystej ciekawości wrzuciłem ciało funkcji DownloadEuroRate() do konstruktora mainwindow.cpp i śmiga bezproblemowo. Nie bardzo wiem co jest tego powodem.

1 użytkowników online, w tym zalogowanych: 0, gości: 1