R.....y
R.....y
0
  • Najlepsze
  • Wszystkie komentarze
M.....f
M.....f
@RedveKoronny:

dodatkowo to nie jest etap programu gdzie wydajność jest kluczowa
8
M.....f
M.....f
@RedveKoronny:

1. Używasz std::map https://www.wykop.pl/wpis/66670015/z-czego-wynika-popularnosc-std-map-aka-drzewo-czer/
2. Pola w klasie nie powinny być "const"
3. items.pushback(Item(name, currency)); można zrobić emplaceback
4. Nie wiem czy to jest zamierzone, ale pola w InputData są publiczne.
5. Używasz std::endl
3
M.....f
M.....f
@RedveKoronny:

różnica w wydajności tutaj będzie marginalna


Tak jak wszystko w twoim programie, ale to pokazuje twoje niechlujne
2
M.....f
M.....f
@RedveKoronny:
6. bool IsNum jest const
7. Nie ma testów
8. Nie ma namespaców
9. Funktory śmierdzą
10. Mam wrażenie, że jest za dużo includów
1
M.....f
M.....f
@RedveKoronny:

11. znaleść
1
MamCieNaHita
@RedveKoronny: na szybko, bo kto robi dobrze
1. ten statyczny std map najlepiej zamienić na free function ze switch casem, a jak tak bardzo się upierasz, to zrób go globalnego(statici w funkcji zawsze sprawdzają czy są zainicjalizowane z każdym zawołaniem funkcji)
2. curl request nie używa raii
3. te wszelkie predykaty powinny być free function
4. hextochar też, i te strumienie są okropnie wolne
5. to zastępowanie reserved
1
M.....f
M.....f
@maisterwihaister: Oczywiście, że robi.
1
maisterwihaister
@Malkof: rzeczywiście, zapobiega też copy-list-initialization. Tu przykład jakby ktoś jeszcze nie wiedział: https://en.cppreference.com/w/cpp/language/explicit
1
R.....y
R.....y
@RedveKoronny: jakby ktoś próbował odpalić, to jednak program defaultowo nie szuka pliku 'items' tylko w--------a segmentation fault
0
R.....y
R.....y
@RedveKoronny: dobra, naprawione
0
zrakiep
@RedveKoronny: na szybko to Update nie powinno pisać na ekran. Powinieneś mieć oddzielnie logikę gdzie pobierasz dane i je przetwarzasz i oddzielnie logikę gdzie z nią coś robisz - czy to na ekran czy może do .csv by se w excelu wykresy porobić. Mapa walut też niepotrzebna - jak masz 3 wartości to switch w konstruktorze i zapisanie sobie stringa nazwy waluty - jako bonus jak walute pomylisz to wywali
0
zrakiep
@RedveKoronny: o, i jeszcze nie ma potrzeby by wszystko miało swój katalog - tj te Itemy i InputData. Moze luze w src leżeć, łatwiej przeglądać będzie
0
Whiskeyjack29
Whiskeyjack29
@zrakiep: to mnie zawsze zastanawiało, w cpp wszystko trzymają w src luzem, ewentualnie 1 katalog wgłąb na sub-moduł/bibliotekę potrzebną w projekcie. W innych javach czy c# tysiące katalogów ( ͡° ͜ʖ ͡°)
0
R.....y
R.....y
Mapa walut też niepotrzebna - jak masz 3 wartości to switch w konstruktorze i zapisanie sobie stringa nazwy waluty


@zrakiep: jest po to żeby móc bardzo łatwo dodać kolejne waluty w przyszłości, i żeby zachować zgodność z dokumentacją steama

Te generowanie linka też może być w konstruktorze, nic się tam nie dzieje takiego by potrzebna była oddzielna
0
R.....y
R.....y
Treść przeznaczona dla osób powyżej 18 roku życia...
0
R.....y
R.....y
@RedveKoronny: 10. wywaliłem tylko iostream z main.cpp które zostało po debugowaniu
0
R.....y
R.....y
o, i jeszcze nie ma potrzeby by wszystko miało swój katalog - tj te Itemy i InputData. Moze luze w src leżeć, łatwiej przeglądać będzie


@zrakiep: zrobiłem sobie skrypt który tak mi tworzy klasy. Wiem że zadziała i bez tego, ale moim zdaniem jest sporo czyściej
0
R.....y
R.....y
@Malkof:

1. Używasz std::map


różnica w wydajności tutaj będzie
0
maisterwihaister
@RedveKoronny: explicit nic nie robi kiedy konstruktor ma więcej niż 1 parametr
0
j.....0
mogę prosić kogoś o code review?


@RedveKoronny: widzę lubisz krytykę, tą sensowną i tą niezbyt skoro tutaj piszesz xD
Ale proszę bardzo:

1) Skoro i tak już wciągasz bibliotekę do parsowania json'ów to jest jakieś uzasadnienie, aby ta konfiguracja (plik items) była w takiej dziwnej formie? Nie dałoby się z tego zrobić
0