Ocena jakości kodu, na podstawie klasy.

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/Erictriangle/MngX/blob/master/include/control.h

oraz .cpp:
https://github.com/Erictriangle/MngX/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.

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?

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