In generale mi sembra che il codice sia corretto. Ho inserito alcuni commenti su miglioramenti che immagino possano esserti utili per il futuro. Ho inserito una versione con tutti questi commenti alla fine e anche una funzione main per il test.
Se inserisci questo codice all'interno di un file header è necessario fare uso di
inclusion guards oppure di
#pragma once come descritto nella stessa pagina del link. Inoltre tutte le funzioni dovrebbe essere dichiarate inline se la loro definizione è presente nel file header.
È considerata una cattiva pratica quella di usare
using namespace std;, soprattutto in file che devono essere inclusi in altri perché potresti causare dei problemi nel caso qualcuno dichiarasse delle funzioni, classi o variabili con lo stesso nome di funzioni, classi o variabili definiti nelle librerie standard.
Sinceramente dichiarerei la classe
Lista e la classe
ListaNumeri separatamente. Invece l'enumerazione
tipo_numero sembra maggiormente legata alla classe
ListaNumeri in quanto non ha granché senso da sola e la vedrei più come qualcosa di interna.
In versioni moderne del C++ puoi inizializzare le variabili membro nel costruttore come segue:
- Codice:
Lista() : testa(nullptr) { }
In versioni ancora più moderne puoi scrivere invece direttamente
- Codice:
Elem* testa = nullptr;
Puoi avere maggiori informazioni sui costruttori per esempio
in questa pagina mentre
in quest'altra pagina si parla della inizializzazione delle variabili membro. La tua versione non è sbagliata comunque, questo commento è più che altro informativo sulle diverse opzioni disponibili. Nota che ho fatto uso di
nullptr invece di
NULL. Guarda
qui perché.
Il codice della copia mi sembra corretto, tuttavia gestirei al suo interno l'eliminazione del contenuto corrente della lista invece di delegarlo alla funzione che la chiama. Questo perché la funzione copia causerebbe memory leak se non chiamata su una lista vuota. Discorso simile vale per la copia di se stessi.
Puoi inizializzare e creare una struttura come:
- Codice:
// BEFORE
elem* temp = new elem;
temp->num = n;
temp->prox = NULL;
// AFTER
elem* temp = new elem{ n, nullptr };
Non è chiaro perchè
estrai restituisce un intero invece di un valore booleano. Se fai uso di una versione recente del C++, l'alternativa di solito preferibile per
estrai è quella di usare
std::optional.
La tua scelta di operatori da usare è un po' strana. Per esempio
operator! è normalmente usato come NOT LOGICO e ci si aspetta quindi restituisca un valore booleano. Per una lista di solito questo è interpretato come test per vedere se la lista è vuota. Tu invece restituisci la lunghezza della lista. Questo significa che se scrivi:
- Codice:
if (!lista) {
// ...
}
si entra quando
lista->testa != nullptr che è l'opposto di quanto uno si aspetta.
Il costruttore di copia di ListaNumeri non ha alcuna definizione. Neanche l'assegnazione.
Puoi usare il
n % 2 per accedere direttamente alla lista corretta come segue:
- Codice:
liste[n % 2].inserisci(n);
Per quale ragione inserisci restituisce un valore in
ListaNumeri se restituisce sempre lo stesso valore?
Non mi sembra ci sia bisogno di avere
operator<< come
friend della classe.
Stilisticamente credo sarebbe meglio essere più consistenti nel modo in cui dai i nomi ai tipi. Alcuni sono in CamelCase mentre altri sono in snake_case. Farei uso dello stesso stile ovunque. Non importa quale.
In generale, piuttosto che scrivere:
- Codice:
if (condizione) {
// resto della funzione
}
sarebbe meglio scrivere
- Codice:
if (!condizione) {
return;
}
// resto della funzione
È comunque più che altro una scelta stilistica.
- Codice:
#pragma once
#include <iostream>
#include <optional>
//
// Classe Lista
//
class Lista {
struct Elem {
int num;
Elem* prox;
};
Elem* testa = nullptr;
void copia(const Lista&);
void elimina();
public:
Lista() = default;
Lista(const Lista& src) { copia(src); }
virtual ~Lista() { elimina(); }
Lista& operator=(const Lista&);
void inserisci(int);
std::optional<int> estrai();
int somma() const;
int lunghezza() const;
};
inline void Lista::copia(const Lista& src)
{
// Niente da fare se copiamo noi stessi
if (this == &src) {
return;
}
// Rimuoviamo il contenuto corrente della lista
elimina();
// Niente da fare se stiamo copiando una lista vuota
if (src.testa == nullptr) {
return;
}
// Crea il nodo di testa
testa = new Elem{ src.testa->num, nullptr };
// Copia il resto della lista
Elem* temp = testa;
Elem* aux = src.testa;
while (temp != nullptr) {
temp->prox = new Elem{ aux->num, nullptr };
temp = temp->prox;
aux = aux->prox;
}
}
inline void Lista::elimina()
{
Elem* temp = testa;
while (temp != nullptr) {
testa = testa->prox;
delete temp;
temp = testa;
}
}
inline Lista& Lista::operator=(const Lista& src)
{
copia(src);
return *this;
}
// Inserzione in coda.
inline void Lista::inserisci(int n)
{
Elem* temp = new Elem{ n, nullptr };
if (testa == nullptr) {
testa = temp;
} else {
Elem* ultimo = testa;
while (ultimo->prox != nullptr) {
ultimo = ultimo->prox;
}
ultimo->prox = temp;
}
}
// Estrazione in testa.
inline std::optional<int> Lista::estrai()
{
if (testa == nullptr) {
return {};
}
Elem* temp = testa;
testa = testa->prox;
int num_estratto = temp->num;
delete temp;
return { num_estratto };
}
// Restituisce il numero di elementi presenti in una lista.
inline int Lista::lunghezza() const
{
int conta = 0;
for (Elem* temp = testa; temp != nullptr; temp = temp->prox) {
conta++;
}
return conta;
}
// Restituisce la somma di tutti i valori memorizzati nella lista.
inline int Lista::somma() const
{
int somma = 0;
Elem* aux = testa;
while (aux != nullptr) {
somma += aux->num;
aux = aux->prox;
}
return somma;
}
//
// Classe ListaNumeri
//
class ListaNumeri {
// Crea due sottoliste: una per i numeri pari, una per i numeri dispari.
Lista liste[2];
public:
enum TipoNumero { PARI, DISPARI };
ListaNumeri() {}
ListaNumeri(const ListaNumeri& src)
: liste{ src.liste[0], src.liste[1] }
{}
ListaNumeri& operator=(const ListaNumeri& src)
{
liste[0] = src.liste[0];
liste[1] = src.liste[1];
return *this;
}
void inserisci(int);
std::optional<int> estrai(TipoNumero);
int lunghezza() const;
int lunghezza(TipoNumero) const;
int somma(TipoNumero) const;
};
void ListaNumeri::inserisci(int n)
{
liste[n % 2].inserisci(n);
}
inline std::optional<int> ListaNumeri::estrai(TipoNumero tn)
{
return liste[tn].estrai();
}
// Restituisce il numero di elementi contenuti in una determinata sottolista.
inline int ListaNumeri::lunghezza(TipoNumero tn) const
{
return liste[tn].lunghezza();
}
// Restituisce la somma dei valori degli elementi contenuti in una
// delle due sottoliste.
inline int ListaNumeri::somma(TipoNumero tn) const
{
return liste[tn].somma();
}
// Restituisce il numero totale di elementi contenuti nella lista di numeri.
inline int ListaNumeri::lunghezza() const
{
return liste[PARI].lunghezza() + liste[DISPARI].lunghezza();
}
std::ostream& operator<<(std::ostream& os, const ListaNumeri& src)
{
os << '<' << src.lunghezza(ListaNumeri::PARI) << ", " << src.lunghezza(ListaNumeri::DISPARI) << '>';
return os;
}
Il sorgente di test.
- Codice:
#include <iostream>
#include "listaNumeri.hpp"
int main(int argc, char* argv[])
{
ListaNumeri lista;
lista.inserisci(10);
lista.inserisci(12);
lista.inserisci(15);
lista.inserisci(1);
lista.inserisci(34);
std::cout << "EXPECTED: <3, 2>\n";
std::cout << "GOT: " << lista << "\n";
std::cout << "EXPECTED: 56, 16\n";
std::cout << "EXPECTED: " << lista.somma(ListaNumeri::PARI) << ", " << lista.somma(ListaNumeri::DISPARI) << "\n";
lista.estrai(ListaNumeri::DISPARI);
std::cout << "EXPECTED: <3, 1>\n";
std::cout << "GOT: " << lista << "\n";
std::cout << "EXPECTED: 56, 1\n";
std::cout << "EXPECTED: " << lista.somma(ListaNumeri::PARI) << ", " << lista.somma(ListaNumeri::DISPARI) << "\n";
lista.estrai(ListaNumeri::PARI);
lista.estrai(ListaNumeri::PARI);
lista.inserisci(4);
lista.inserisci(3);
std::cout << "EXPECTED: <2, 2>\n";
std::cout << "GOT: " << lista << "\n";
std::cout << "EXPECTED: 38, 4\n";
std::cout << "EXPECTED: " << lista.somma(ListaNumeri::PARI) << ", " << lista.somma(ListaNumeri::DISPARI) << "\n";
}
