[0] Brak konstruktorów – przydałby się chociażby dla
CrystallGrammarModel , ręczne ustawianie parametrów to kiepski pomysł.
[1] Metoda
CristallGrammar::addGrammar i pochodne: użycie new-delete jest tu kompletnie niepotrzebne. Ponadto patrz [0] – konstruktor!
[2] Klasa
CrystallValues definiuje właściwie wolne funkcje i nie ma potrzeby opakowywania ich w klasę – C++ nie jest restrykcyjnie obiektowy. Ewentualnie można zapakować je w namespace.
[3] Z drugiej strony – używana w metodach
CrystallValues zmienna
Summary wygląda na globalną. Pozbądź się tego natychmiast!
[4] Jak już wspominałem:
using namespace na poziomie nagłówków to zła praktyka, polecam to zmienić.
[5] Nazywanie zmiennych wielką literą trochę mnie razi, ale jest konsekwencja, więc uznam to za coding style.
[6] Nie ma potrzeby nadawać explicite wartości dla wartości
enum poza jednym przypadkiem
Rules , gdzie wyraźnie są to flagi bitowe.
[7] Zdecyduj się, czy metody nazywasz wielką czy małą literą.
@Rincewind: @sylwke3100: w praktyce wypadało by ograniczyć do review do mniejszych fragmentów. Mało komu będzie chciało się przeklikiwać przez cały projekt.
Elon Musk właśnie oficjalnie pokazał faka Unii Europejskiej. Jedyny człowiek którego stać na to żeby być wolnym #bekazlewactwa #ekonomia #polska #bitcoin
Repozytorium: https://github.com/sylwke3100/cristallparser
[0] Brak konstruktorów – przydałby się chociażby dla
CrystallGrammarModel, ręczne ustawianie parametrów to kiepski pomysł.
[1] Metoda
CristallGrammar::addGrammari pochodne: użycie new-delete jest tu kompletnie niepotrzebne. Ponadto patrz [0] – konstruktor!
[2] Klasa
CrystallValuesdefiniuje właściwie wolne funkcje i nie ma potrzeby opakowywania ich w klasę – C++ nie jest restrykcyjnie obiektowy. Ewentualnie można zapakować je w namespace.
[3] Z drugiej strony – używana w metodach
CrystallValueszmienna
Summarywygląda na globalną. Pozbądź się tego natychmiast!
[4] Jak już wspominałem:
using namespacena poziomie nagłówków to zła praktyka, polecam to zmienić.
[5] Nazywanie zmiennych wielką literą trochę mnie razi, ale jest konsekwencja, więc uznam to za coding style.
[6] Nie ma potrzeby nadawać explicite wartości dla wartości
enumpoza jednym przypadkiem
Rules, gdzie wyraźnie są to flagi bitowe.
[7] Zdecyduj się, czy metody nazywasz wielką czy małą literą.
#codereview #cpp
parseDatajest o wiele za duża, należałoby podzielić ją na co najmniej 4 metody, a prawdopodobnie ponad 6. ;)