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.
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
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ę.
Potknięcia:
- Niepotrzebne zmienne globalne;
- Czemu tak
while(wybor == 1|| wybor == 2 || wybor == 3);
zamiastwhile(wybor != 4);
? - Fatalne formatowanie;
- Polskie nazwy obiektów, funkcji etc.;
- Niepotrzebny
#include <cstdlib>
; - Masz miejsce na 20 książek
ksiazka dane[20]
, a nigdzie nie pilnujesz ile książek dodał użytkownik; - Tak samo jak nie pilnujesz ile książek zostanie zaczytanych z pliku:
while (getline(plik, linia))
; - Powtarzasz się w kodzie. Powtarzające się rzeczy powsadzaj do funkcji, bo funkcje
odczyt_pojedynczy
iodczyt_caly
są niemal identyczne; - 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; - Prototypy funkcji powyżej maina niepotrzebne;
@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:
- Zauważ jak mały jest
main
; - 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... - Mamy
vector
czyli taki pojemnik na dane, który nie ogranicza Cie odgórnie ustalonym rozmiarem.
grzesiek51114 napisał(a):
Potknięcia:
- Niepotrzebne zmienne globalne;
- Czemu tak
while(wybor == 1|| wybor == 2 || wybor == 3);
zamiastwhile(wybor != 4);
?- Fatalne formatowanie;
- Polskie nazwy obiektów, funkcji etc.;
- Niepotrzebny
#include <cstdlib>
;- Masz miejsce na 20 książek
ksiazka dane[20]
, a nigdzie nie pilnujesz ile książek dodał użytkownik;- Tak samo jak nie pilnujesz ile książek zostanie zaczytanych z pliku:
while (getline(plik, linia))
;- Powtarzasz się w kodzie. Powtarzające się rzeczy powsadzaj do funkcji, bo funkcje
odczyt_pojedynczy
iodczyt_caly
są niemal identyczne;- 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;- 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;
}
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ę :)