Ocena jakości kodu, na podstawie klasy.

Odpowiedz Nowy wątek
2019-02-03 15:09
0

Cześć,
Ostatnimi czasy pisałem dla samorozwoju niewielki projekt. Jako że sesja egzaminacyjna dobiegła dla mnie końca, chciałbym
wznowić nad nim pracę. Choć jestem studentem, kod ten praktycznie opiera się na wiedzy samodzielnie przeze mnie zdobytej i
brakuję tam oceny kogoś bardziej doświadczonego. Chciałbym wam poprosić o ocenę i rady na podstawie jednej z napisanych przeze mnie klas.
Co można poprawić, czy kod jest czytelny, czy może użyte rozwiązania mają jakikolwiek sens.

Plik nagłówkowy:
https://github.com/Erictriang[...]blob/master/include/control.h

oraz .cpp:
https://github.com/Erictriang[...]X/blob/master/src/control.cpp

(Wiem że formatowanie mi się trochę rozjechało.)

Cóż ta klasa robi?
Zacznijmy od main'a:
https://github.com/Erictriangle/MngX/blob/master/main.cpp

Control control(argc, argv);
Config_Directory config_directory;
Config config(config_directory.get_path());

while(!control.empty() && control.status())
    Control::exec_command(control, config, config_directory);

Obiekt pobiera dane wprowadzone przez użytkowania, np. "-c add-directory global /home/user -h config".
Wejściowy ciąg znaków jest dzielony, przypisywany do obiektów struktury:


    struct command{
        std::string name; // -c
        std::deque<std::string> arguments; //add-directory global /home/user
    };

a te gromadzone w std::deque<command> commands;
Jak wykonywane są operację w danej klasie?

Poprzez flagę (tutaj -c), wskakujemy do wybranego modułu:

void
Control::exec_command(Control& control, Config& config,
    Config_Directory& config_directory){

    command cmd = control.get_command();
    auto key = flag_command_map.find(cmd.name);

    switch(key->second){
    case UNKNOWN_COMMAND:
        break;

    case HELP:
        exec_help(cmd.arguments);
        break;

    case CONFIG:
        exec_config(cmd.arguments, config, config_directory);
        break;

    case ARCHIVE:
        exec_archive(config);
        break;

    default:
        throw " -=[ EXCEPTION ]=-  exec_command exceptions!";
    }
}

Następnie do bardziej wyspecjalizowanej metody (exec_config_add_row - wiem że jest rozbieżność między nazwą polecenia a metody):

void
Control::exec_config(string_deq& cmd, Config& config, Config_Directory& config_directory){
    auto key = take_key(config_commands_map, cmd.front());
    switch(key){
    case UNKNOWN_COMMAND:
        screen::incorrect_subcommand("-c | --config", cmd.front());
        break;

    case CREAT:
        exec_config_creat(cmd, config);
        break;

    case LOAD:
        exec_config_load(cmd, config_directory);
        break;

    case ADD_DIRECTORY:
        exec_config_add_row(cmd, config);
        break;

    case REMOVE_DIRECTORY:
        exec_config_remove_row(cmd, config);
        break;

    default:
        throw " -=[ EXCEPTION ]=-  exec_config exception!"; 
    }
}

I na końcu:

void
Control::exec_config_add_row(string_deq& cmd, Config& config){
    Config::SECTION section = Config::GLOBAL;
    cmd.pop_front(); //first is add_directory

    while(!cmd.empty()){
        if(Config::input_section_map.find(cmd.front()) != Config::input_section_map.end())
            section = Config::input_section_map.find(cmd.front())->second;
        else
            config.add_row(section, cmd.front());
        cmd.pop_front();
    }
}

Schemat powtarza się dopóki commands nie jest pusty.
Podałem tę klasę jako przykład, ponieważ wydaje mi się że przy większej ilości możliwych do wykonania operacji
kod ten stanie się nieprzejrzysty, a do tego praktycznie wszystkie inne pliki czeka duży commit za niedługo (zwłaszcza klasę config,
gdzie aktualnie mam duże wątpliwości co do długości metod i zarządzania obiektami biblioteki std::fstream).
Do tego zawiera przykłady zarówno krótkich metod prywatnych, jak i nieco dłuższych.

Z góry dziękuję za zainteresowanie tematem i ewentualną pomoc.

Pozostało 580 znaków

2019-02-03 16:39
3

Z tego co widzę to tak:

  • naucz się stosować camelCase do nazw zmiennych i funkcji, a PascalCase do nazw klas/struktur/enumów/namespace'ów itd
  • piszesz w C++, więc otwierający nawias wąsaty stawiaj po bożemu w nowej linii, nie jak zdegenerowani dżawowcy
  • nie żałuj sobie bloku {} nawet dla jednolinijkowych wyrażeń
  • Config_Directory jest inicjalizowane domyślnym konstruktorem, a potem przekazujesz ścieżkę do Config - jaki jest zatem sens istnienia tej klasy, skoro można by klasie Config dodać odpowiednie pole?

"Sugeruję wyobrazić sobie Słońce widziane z orbity Merkurego, a następnie dupę tej wielkości. W takiej właśnie dupie specjalista ma teksty o wspaniałej atmosferze, pracy pełnej wyzwań i tworzeniu innowacyjnych rozwiązań. Pracuje się po to, żeby zarabiać, a z resztą specjalista sobie poradzi we własnym zakresie, nawet jeśli firma mieści się w okopie na granicy obu Korei."
-somekind,
konkretny człowiek-konkretny przekaz :]
Czemu pascalCase dla nazw zmiennych i funkcji? W C++ częściej się spotykam ze snake_case dla tychże. To samo się dotyczy 2 pkt. najczęściej spotykam się z { w tej samej linii (sam zawsze tak piszę). - hauleth 2019-02-03 21:17
@hauleth: Kwestia gustu i przyjętego standardu. Dla mnie klamry w nowych liniach są czytelniejsze. Wyraźnie określają początek i koniec bloku, patrzę na klamrę otwierającą i jadę w dół aż nie zobaczę zamykającej w tej samej kolumnie. Dla mnie jest wygodniej, dla innych to nadmiarowe/brzydkie/nieczytelne... - Bartosz36 2019-02-04 14:44

Pozostało 580 znaków

Odpowiedz
Liczba odpowiedzi na stronę

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