Kod programu na zaliczenie.

0

Oto kod źródłowy programu, którego historię opisałem tutaj: Problem ze zleceniem.
Program był pisany na szybko, miałem bardzo mało czasu aby go zrobić. Dlatego też nie jest idealnie zrobiony, ale działa. Chciałbym abście ocenili ten program i napisali czy dałoby się go lepiej napisać. Oto treść zadania, pod które napisałem program:

Wiedza wymagana do realizacji zadań częściowo została zawarta w wytycznych umieszczonych na platformie, dlatego powinniście Państwo najpierw zapoznać się z nimi oraz ze źródłami literaturowymi. Następnie należy przeanalizować, zaprojektować i zaimplementować program rozwiązujący wymyślone przez siebie zadanie operujące na jednej strukturze, strumieniach plikowych, argumentach pobieranych z linii uruchomienia programu (nazwy plików) oraz przedstawiające opis pewnego fragmentu rzeczywistości. Wybrany fragment rzeczywistości i struktury nie mogą powtarzać się pomiędzy Państwem. W związku z tym na forum utworzony zostanie jeden wątek, w którym każdy musi poinformować pozostałych uczestników kursu o wybranym zagadnieniu. W przypadku powtórzonych prac wszystkie zostaną ocenione na 0 pkt.

Na zaliczenie modułu należy przesłać do oceny tylko plik źródłowy *.cpp, nazwany zgodnie z zasadami podanymi w opisie kursu. Wewnątrz pliku źródłowego w komentarzach należy opisać działanie każdej funkcji. Funkcja main() musi zawierać informacje dla użytkownika o sposobie korzystania z programu.

#include <iostream>
#include <fstream>
#include <vector>
#include <stdlib.h>//Potrzebne dla funckji zamieniających łańcuch znakowy na liczby.
#include <conio.h>//Potrzebne dla funckji getch()
using namespace std;
struct pracownik
{
    string imie;
    string nazwisko;
    int rok;
    string pesel;
    float pensja;
    int staz;
};
bool OdtworzBaze(vector<pracownik>&dane, fstream& plik, string nazwa);
bool PokazBaze(const vector<pracownik>&dane);
bool ZapisZmiany(const vector<pracownik>&dane, fstream& plik, string nazwa);
bool ZmodyfikujRekord(vector<pracownik>&dane);
bool UsunRekord(vector<pracownik>&dane);
bool DodajRekord(vector<pracownik>&dane);

int main(int argc, char** argv)
{
    if(argv[1]==NULL)//Sprawdzenie czy podano nazwe pliku.
    {
        cout<<"Nie podales nazwy pliku.\n";
        return 1;//Zwrócenie informacji o nieprawidłowym wykonaniu
    }
    char opcja=0;
    fstream plik;
    vector<pracownik>rekordy;//Przechowuje bazę, na której działają poszczególne funckje.
    string tmp=argv[1];
    OdtworzBaze(rekordy, plik, tmp.c_str());//Otworzenie pliku z bazą o nazwie podanej w linii poleceń.
    cout<<"Dostepne opcje:\n0-Zamkniecie programu.\n1-Pokazanie bazy.\n2-Zapisanie zmian.\n3-Zmodyfikowanie rekordu.\n4-Usuniecie rekordu.\n5-Dodanie rekordu.\n";
    do
    {
        cout<<"Wybierz opcje: ";
        opcja=getch();//Pobranie znaku identyfikującego opcję.
        cout<<"\n";
        switch(opcja)
        {
        case '0': return 0;
        case '1':
            PokazBaze(rekordy);
            break;
        case '2':
            ZapisZmiany(rekordy, plik, tmp);
            break;
        case '3':
            ZmodyfikujRekord(rekordy);
            break;
        case '4':
            UsunRekord(rekordy);
            break;
        case '5':
            DodajRekord(rekordy);
            break;
        default:
            cout<<"Nie ma takiej opcji w menu.\n";
        }

    }while(opcja!=0);//Działanie do momentu wybrania zera na klawiaturze.
    return 0;
}

bool OdtworzBaze(vector<pracownik>&dane, fstream& plik, string nazwa)
{
    dane.clear();
    string tmp;
    plik.open(nazwa.c_str(), ios::in|ios::out);//Otworzenie pliku z opcjami do zapisu i odczytu.
    if(!plik.is_open()) return 0;//Zakończenie pracy funckji, jeśli plik nie jest otwarty.
    for(int i=0;getline(plik, tmp);++i)//Wykonywanie dopóki program będzie mógł pobrać kolejene wiersze z pliku.
    {//Wczytywanie danych z pliku do dyncamicznej tablicy.
        switch(i%6)//Wybiera do jakiego pola wpisać zawartość pobranej linii.
        {
        case 0:
            dane.push_back(pracownik());
            dane[dane.size()-1].imie=tmp;
            break;
        case 1:
            dane[dane.size()-1].nazwisko=tmp;
            break;
        case 2:
            dane[dane.size()-1].rok=atoi(tmp.c_str());//atoi zmienia łańcuch znakowy na liczbę całkowitą.
            break;
        case 3:
            dane[dane.size()-1].pesel=tmp;
            break;
        case 4:
            dane[dane.size()-1].pensja=atof(tmp.c_str());//atof zmienia łańcuch znakowy na liczbę zmiennoprzecinkową.
            break;
        case 5:
            dane[dane.size()-1].staz=atoi(tmp.c_str());
            break;
        }
    }
    return 1;
}
bool PokazBaze(const vector<pracownik>&dane)
{
    for(size_t i=0;i<dane.size();++i)//Wyświetlenie zawartości wszystkich komórek dynamicznej tablicy, która przechowuje w pamięci ram bazę.
    {
        cout<<"Rekord nr. "<<i<<":\n";
        cout<<"Imie: "<<dane[i].imie<<"\n";
        cout<<"Nazwisko: "<<dane[i].nazwisko<<"\n";
        cout<<"Rok: "<<dane[i].rok<<"\n";
        cout<<"Pesel: "<<dane[i].pesel<<"\n";
        cout<<"Pensja: "<<dane[i].pensja<<"\n";
        cout<<"Staz: "<<dane[i].staz<<"\n\n";
    }
    return 1;
}
bool ZmodyfikujRekord(vector<pracownik>&dane)
{
    unsigned int rekord=0;
    char wiersz=0;
    string tmp;
    cout<<"Wybierz numer rekordu: ";
    cin>>rekord;
    if(rekord>=dane.size())//Sprawdzenie czy użyto poprawnego indexu tablicy.
    {
        cout<<"Nie ma takiego rekordu.\n";
        return 0;//Zwrócenie fałszu w przypadku złego indexu.
    }
    cout<<"Wybierz wiersz do modyfikacji(0-imie, 1-nazwisko, 2-rok, 3-pesel, 4-pensja, 5-staz: ";
    wiersz=getch();//Pobranie jednego znaku bez konieczności zatwierdzania enterem.
    cout<<"Wpisz zawartosc wiersza: ";
    cin>>tmp;
    switch(wiersz)//Wybranie pola jakie chce zmienić użytkownik.
    {
    case '0':
        dane[rekord].imie=tmp;
        break;
    case '1':
        dane[rekord].nazwisko=tmp;
        break;
    case '2':
        dane[rekord].rok=atoi(tmp.c_str());
        break;
    case '3':
        dane[rekord].pesel=tmp;
        break;
    case '4':
        dane[rekord].pensja=atof(tmp.c_str());
        break;
    case '5':
        dane[rekord].staz=atoi(tmp.c_str());
        break;
    }
    return 1;
}
bool ZapisZmiany(const vector<pracownik>&dane, fstream& plik, string nazwa)
{
    plik.close();
    plik.open(nazwa.c_str(), ios::in|ios::out|ios::trunc);//Zamknięcie i otwrcie pliku z dodatkową falgą ios::trunc w celu wyczyszczenia jego zawartpści.
    for(size_t i=0;i<dane.size();++i)//Pętla działa na wszystkich elementach tablicy.
    {
        plik<<dane[i].imie<<"\n";//Zapis zawartości komórek dynamicznej tablicy do pliku.
        plik<<dane[i].nazwisko<<"\n";
        plik<<dane[i].rok<<"\n";
        plik<<dane[i].pesel<<"\n";
        plik<<dane[i].pensja<<"\n";
        if(i==dane.size()-1) plik<<dane[i].staz;//Zapobiegnięcie zapisania pustej linii do pliku.
        else plik<<dane[i].staz<<"\n";
    }
    plik.close();
    cout<<"Zapisano.\n";
    return 1;
}
bool UsunRekord(vector<pracownik>&dane)
{
    unsigned int rekord=0;
    cout<<"Wpisz rekord do skasowania: ";
    cin>>rekord;
    if(rekord>=dane.size())//Sprawdzenie czy użyto poprawnego indeksu tablicy.
    {
        cout<<"Nie ma takiego rekordu\n";
        return 0;
    }
    dane.erase(dane.begin()+rekord);//Usunięcie danego elementu z tablicy.
    return 1;
}
bool DodajRekord(vector<pracownik>&dane)
{
    dane.push_back(pracownik());//Utowrzenie nowego elementu w tablicy.
    cout<<"Podaj imie: ";//Pobranie danych od użytkownika do nowego rekordu.
    cin>>dane[dane.size()-1].imie;
    cout<<"Podaj nazwisko: ";
    cin>>dane[dane.size()-1].nazwisko;
    cout<<"Podaj rok: ";
    cin>>dane[dane.size()-1].rok;
    cout<<"Podaj pesel: ";
    cin>>dane[dane.size()-1].pesel;
    cout<<"Podaj pensje: ";
    cin>>dane[dane.size()-1].pensja;
    cout<<"Podaj staz: ";
    cin>>dane[dane.size()-1].staz;
    return 1;
}
4
  1. Nie używaj polskich nazw funkcji i zmiennych.
  2. Zapisuj pracownika a nie listę pracowników.
  3. Używaj foreach a jak nie możesz użyć C++11 to używaj iteratorów.
  4. Po co tworzysz string tmp? Nie ma to najmniejszego sensu.
  5. Z funkcji zwracających bool zwracasz 1. Dodatkowo czemu te funkcje nie są typu void?
  6. Wywal te komentarze, bo one nie mają najmniejszego sensu. Jak już to dokumentuj funkcje, a nie poszczególne linijki.
  7. Absolutnie dodawaj nową, pustą linię na końcu pliku.

Pewnie byłoby tego więcej, ale to może później.

2

1) nie sprawdzasz liczby argumentów (argc)

2) za mało spacji przy przypisaniu wartości

3) zwracasz int z funkcji która powinna zwrócić bool

return 0;//Zwrócenie fałszu w przypadku złego indexu.
...
return 1;

4) używasz przesyłania obiektów przez wartość zamiast przez referencję (ponoć w nowszych kompilatorach to jest ok)

5) zamiast komentarzy na końcu linii stosuj raczej komentarze przed linią (jeśli je musisz dawać)

6) brakująca funkcja to: ZamknijBaze

0

@hauleth:

  1. W swoich projektach wszystko piszę po angielsku, ale w tym przypadku zrobiłem wyjątek, bo to był projekt na zaliczenie.
  2. Czyli co konkretnie zrobić? Wyczyścić linie, gdzie są dane określonego pracownika?
  3. Coś jest w tym złego, skoro 0 to false a wszystko inne to true?

@vpiotr:

  1. Masz na myśli, że zamiast
    dane[dane.size()-1].nazwisko=tmp;

    powinienem zapisać

    dane[dane.size()-1].nazwisko = tmp;
  2. Przecież przekazuję obiekty przez referencję. Piszę bool UsunRekord(vector<pracownik>&dane); zamiast bool UsunRekord(vector<pracownik>dane);

Dzięki za rady w każdym razie ;)

0

Co do angielszczyzny, for-each i "C z klasami" to bym się tym nie przejmował tak bardzo w tym projekcie.
W projekcie komercyjnym to oczywiście jest słabe, ale jakby ktoś oddał taki projekt na moich zajęciach (gdybym jakieś prowadził) gdzie kod wygląda jak spod igły (albo z książki o OOP) to mógłbym nabrać podejrzeń że nie jest autorstwa biednego studenta który mi go dostarczył i nawet gdyby był autorem, miałby dodatkowe pytania.

BTW, mamy tu jednego takiego studenta na forum, który zna lepiej język (chyba Jave) niż wykładowca, ale to wyjątek :)

0

@vpiotr: Co masz na myśli mówiąc, że kod wygląda jak "spod igły (albo z książki o OOP)"?
A co do tego studenta, to niestety nie jest żaden wyjątek, gdyby spojrzeć na niektórych nauczycieli z mojego technikum.

0

Teraz to jest kod typu c z klasami. Tzn sam klas nie definiujesz a jedynie uzywasz. Czyli niby piszesz w C++ ale jednak w C.

Na zaliczenie z przedmiotow nie zwiazanych z OOP to moze byc akceptowalne.

Na rozmowe o prace na stanowisko "programista c++" - raczej nie.

0

@vpiotr: Myślałem nad tym, aby zrobić klasę, w której te funkcje są metodami. Jednak aby pasowało do treści zadania użyłem tylko struktury. Czy do dobrze czy źle to sam pewien nie jestem. I skąd by było u Ciebie podejrzenie, że kod nie należy do studenta? Bo użyłem klas i żadnych nie definiowałem?

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