Współpraca z plikami txt - jakość mojego kodu

Question

Witam.

Próbuję trochę programować w C++ i opieram się głównie na kursach pana Zelenta - kurs "C++" przerobiłem już generalnie cały, a "Object C++" mniej więcej do połowy. Jestem w trakcie tworzenia pewnego kodu, który ogólnie zdaje się poprawnie działać (używam CodeBlocks), ale zastanawia mnie, czy można go napisać lepiej - dokładniej wygląda to tak:

Chcąc ułatwić sobie współpracę mojego programu z plikami tekstowymi, napisałem klasę zawierającą 2 metody - jedna ma zwrócić liczbę wierszy we wczytanym pliku, a druga - treść wiersza o podanym numerze. Stworzyłem coś takiego:

#include <iostream>
#include <fstream>

using namespace std;

class Pliki {
	public:
	
	fstream plik;
	string name;
	int cykl;
	
	Pliki(string n, int c);
	
	int liczba_w(); //liczba wierszy w pliku
	string tresc_w(int nr); //treść wiersza o podanym numerze w pliku
};

#include <iostream>
#include <fstream>
#include "pliki.h"

using namespace std;

Pliki::Pliki(string n, int c) {
	name=n;
	cykl=c;
}
int Pliki::liczba_w() {
	plik.open(name.c_str(), ios::in);
	int ile=0;
	string linia;
	while(!plik.eof()) {
		getline(plik, linia);
		ile++;
	}
	plik.close();
	return ile;
}
string Pliki::tresc_w(int nr) {
	plik.open(name.c_str(), ios::in);
	int nr_linii=1;
	string linia, tresc;
	while(getline(plik, linia)) {
		if(nr==nr_linii) tresc=linia;
		nr_linii++;
	}
	plik.close();
	return tresc;
}

Skupiając się na tych dwóch metodach - czy można rozwiązać te problemy w lepszy sposób? Chodzi mi o to, że może np. nie znam jakichś funkcji w c++, które można by tutaj wykorzystać albo po prostu któreś fragmenty można by zapisać inaczej.

Comments

Najpierw pozbądź się:

using namespace std;

https://stackoverflow.com/questions/1452721/why-is-using-namespace-std-considered-bad-practice

https://forum.pasja-informatyki.pl/536978/using-namespace-std#a536980

---

Dzięki za radę. Dość mocno przyzwyczaiłem się do stosowania takiej właśnie linii. Po jej usunięciu, idąc za komunikatami błędów, dodałem std:: przed wszystkimi wystąpieniami: fstream, string oraz ios::in. Kompiluje się ponownie.

---

Klasy w c++ dają coś takiego jak hermetyzacja, poczytaj o tym. Nazwy, jeżeli potrzebujesz komentarza, by coś wyjaśnić, to znaczy, że Twoja nazwa jest zła.

Answer 1

czy można rozwiązać te problemy w lepszy sposób?

Tak. Wczytaj w konstruktorze cały plik do std::vector po to, by nie czytać całego pliku za każdym razem, gdy będzie prośba o podanie liczby wierszy (już nie mówiąc o czytaniu przy pomocy tej klasy wszystkich linii w jakiejś pętli).

Zmień też nazwę, bo ta nie ma związku z przeznaczeniem klasy.

 while(!plik.eof()) {
        getline(plik, linia);

Przy takim czytaniu zawsze będziesz miał gratis jedną nadmiarową pustą linię. Tak jest poprawnie:

while(getline(plik, linia)) {
    //...
}

 

Comments

Dzięki (w sumie dość późne) za wskazówki. Zmieniłem mój kod na widoczny poniżej. Na razie nie ustawiałem hermetyzacji, ale planuję się tym zainteresować.

#include <iostream>
#include <vector>
#include <fstream>

class Plik_txt {
	public:
	
	std::vector<std::string> vec;
	std::fstream plik;
	std::string name;
	int cykl;
	
	Plik_txt(std::string n, int c);
};

#include <iostream>
#include "plik_txt.h"

Plik_txt::Plik_txt(std::string n, int c) {
	name=n;
	cykl=c;
	plik.open(name.c_str(), std::ios::in);
	std::string linia;
	while(getline(plik, linia)) {
		vec.push_back(linia);
	}
	plik.close();
}

Jak widać, usunąłem te dwie metody, ponieważ teraz do ich wyników mogę się dostać za pomocą pojedynczych instrukcji. Liczbę wierszy w pliku dostanę stosując funkcję size() na tablicy typu std::vector (u mnie by było: vec.size()), a treść danego wiersza stanowi odpowiednia szufladka tej tablicy, np. wiersz nr 1 to vec[0]. Fajnie, że udało się to tak uprościć :)

Zmień też nazwę, bo ta nie ma związku z przeznaczeniem klasy

W tym przypadku bez pełnego przekonania zmieniłem nazwę na Plik_txt. Pomyślałem sobie bowiem, że nazwa powinna wyjaśniać to, co jakby stanowi obiekt tworzony z danej klasy. Tutaj obiekt łączy się z plikiem tekstowym i generalnie przechowuje jego nazwę i zawartość, stąd taka nazwa (nie chciałem, żeby była za długa). Też byście tak tą klasę nazwali, czy może jakoś inaczej?

Inna sprawa, że ostatnio staram się stosować angielskie nazwy, więc tutaj by było np. File_txt.

---

• Przenieś linię  std::fstream plik; do konstruktora. Nie ma powodu, by ta zmienna była polem klasy.
• Jeśli używasz strumienia tylko do odczytu, użyj std::ifstream.
• Usuń linię z  plik.close();, bo jest zbędna - mechanizm RAII - jeśli zastosujesz się do punktu pierwszego - sam to zrobi.