Pierwszy program - co dodać , co zmienić, a co wyrzucić

0

Witam. Zaczynam naukę w domowym zaciszu c++. Bardzo opornie mi to idzie niestety, ale cóż jeszcze chęci są. Napisałem pierwszy w miarę mający sens programik tworzący książkę adresową i chciałbym prosić żeby jakiś fachowiec zerknął na to co jest źle, co zmienić , gdzie można by dodać wskaźniki z którymi trochę ciężko mi idzie na razie. Po prostu proszę kogoś o ukierunkowanie co poprawić. Z góry dzięki wielkie. W załączniku jest paczka.

1

Pierwsza sprawa: jeśli korzystasz z Visual Studio to koniecznie polecam korzystać z opcji formatowania dokumentu - bo to jest średnio czytelne. Zobacz jak Ci dokument przeformatuje i zacznij od trzymania tego stylu.
2. wyrabiaj dobre nawyki i od początku pisz kod po angielsku: nazwy zmienny, funkcji

0

Korzystam z Code Blocks, nie zastanawiałem się nad tym czy jest w ogóle taka opcja , ale fakt jest to na pewno istotne, postaram się coś z tym zrobić.
Z angielskim, kolejna trafna uwaga.
Dziękuję.

1

Potknięcia:

  1. Niepotrzebne zmienne globalne;
  2. Czemu tak while(wybor == 1|| wybor == 2 || wybor == 3); zamiast while(wybor != 4);?
  3. Fatalne formatowanie;
  4. Polskie nazwy obiektów, funkcji etc.;
  5. Niepotrzebny #include <cstdlib>;
  6. Masz miejsce na 20 książek ksiazka dane[20], a nigdzie nie pilnujesz ile książek dodał użytkownik;
  7. Tak samo jak nie pilnujesz ile książek zostanie zaczytanych z pliku: while (getline(plik, linia));
  8. Powtarzasz się w kodzie. Powtarzające się rzeczy powsadzaj do funkcji, bo funkcje odczyt_pojedynczy i odczyt_caly są niemal identyczne;
  9. Przy odczycie pojedynczym nie musisz uzupełniać całej tablicy żeby zobaczyć pojedynczy wpis. Starczy zliczać linijki i dopiero wskazaną zaczytać do obiektu klasy ksiazka. Do jednego egzemplarza, a nie tablicy;
  10. Prototypy funkcji powyżej maina niepotrzebne;
2

@Ccoder: Możesz zacząć powoli czytać sobie trochę o klasach, np. stąd: http://fasadin.4programmers.net/?view=flipcard i zrobić np. coś w podobie do tego:

#include<iostream>
#include<string>
#include<vector>
using namespace std;

class person
{
private:
    string name;
    string surname;
    unsigned phone;

public:
    person(const string& name, const string& surname, unsigned phone)
        :name(name), surname(surname), phone(phone)
    {
    }

    void show()
    {
        cout << "Imie: " << name << "\nNazwisko: " << surname << "\nTelefon: " << phone << "\n\n";
    }
};

class database
{
private:
    vector<person> persons;

public:
    void add_person()
    {
        string name = "", surname = "";
        unsigned phone = 0;

        cout << "Imie: "; cin >> name;
        cout << "Nazwisko: "; cin >> surname;
        cout << "Telefon: "; cin >> phone;

        persons.push_back(person(name, surname, phone));
        cout << "\nDodano nowa osobe.\n\n";
    }

    void show_all_persons()
    {
        if (persons.size() > 0)
        {
            for (size_t i = 0; i < persons.size(); ++i)
                persons[i].show();
        }
        else cout << "Baza danych jest pusta!\n\n";
    }
};

class gui
{
private:
    database db;

public:
    void show()
    {
        char choice = ' ';
        do
        {
            cout << "1. Dodaj osobe;\n2. Wypisz osoby;\n3. Wyjdz\n\nWybieram: ";
            cin >> choice;

            switch (choice)
            {
            case '1':
                db.add_person();
                break;

            case '2':
                db.show_all_persons();
                break;

            default:
                choice = '3';
                break;
            }
        }
        while (choice != '3');
    }
};

int main()
{
    gui g;
    g.show();
    return 0;
}

Zalety:

  1. Zauważ jak mały jest main;
  2. Mamy osobną strukturę danych do przechowywania informacji o osobie, osobną do przechowywania listy osób i jeszcze osobną jedynie do wyświetlania menu. Mamy więc separację warstw w projekcie, chociaż można by jeszcze dołożyć np. klasę odpowiedzialną za samo tylko prezentowanie danych i tam poprzenosić rożnego rodzaju show etc...
  3. Mamy vector czyli taki pojemnik na dane, który nie ogranicza Cie odgórnie ustalonym rozmiarem.
0
grzesiek51114 napisał(a):

Potknięcia:

  1. Niepotrzebne zmienne globalne;
  2. Czemu tak while(wybor == 1|| wybor == 2 || wybor == 3); zamiast while(wybor != 4);?
  3. Fatalne formatowanie;
  4. Polskie nazwy obiektów, funkcji etc.;
  5. Niepotrzebny #include <cstdlib>;
  6. Masz miejsce na 20 książek ksiazka dane[20], a nigdzie nie pilnujesz ile książek dodał użytkownik;
  7. Tak samo jak nie pilnujesz ile książek zostanie zaczytanych z pliku: while (getline(plik, linia));
  8. Powtarzasz się w kodzie. Powtarzające się rzeczy powsadzaj do funkcji, bo funkcje odczyt_pojedynczy i odczyt_caly są niemal identyczne;
  9. Przy odczycie pojedynczym nie musisz uzupełniać całej tablicy żeby zobaczyć pojedynczy wpis. Starczy zliczać linijki i dopiero wskazaną zaczytać do obiektu klasy ksiazka. Do jednego egzemplarza, a nie tablicy;
  10. Prototypy funkcji powyżej maina niepotrzebne;

1.Oprócz struktury i wywołań funkcji nie potrafię zlokalizować żadnej globalnej zmiennej.
2.Poprawione.
3.Nie mam pojęcia jak powinno wyglądać prawidłowe , Ja się w tym łapie, wiem , że jest tragiczne , ale nie wiem jak to poprawić, jakiś wzorzec ?
4.Poprawione.
5.Poprawione.
6.Jak by tu zrobić , aby nie była ograniczona liczba książek, nie trzeba by tego pilnować ?
7.Nie mam pojęcia jak to poprawić.
8.Próbuję i nie mogę sobie poradzić , przerzucam od momentu file.open do file.close do osobnej funkcji i potem w danej funkcji chcę wywołać od tego momentu ale nie przekazuje mi konkretnych danych które mi zlicza i wtedy mi nie wyświetla całej książki.
9.Kompletnie nie wiem jak nie uzupełniać całej tablicy.
10.Muszą być , bo jak je mam po main to mi wywali błąd jak ich nie będzie.

#include <iostream>
#include <fstream>
#include <string>
#include <algorithm>

using namespace std;

struct adress_book
{
string name;
string surname;
string street;
int h_number;
int f_number;
int p_number;

};
adress_book data[20];

int registration();
int single_reading();
int whole_reading();

fstream file;
int main()
{
int choice;
while(true)
{
cout<<"-----------------------------------------------"<<endl;
cout<<"MENU |"<<endl;
cout<<"1.Dodaj wpis do ksiazki adresowej |"<<endl;
cout<<"2.Odczytaj wpis z ksiazki adresowej |"<<endl;
cout<<"3.Odczytaj wszystkie wpisy z ksiazki adresowej |"<<endl;
cout<<"4.Wyjscie z programu |"<<endl;
cout<<"-----------------------------------------------"<<endl;
cin>>choice;
switch(choice)
{
case 1: registration(); break;
case 2: single_reading(); break;
case 3: whole_reading() ; break;
case 4: exit(0);
}
}

return 0;

}
int registration()
{
int h_m_entries;
cout<<"Ile wpisow chcesz dodac ? "<<endl;
cin>>h_m_entries;
for(int i=0;i<h_m_entries;i++)
{

cout<<"Podaj Imie : ";
cin>>data[h_m_entries].name;
 cout<<"Podaj Nazwisko : ";
cin>>data[h_m_entries].surname;
 cout<<"Podaj Ulice : ";
cin>>data[h_m_entries].street;
 cout<<"Podaj nr. domu : ";
 cin>>data[h_m_entries].h_number;
 cout<<"Podaj nr. lokalu : ";
 cin>>data[h_m_entries].f_number;
 cout<<"Podaj nr. telefonu : ";
 cin>>data[h_m_entries].p_number;


file.open("ksiazkaadresowa.txt",ios::out | ios::app);
file<<data[h_m_entries].name<<endl;
file<<data[h_m_entries].surname<<endl;
file<<data[h_m_entries].street<<endl;
file<<data[h_m_entries].h_number<<endl;
file<<data[h_m_entries].f_number<<endl;
file<<data[h_m_entries].p_number<<endl;
file.close();

}

}
int whole_reading()
{
string verse;
int verse_number=1;
int registration_number=0;
file.open("ksiazkaadresowa.txt",ios::in);
if(file.good()==false)
{
cout<<"Nie mozna otworzyc pliku!";
exit(0);
}
while (getline(file, verse))
{
switch (verse_number)
{
case 1: data[registration_number].name=verse; break;
case 2: data[registration_number].surname=verse; break;
case 3: data[registration_number].street=verse; break;
case 4: data[registration_number].h_number=atoi(verse.c_str()); break;
case 5: data[registration_number].f_number=atoi(verse.c_str()); break;
case 6: data[registration_number].p_number=atoi(verse.c_str()); break;
}
if (verse_number==6) {verse_number=0; registration_number++;}
verse_number++;
}
file.close();
for(int i=0;i<registration_number;i++)
{
cout<<"imie : " <<data[i].name<<endl;
cout<<"nazwisko : " <<data[i].surname<<endl;
cout<<"ulica : " <<data[i].street<<endl;
cout<<"nr_domu : " <<data[i].h_number<<endl;
cout<<"nr_lokalu : " <<data[i].f_number<<endl;
cout<<"telefon : " <<data[i].p_number<<endl;
}
}
int single_reading()
{
string verse;
int verse_number=1;
int registration_number=0;
int registration=0;
file.open("ksiazkaadresowa.txt",ios::in);
if(file.good()==false)
{
cout<<"Nie mozna otworzyc pliku!";
exit(0);
}
while (getline(file, verse))
{
switch (verse_number)
{
case 1: data[registration_number].name=verse; break;
case 2: data[registration_number].surname=verse; break;
case 3: data[registration_number].street=verse; break;
case 4: data[registration_number].h_number=atoi(verse.c_str()); break;
case 5: data[registration_number].f_number=atoi(verse.c_str()); break;
case 6: data[registration_number].p_number=atoi(verse.c_str()); break;
}
if (verse_number==6) {verse_number=0; verse_number++;}
verse_number++;
}
file.close();
cout<<"Podaj nr. wpisu"<<endl;
cin>>registration;
cout<<"imie : " <<data[registration-1].name<<endl;
cout<<"nazwisko : " <<data[registration-1].surname<<endl;
cout<<"ulica : " <<data[registration-1].street<<endl;
cout<<"nr_domu : " <<data[registration-1].h_number<<endl;
cout<<"nr_lokalu : " <<data[registration-1].f_number<<endl;
cout<<"telefon : " <<data[registration-1].p_number<<endl;
}

0

Tak na szybko zrobiłbym coś podobnego do tego.
Program jest teraz zrobiony na samych strukturach.

#include<iostream>
#include<string>
#include<vector>
#include<fstream>
using namespace std;

struct person
{
    string name;
    string surname;
    unsigned phone;

    person(const string& name, const string& surname, unsigned phone)
        :name(name), surname(surname), phone(phone)
    {
    }
};

struct database { vector<person> persons; };
struct gui { database db; };

void show_person(const person& p)
{
    cout << "Imie: " << p.name << "\nNazwisko: " << p.surname << "\nTelefon: " << p.phone << "\n\n";
}

void add_person(database& db)
{
    string name = "", surname = "";
    unsigned phone = 0;

    cout << "Imie: "; cin >> name;
    cout << "Nazwisko: "; cin >> surname;
    cout << "Telefon: "; cin >> phone;

    db.persons.push_back(person(name, surname, phone));
    cout << "\nDodano nowa osobe.\n\n";
}

void save_persons(const database& db, const char* path)
{
    fstream output;
    output.open(path, ios::out | ios::trunc);
    if (output.is_open())
    {
        for (size_t i = 0; i < db.persons.size(); ++i)
        {
            output << db.persons[i].name << "\n" << db.persons[i].surname << "\n" << db.persons[i].phone << "\n";
        }
        output.close();
    }
}

void load_persons(database& db, const char* path)
{
    fstream input;
    input.open(path, ios::in);
    if (input.is_open())
    {
        db.persons.clear();
        unsigned line_number = 0, item_number = 0;
        string line = "";

        string name = "", surname = "";
        unsigned phone = 0;

        while (getline(input, line))
        {
            switch (item_number)
            {
            case 0:
                name = line;
                break;
            case 1:
                surname = line;
                break;
            case 2:
                phone = atoi(line.c_str());
                break;
            }

            if (item_number == 2)
            {
                db.persons.push_back(person(name, surname, phone));
                item_number = 0;
            }
            else item_number++;
            line_number++;
        }

        input.close();
    }
}

void show_selected_person_from_file(unsigned person_number, const char* path)
{
    fstream input;
    input.open(path, ios::in);
    if (input.is_open())
    {
        unsigned line_number = 0, items_per_person = 3, selected_line = person_number * items_per_person, count = 0;
        string line = "";

        while (getline(input, line))
        {
            if (line_number == selected_line && count < items_per_person)
            {
                switch (count)
                {
                case 0:
                    cout << "Imie: " << line;
                    break;
                case 1:
                    cout << "Nazwisko: " << line;
                    break;
                case 2:
                    cout << "Telefon: " << line;
                    break;
                }

                cout << "\n";
                count++;
                selected_line++;
            }
            line_number++;
        }
        cout << "\n";
        input.close();
    }
}

void show_all_persons(const database& db)
{
    if (db.persons.size() > 0)
    {
        for (size_t i = 0; i < db.persons.size(); ++i)
            show_person(db.persons[i]);
    }
    else cout << "Baza danych jest pusta!\n\n";
}

void show_gui(gui& g)
{
    const char* db_path = "persons.txt";
    char choice = ' ', quit = '6';
    do
    {
        cout << "1. Dodaj osobe;\n2. Wypisz osoby;\n3. Wczytaj osoby;\n4. Zapisz osoby;\n5. Wczytaj osobe o numerze;\n" << quit << ". Wyjdz\n\nWybieram: ";
        cin >> choice;

        switch (choice)
        {
        case '1':
            add_person(g.db);
            break;
        case '2':
            show_all_persons(g.db);
            break;
        case '3':
            load_persons(g.db, db_path);
            break;
        case '4':
            save_persons(g.db, db_path);
            break;
        case '5': {
            unsigned person_number = 0;
            cout << "Numer osoby: "; cin >> person_number;
            show_selected_person_from_file(person_number, db_path);
            break;
        }
        default:
            choice = quit;
            break;
        }
    }
    while (choice != quit);
}

int main()
{
    show_gui(gui());
    return 0;
}

Osoby numerowane są od zera więc najpierw wprowadź ze dwie osoby, zapisz je do pliku i np. wybierz opcję nr 5.
Jak poprosi Cię o wskazanie numeru osoby to np. dla dwóch osób w pliku będzie mógł wybrać 0 lub 1.

vector<person> persons jest pojemnikiem na dane, któremu nie trzeba pilnować rozmiaru.

Uwaga: program nie sprawdza czy plik jest pusty więc pewnie się wywali jak będziesz chciał odczytać osobę nr 2 z pustego pliku. Już mi się tego nie chciało dopisywać.

Format pliku wynikowego to np:

Grzesiek
Programowy
666111222
Tomek
Karolinowy
444111888

PS: z góry przepraszam ceplusplusowców jeżeli popełniłem jakąś herezję :)

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