Ocena kodu aplikacji

0

Witam

Napisałem sobie (dla sportu w zasadzie) program dzięki któremu możemy zaplanować wyłączanie komputera po pewnej godzinie.
Chciałbym prosić o ocenę kodu który dołączam oraz ewentualnie test działania.
Zalecam użycie na maszynie wirtualnej, żeby nie było :)
Testowany na Windows 7 64 bit Ultimate.

Program tak się mniej więcej prezentuje, bo działa w tle w zasadzie.
screen1.PNG

Program pobiera godzinę o której należy zamknąć system z pliku configuration.txt w głównym katalogu, obsługuje wpisanie niepoprawnej godziny planowanego zamknięcia oraz brak pliku konfiguracyjnego (to w sumie jedyne co obsługuje :) )
W związku z czym jeżeli uruchomisz plik wykonywalny który jest w archiwum lub samodzielnie skompilujesz kod programu i okazało by się że nie wyskakuje okienko (patrz screen wyżej) to należny sprawdzić i ewentualnie edytować zawartość tego pliku.
Tak domyślnie wygląda ów plik.
kko.PNG
Wspomnę dodatkowo że w pierwszej kolumnie jest 'indeks' dnia. 0 oznacza poniedziałek, 1 wtorek etc.
W drugiej kolumnie godzina - zakres 0-23 potem program wypisze komunikat o błędzie.
W 3 kolumnie minuta [godziny o której zamknąć system] - zakres : 0-59

Aha i nazwałem go ASSP od
Automatic System Shutdown Program - więc jak się można domyślić na gwałt potrzebuje pomysłów na lepsza nazwę :)

IDE: CodeBlock 13.12
Kompilator: g++ (tdm-2) 4.8.1
Użyłem SFML 2.3.2
Obsługiwane platformy: na razie tylko Windows a docelowo również GNU/Linuks

Wspomnę na koniec że kod może być niskiej jakości, sam wiem które elementy należałoby napisać inaczej ale postawiłem sobie termin na dziś by go oddać więc chce byście wiedzieli że cześć kodu jest pisana "na szybko".

=====================
Wszystko spakowałem do archiwum RAR programem WinRar.
W archiwum jest również biblioteka SFML 2.3.2
W związku z tym jeżeli ktoś ma CodeBlock i otworzy plik projektu to nie powinna wystąpić potrzeba zmiany ścieżek itd.
LINK: http://sendfile.es/pokaz/617017---u5d7.html

0

configa zapisuj np w pliku ini. Zniknie problem z niepoprawnym formatem.

1

Nie przeglądałem dokładnie całego kodu, ale te rzeczy wyłapałem

CSignalsManager::~CSignalsManager()
{
    delete m_SignalsQueue;
    m_SignalsQueue = nullptr;
}

void CSignalsManager::initialize()
{
    m_SignalsQueue = new std::queue<uint32_t>;
}

Tego nie rozumiem. Czemu nie stworzysz zwykłego obiektu tylko babrasz się we wskaźnikach? (a jak już używasz wskaźników to używaj smart pointerów)

  1. Czemu masz kolejkę uint32_t? Z tego co widziałem to zawsze do sendSignal() przekazywałeś enuma ESignals. Np.
CSignalsManager::sendSignal(ESignals::ApplicationStart);

więc czemu nie zrobisz kolejki std::queue<ESignals>;


3. 
```cpp
enum Cos {};

Lepiej jest używać enum class

enum class Cos {};
</li> </ol>
/// Magic nmber - 15 - is delay of shutdown in minutes
                     newTimeShutdown(*window,dialogBox,timer,timerThread,15);

Zamiast magick + komentarz lepiej zrobić consta o sensownej nazwie.
A skoro 15 jest "domyślną" ilością minut to może warto zrobić z tego domyślny parametr zamiast zawsze przekazywać

void newTimeShutdown(sf::RenderWindow & window,
                     CDialogBox &dialogBox,
                     CTimer & timer,
                     sf::Thread* & timerThread,
                     int delayInMinutes = 15); // tu też const by się przydał jakiś
Wszędzie praktycznie masz wypisywanie na std::cout. Lepiej byłoby zrobić jakiś logger</li> </ol>

A tak w ogóle to sam mogłem pokręcić bo się dopiero uczę (:
Jak wrzucisz kod na gh to pewnie dostaniesz review od większej ilości bardziej kompetentnych osób

0

1 Tego nie rozumiem. Czemu nie stworzysz zwykłego obiektu tylko babrasz się we wskaźnikach? (a jak już używasz wskaźników to używaj smart pointerów)

Nie potrzebuje inteligentnych wskaźników w tak małej klasie a wskaźników używam bo klasa jest statyczna a statyczne dane muszą być zdefiniowane poza definicją klasy. Gdy nie używałem wskaźników to mi jakieś błędy wyskakiwały - nie chciałem się użerać wiec używam wskaźników.

3 Lepiej jest używać enum class

Czemu ?

5 Pomyśle nad loggerem.

0
kacper546 napisał(a):

Nie potrzebuje inteligentnych wskaźników w tak małej klasie

Jak tam wolisz. Do takiego czegoś wystarczył by unique_ptr.

kacper546 napisał(a):

a wskaźników używam bo klasa jest statyczna a statyczne dane muszą być zdefiniowane poza definicją klasy.

Nie wiem w czym problem.

kacper546 napisał(a):

Czemu ?

http://stackoverflow.com/questions/18335861/why-is-enum-class-preferred-over-plain-enum

0

Witam ponownie

Ogarnąłem gita i githuba.

Proszę o ocenę kodu programu:
Link do repozytorium na GitHubie: skasowane

Mam zamiar jeszcze popracować na tym programem więc sam będę modyfikował kod i starał się wprowadzać ewentualne zmiany sugerowane przez Was.

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