[C++] Ottimizzare l'allocazione delle stringhe

[MaStErYuRi]

Sto lavorando a questa classe Persona. Il problema è che vorrei ottimizzare l'allocazione delle stringhe Nome e Cognome. Nel costruttore senza argomenti ho riscontrato che devo necessariamente stabilire un valore massimo di elementi, nella fattispecie MAX, altrimenti se inserisco un nome od un cognome di una lunghezza superiore ai 16 caratteri (credo sia uno standard) il programma va in pappa, e le stringhe si sovrappongono. Però mi interesserebbe un'allocazione più intelligente, come per i costruttori che ho scritto, sia quello di copia che quello con gli argomenti. Un altro problema è che se alloco un oggetto con il costruttore con argomenti, ed applico su di esso la cin >>, se le stringhe che inserisco da tastiera sono più lunghe di quelle inserite come argomenti, ritorno allo stesso problema precedente.

Spero di essere stato chiaro, se servono altri chiarimenti chiedete.
Grazie anticipatamente per l'aiuto.

PS Ho anche avanzato l'ipotesi di aver sbagliato completamente l'impostazione della progettazione e che va riveduta dal principio, quindi nel caso, non abbiate paura di dirmelo. A patto che però mi consigliate una soluzione. :)

#ifndef PERSONA_H
#define PERSONA_H

#include <iostream>
using namespace std;

class Persona
{
	friend istream& operator >> (istream& in, Persona& P);
	friend ostream& operator << (ostream& out, const Persona& P);

	public:
	Persona();
	Persona(const Persona &);
	Persona(const char *, const char *, const int);
	const char * getN() const;
	const char * getC() const;
	const int getE() const;
	~Persona();

	private:
	char * Nome;
	char * Cognome;
	int Eta;
};

#endif

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

using namespace std;

#define MAX 100

Persona::Persona() //Da migliorare, per ottimizzare l'occupazione di memoria
{
	char s [] = "";
	Nome = new char[MAX];
	Cognome = new char[MAX];
	strcpy(Nome,s);
	strcpy(Cognome,s);
	Eta=0;
}


Persona::Persona(const Persona & P)
{
	Nome = new char [strlen(P.getN()+1)];
	Cognome = new char [strlen(P.getC())+1];
	strcpy(Nome,P.getN());
	strcpy(Cognome,P.getC());
	Eta=P.getE();
}

Persona::Persona(const char * N, const char * C, const int E)
{
	Nome = new char [strlen(N)+1];
	Cognome = new char [strlen(C)+1];
	strcpy(Nome,N);
	strcpy(Cognome,C);
	Eta=E;
}

Persona::~Persona()
{
	delete [] Nome;
	delete [] Cognome;
}

const char * Persona::getN() const
{
	return Nome;
}

const char * Persona::getC() const
{
	return Cognome;
}

const int Persona::getE() const
{
	return Eta;
}

ostream& operator << (ostream& out, const Persona& P)
{
	out << P.Nome << " " << P.Cognome << " " << P.Eta << " anni" << endl;
	return out;
}

istream& operator >> (istream& in, Persona& P)
{
	char s1[MAX];
	char s2[MAX];
	cout << "Inserire Nome: "; in.getline(s1,MAX);
	cout << "Inserire Cognome: "; in.getline(s2,MAX);
	cout << "Inserire età: "; in >> P.Eta;
	strcpy(P.Nome,s1);
	strcpy(P.Cognome,s2);
	
	return in;
}

[MItaly]

Usa la classe std::string e dimenticati di questi problemi.

[sebaldar]

concordo con MItaly riguardo l'uso di str::string, altrimenti devi riallocare le stringhe ogni volta

istream& operator >> (istream& in, Persona& P)
{

	char str[255];  // di appoggio
	
	cout << "Inserire Nome: ";
	cin.getline ( str, 255 );
	delete [] P.Nome;
	P.Nome = new char [ strlen( str ) + 1 ];
	strcpy ( P.Nome, str );

	cout << "Inserire Cognome: "; 
	cin.getline ( str, 255 );
	delete [] P.Cognome;
	P.Cognome = new char [ strlen( str ) + 1 ];
	strcpy ( P.Cognome, str );

	cout << "Inserire età: ";
        in >> P.Eta;
	
	return *this;

}

[MaStErYuRi]

Si, ci avevo pensato, però essendo un esercizio didattico, non mi andava di semplificarmi la vita, tra l'altro in questo modo si capiscono più cose. Ad ogni modo nella nottata pare che abbia trovato una soluzione.

Persona::Persona()
{
	char s [] = "";
	Nome = new char;
	Cognome = new char;
	strcpy(Nome,s);
	strcpy(Cognome,s);
	Eta=0;
}

istream& operator >> (istream& in, Persona& P)
{
	delete [] P.Nome;
	delete [] P.Cognome;
	
	char s1[MAX];
	char s2[MAX];

	cout << "Inserire Nome: "; in.getline(s1,MAX);
	cout << "Inserire Cognome: "; in.getline(s2,MAX);
	cout << "Inserire età: "; in >> P.Eta;
	
	P.Nome = new char [strlen(s1)+1];
	P.Cognome = new char [strlen(s2)+1];

	strcpy(P.Nome,s1);
	strcpy(P.Cognome,s2);
	
	return in;
}

Ovvero, ogni volta che uso la cin dealloco P.Nome e P.Cognome e le rialloco secondo la lunghezza giusta, servendomi di s1 ed s2 che, si, hanno lunghezza MAX, ma vengono deallocate alla fine del blocco ogni volta. Che ne pensate?

[MItaly]

Penso che la tua classe in quella maniera viola il principio di singola responsabilità (vuole contemporaneamente rappresentare una persona e gestire la memoria delle stringhe), non è exception-safe (se la seconda new fallisce viene sollevata una std::bac_alloc, ma la memoria associata a P.Nome non viene liberata), ha una certa dose di codice duplicato, gestisce la memoria in maniera poco efficiente (continui ad allocare/deallocare roba) e il continuo intervallarsi di codice per gestire la memoria delle stringhe e codice della classe vero e proprio rende la lettura e la verifica del codice difficoltosa.

Se vuoi imparare davvero, scriviti una classe stringa per bene, e poi usala all'interno di questa tua classe Persona, in modo da separare i due compiti; naturalmente nei progetti veri in genere userai la classe string della libreria standard, che sicuramente è più collaudata della tua.

[MaStErYuRi]

Grazie della risposta, molto esauriente. Ora provo ad utilizzare la classe string della libreria standard, se tutto va bene magari proverò anche a scriverla io.

[MaStErYuRi]

Non so come operare con la classe string, non ci avevo pensato ma sorgono i problemi dei parametri che devi passare alle funzioni, ecc...

non hai un esempio di classe del genere che gestisca stringhe?
per esempio persona, libro, ecc...

[MaStErYuRi]

Ho riscritto la classe, così sembra andare. L'unico problema (spero l'unico) è il distruttore. Non so come comportarmi.

Se do

 delete [] nomestringa

Il compilatore non riconosce nomestringa come un puntatore, come risolvo?

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

using namespace std;

Persona::Persona()
{
	Nome="";
	Cognome="";
	Eta=0;
}


Persona::Persona(const Persona & P)
{
	string Nome(P.getN());
	string Cognome(P.getC());
	int Eta=P.getE();
}

Persona::Persona(const string N, const string C, const int E)
{
	Nome=N;
	Cognome=C;
	Eta=E;
}

Persona::~Persona()
{
	
}

const Persona& Persona::operator = (const Persona& P)
{
	if(this!=&P)
	{
		Nome = P.getN();
		Cognome = P.getC();
		Eta=P.getE();

		return *this;
	}
}

const string Persona::getN() const
{
	return Nome;
}

const string Persona::getC() const
{
	return Cognome;
}

const int Persona::getE() const
{
	return Eta;
}

ostream& operator << (ostream& out, const Persona& P)
{
	out << P.Nome << " " << P.Cognome << " " << P.Eta << " anni" << endl;
	return out;
}

istream& operator >> (istream& in, Persona& P)
{
	cout << "Inserire Nome: "; getline(in,P.Nome);
	cout << "Inserire Cognome: "; getline(in,P.Cognome);
	cout << "Inserire età: "; in >> P.Eta;
	
	return in;
}

EDIT:
Come non detto, ho trovato nomestringa.clear() che dovrebbe fare al caso mio.
Ad ogni modo ditemi se ora la classe è più decente

[MItaly]

#ifndef PERSONA_H
#define PERSONA_H

#include <iostream>
// using namespace std; <-- *mai* istruzioni using negli header

class Persona
{
    istream& operator >> (istream& in, Persona& P);
    ostream& operator << (ostream& out, const Persona& P);

public:
    Persona();
    Persona(const Persona & );
    Persona(const std::string & Nome, const std::string & Cognome, int Eta); // non ha senso dichiarare un parametro non-puntatore e non-reference come const
    
    // I getter si possono tranquillamente lasciare inline
    const std::string & GetNome() const { return nome; };
    const std::string & GetCognome() const { return cognome; };
    const int GetEta() const {return eta; };
    
    ~Persona();

private:
    std::string nome;
    std::string cognome;
    int eta;
};

#endif

#include <iostream>
#include <stdexcept>

#include "Persona.h"

using namespace std;

// in effetti è inutile, basterebbe quello di default
// qui uso la sintassi dell'initializer list per costruire i campi direttamente
// con i valori desiderati, invece di farli costruire con il costruttore di default
// e poi fare assegnamenti nel corpo della funzione
Persona::Persona(const Persona & Right)
    : nome(Right.GetNome()),
    cognome(Right.GetCognome()),
    eta(Right.GetEta())
{

}

Persona::Persona()
    : eta(0)
{
}

Persona(const std::string & Nome, const std::string & Cognome, int Eta)
    : nome(Nome),
    cognome(Cognome),
    eta(Eta)
{
    if(Eta<0)
        throw std::invalid_argument("Eta deve essere positivo.");
}

Persona::~Persona()
{
    // inutile
}

ostream& operator << (ostream& out, const Persona& P)
{
    out << P.Nome << " " << P.Cognome << " " << P.Eta << " anni" << endl;
    return out;
}

istream& operator >> (istream& in, Persona& P)
{
    cout << "Inserire Nome: ";
    getline(in, P.nome);
    cout << "Inserire Cognome: ";
    getline(in, P.cognome);
    cout << "Inserire età: ";
    in >> P.Eta;
    return in;
}

Nota che nell'operatore di input c'è ancora un problema, a te individuarlo.

[MaStErYuRi]

Grazie davvero per la risposta, chiedo scusa per il ritardo ma solo ora ho potuto riprendere.
Ad ogni modo ho qualche altro piccolo dubbio.


Una funzione che non sta ne in una sezione public ne in una private, che tipo di funzione è? Questa scelta quali implicazioni ha sull'accessibliità?

Inoltre

#include <stdexcept>

serve per

throw std::invalid_argument("Eta deve essere positivo.");

???