LinkedList completata e classi errori + modifiche minime classi michele #14
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Here's the translation of the review guide to Italian: Guida per il Revisore di SourceryQuesta pull request completa l'implementazione della LinkedList e introduce classi di gestione degli errori. Include inoltre modifiche minori alle classi esistenti. Diagramma di sequenza per il processo di rimozione del dispositivosequenceDiagram
participant Client
participant LinkedList
participant Node
participant Device
Client->>LinkedList: removeDispositivo(nome)
alt List is empty
LinkedList-->>Client: throw ListaVuotaException
else List not empty
LinkedList->>LinkedList: searchDispositivo(nome)
alt Device not found
LinkedList-->>Client: throw DispositivoNonTrovatoException
else Device found
LinkedList->>Node: Update node connections
LinkedList->>Node: Delete node
LinkedList-->>Client: Success
end
end
Diagramma delle classi per LinkedList e classi di eccezioniclassDiagram
class LinkedList {
-Node* head
-Node* tail
+LinkedList()
+LinkedList(Dispositivo& dispositivo)
+void insert(Dispositivo& dispositivo)
+void removeDispositivo(string nome)
+vector~Dispositivo*~ removeAllDispositiviOff(int currentTime)
+void removeTimer(string nome)
+bool isEmpty()
-Node* searchDispositivo(string nome)
-void connectBefore(Node* p, Node* q)
}
class Node {
+Dispositivo* disp
+Node* prev
+Node* next
+Node(Dispositivo& data)
+Node(const Node& data)
+Node& operator=(const Node& data)
}
class DispositivoNonTrovatoException {
+DispositivoNonTrovatoException()
}
class ListaVuotaException {
+ListaVuotaException()
}
LinkedList *-- Node
DispositivoNonTrovatoException --|> invalid_argument
ListaVuotaException --|> out_of_range
note for LinkedList "Implementazione di una lista doppiamente linkata per la gestione dei dispositivi"
note for Node "Classe nodo interna per la lista linkata"
note for DispositivoNonTrovatoException "Sollevata quando un dispositivo non viene trovato"
note for ListaVuotaException "Sollevata quando la lista è vuota"
Modifiche a Livello di File
Suggerimenti e comandiInterazione con Sourcery
Personalizzazione della Tua EsperienzaAccedi alla tua dashboard per:
Ottenere Aiuto
Original review guide in EnglishReviewer's Guide by SourceryThis pull request completes the LinkedList implementation and introduces error handling classes. It also includes minor modifications to existing classes. Sequence diagram for device removal processsequenceDiagram
participant Client
participant LinkedList
participant Node
participant Device
Client->>LinkedList: removeDispositivo(nome)
alt List is empty
LinkedList-->>Client: throw ListaVuotaException
else List not empty
LinkedList->>LinkedList: searchDispositivo(nome)
alt Device not found
LinkedList-->>Client: throw DispositivoNonTrovatoException
else Device found
LinkedList->>Node: Update node connections
LinkedList->>Node: Delete node
LinkedList-->>Client: Success
end
end
Class diagram for LinkedList and Exception classesclassDiagram
class LinkedList {
-Node* head
-Node* tail
+LinkedList()
+LinkedList(Dispositivo& dispositivo)
+void insert(Dispositivo& dispositivo)
+void removeDispositivo(string nome)
+vector~Dispositivo*~ removeAllDispositiviOff(int currentTime)
+void removeTimer(string nome)
+bool isEmpty()
-Node* searchDispositivo(string nome)
-void connectBefore(Node* p, Node* q)
}
class Node {
+Dispositivo* disp
+Node* prev
+Node* next
+Node(Dispositivo& data)
+Node(const Node& data)
+Node& operator=(const Node& data)
}
class DispositivoNonTrovatoException {
+DispositivoNonTrovatoException()
}
class ListaVuotaException {
+ListaVuotaException()
}
LinkedList *-- Node
DispositivoNonTrovatoException --|> invalid_argument
ListaVuotaException --|> out_of_range
note for LinkedList "Doubly linked list implementation for managing devices"
note for Node "Internal node class for the linked list"
note for DispositivoNonTrovatoException "Thrown when a device is not found"
note for ListaVuotaException "Thrown when list is empty"
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Ciao @MochaSummer2588 - Ho esaminato le tue modifiche - ecco alcuni feedback:
Commenti generali:
- Alla classe LinkedList mancano le implementazioni del costruttore di copia e dell'operatore di assegnazione, il che potrebbe causare problemi di memoria con le copie superficiali. Considera di implementarli o di eliminarli esplicitamente.
- Ci sono diversi errori di battitura nei nomi dei dispositivi in mainNode.cpp (ad es. 'Frigrifero', 'Lavtrice', 'Televisre'). Per favore, correggili per garantire coerenza.
Ecco cosa ho esaminato durante la revisione
- 🟡 Problemi generali: 3 problemi trovati
- 🟢 Sicurezza: tutto a posto
- 🟢 Test: tutto a posto
- 🟡 Complessità: 1 problema trovato
- 🟢 Documentazione: tutto a posto
La tua prova scade l'11 gennaio 2025. Per favore aggiorna per continuare a utilizzare Sourcery ✨
Aiutami a essere più utile! Per favore, fai clic su 👍 o 👎 su ogni commento e userò il feedback per migliorare le tue revisioni.Original comment in English
Hey @MochaSummer2588 - I've reviewed your changes - here's some feedback:
Overall Comments:
- The LinkedList class is missing copy constructor and assignment operator implementations, which could lead to memory issues with shallow copies. Consider implementing these or explicitly deleting them.
- There are several typos in device names in mainNode.cpp (e.g., 'Frigrifero', 'Lavtrice', 'Televisre'). Please correct these for consistency.
Here's what I looked at during the review
- 🟡 General issues: 3 issues found
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟡 Complexity: 1 issue found
- 🟢 Documentation: all looks good
Your trial expires on January 11, 2025. Please upgrade to continue using Sourcery ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
src/LinkedList.cpp
Outdated
| } | ||
| } | ||
|
|
||
| void LinkedList::removeDispositivo(const string nome) |
There was a problem hiding this comment.
problema: Il metodo removeDispositivo deve gestire il caso in cui la lista ha esattamente un nodo (head == tail).
Attualmente, quando head == tail, tentare di accedere a head->next causerà un dereferenziamento di puntatore nullo. Aggiungi un caso separato per gestire questa situazione.
Original comment in English
issue: The removeDispositivo method needs to handle the case where the list has exactly one node (head == tail).
Currently, when head == tail, attempting to access head->next will cause a null pointer dereference. Add a separate case to handle this situation.
Comment on lines
+89
to
+91
| vector<Dispositivo*> LinkedList::removeAllDispositiviOff(const int currentTime) | ||
| { | ||
| if(isEmpty()) |
There was a problem hiding this comment.
suggerimento (prestazioni): removeAllDispositiviOff ha una complessità O(n²) a causa di attraversamenti di liste nidificati.
Considera di ristrutturare per evitare di chiamare removeDispositivo all'interno del ciclo, che causa attraversamenti aggiuntivi della lista.
Suggested change
| vector<Dispositivo*> LinkedList::removeAllDispositiviOff(const int currentTime) | |
| { | |
| if(isEmpty()) | |
| vector<Dispositivo*> LinkedList::removeAllDispositiviOff(const int currentTime) | |
| { | |
| vector<Dispositivo*> removedDevices; | |
| if(isEmpty()) { | |
| return removedDevices; | |
| } | |
| vector<Node*> nodesToRemove; | |
| Node* current = head; | |
| // Primo passaggio: raccogli tutti i nodi da rimuovere | |
| while(current != nullptr) { | |
| if(!current->disp->isOn(currentTime)) { | |
| nodesToRemove.push_back(current); | |
| removedDevices.push_back(current->disp); | |
| } | |
| current = current->next; | |
| } | |
| // Secondo passaggio: rimuovi i nodi raccolti | |
| for(Node* nodeToRemove : nodesToRemove) { | |
| if(nodeToRemove == head) { | |
| head = nodeToRemove->next; | |
| } | |
| if(nodeToRemove == tail) { | |
| tail = nodeToRemove->prev; | |
| } | |
| if(nodeToRemove->prev) { | |
| nodeToRemove->prev->next = nodeToRemove->next; | |
| } | |
| if(nodeToRemove->next) { | |
| nodeToRemove->next->prev = nodeToRemove->prev; | |
| } | |
| delete nodeToRemove; | |
| } | |
| return removedDevices; |
Original comment in English
suggestion (performance): removeAllDispositiviOff has O(n²) complexity due to nested list traversals.
Consider restructuring to avoid calling removeDispositivo inside the loop, which causes additional list traversals.
Suggested change
| vector<Dispositivo*> LinkedList::removeAllDispositiviOff(const int currentTime) | |
| { | |
| if(isEmpty()) | |
| vector<Dispositivo*> LinkedList::removeAllDispositiviOff(const int currentTime) | |
| { | |
| vector<Dispositivo*> removedDevices; | |
| if(isEmpty()) { | |
| return removedDevices; | |
| } | |
| vector<Node*> nodesToRemove; | |
| Node* current = head; | |
| // First pass: collect all nodes to remove | |
| while(current != nullptr) { | |
| if(!current->disp->isOn(currentTime)) { | |
| nodesToRemove.push_back(current); | |
| removedDevices.push_back(current->disp); | |
| } | |
| current = current->next; | |
| } | |
| // Second pass: remove collected nodes | |
| for(Node* nodeToRemove : nodesToRemove) { | |
| if(nodeToRemove == head) { | |
| head = nodeToRemove->next; | |
| } | |
| if(nodeToRemove == tail) { | |
| tail = nodeToRemove->prev; | |
| } | |
| if(nodeToRemove->prev) { | |
| nodeToRemove->prev->next = nodeToRemove->next; | |
| } | |
| if(nodeToRemove->next) { | |
| nodeToRemove->next->prev = nodeToRemove->prev; | |
| } | |
| delete nodeToRemove; | |
| } | |
| return removedDevices; |
Comment on lines
+9
to
+10
| Dispositivo Frigorifero = CreaDispositivo::creaDispositivo("Frigrifero", 76); //Manuale | ||
| Dispositivo Lavatrice = CreaDispositivo::creaDispositivo("Lavtrice", 8); //110 min |
There was a problem hiding this comment.
problema (errore di battitura): I nomi dei dispositivi contengono errori di battitura che potrebbero causare errori di ricerca.
"Frigrifero" dovrebbe essere "Frigorifero" e "Lavtrice" dovrebbe essere "Lavatrice". Questi errori di battitura causeranno errori di ricerca dei dispositivi.
Suggested change
| Dispositivo Frigorifero = CreaDispositivo::creaDispositivo("Frigrifero", 76); //Manuale | |
| Dispositivo Lavatrice = CreaDispositivo::creaDispositivo("Lavtrice", 8); //110 min | |
| Dispositivo Frigorifero = CreaDispositivo::creaDispositivo("Frigorifero", 76); //Manuale | |
| Dispositivo Lavatrice = CreaDispositivo::creaDispositivo("Lavatrice", 8); //110 min |
Original comment in English
issue (typo): Device names contain typos that could cause lookup failures.
"Frigrifero" should be "Frigorifero" and "Lavtrice" should be "Lavatrice". These typos will cause device lookup failures.
Suggested change
| Dispositivo Frigorifero = CreaDispositivo::creaDispositivo("Frigrifero", 76); //Manuale | |
| Dispositivo Lavatrice = CreaDispositivo::creaDispositivo("Lavtrice", 8); //110 min | |
| Dispositivo Frigorifero = CreaDispositivo::creaDispositivo("Frigorifero", 76); //Manuale | |
| Dispositivo Lavatrice = CreaDispositivo::creaDispositivo("Lavatrice", 8); //110 min |
| @@ -0,0 +1,194 @@ | |||
| #include "../include/LinkedList.h" | |||
There was a problem hiding this comment.
problema (complessità): Considera di semplificare i metodi insert() e remove() unificando la logica di manipolazione dei puntatori in un unico metodo connectNodes(), riducendo la gestione dei casi speciali e migliorando la manutenibilità.
I metodi insert() e remove() possono essere semplificati unificando la logica di manipolazione dei puntatori. Ecco come ridurre la gestione dei casi speciali:
- Estendi connectBefore() per gestire tutti gli aggiornamenti dei puntatori:
void LinkedList::connectNodes(Node* prev, Node* curr, Node* next) {
if (prev) prev->next = curr;
else head = curr;
if (next) next->prev = curr;
else tail = curr;
curr->prev = prev;
curr->next = next;
}- Semplifica insert() usando l'approccio unificato:
void LinkedList::insert(Dispositivo& dispositivo) {
Node* newNode = new Node(dispositivo);
if (isEmpty()) {
connectNodes(nullptr, newNode, nullptr);
return;
}
Node* current = head;
while (current && current->disp->getOrarioAccensione() <= dispositivo.getOrarioAccensione()) {
current = current->next;
}
connectNodes(current ? current->prev : tail, newNode, current);
}- Semplifica allo stesso modo remove():
void LinkedList::removeDispositivo(const string nome) {
if (isEmpty()) throw ListaVuotaException();
Node* current = searchDispositivo(nome);
connectNodes(current->prev, current->next, current->next);
delete current;
}Questo approccio:
- Elimina la logica duplicata di manipolazione dei puntatori
- Riduce la gestione dei casi speciali
- Rende il codice più manutenibile
- Preserva tutte le funzionalità
Original comment in English
issue (complexity): Consider simplifying the insert() and remove() methods by unifying the pointer manipulation logic into a single connectNodes() method, reducing special case handling and improving maintainability.
The insert() and remove() methods can be simplified by unifying the pointer manipulation logic. Here's how to reduce the special case handling:
- Extend connectBefore() to handle all pointer updates:
void LinkedList::connectNodes(Node* prev, Node* curr, Node* next) {
if (prev) prev->next = curr;
else head = curr;
if (next) next->prev = curr;
else tail = curr;
curr->prev = prev;
curr->next = next;
}- Simplify insert() using the unified approach:
void LinkedList::insert(Dispositivo& dispositivo) {
Node* newNode = new Node(dispositivo);
if (isEmpty()) {
connectNodes(nullptr, newNode, nullptr);
return;
}
Node* current = head;
while (current && current->disp->getOrarioAccensione() <= dispositivo.getOrarioAccensione()) {
current = current->next;
}
connectNodes(current ? current->prev : tail, newNode, current);
}- Similarly simplify remove():
void LinkedList::removeDispositivo(const string nome) {
if (isEmpty()) throw ListaVuotaException();
Node* current = searchDispositivo(nome);
connectNodes(current->prev, current->next, current->next);
delete current;
}This approach:
- Eliminates duplicate pointer manipulation logic
- Reduces special case handling
- Makes the code more maintainable
- Preserves all functionality
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Sommario di Sourcery
Implementare la classe LinkedList e aggiungere classi di eccezioni per liste vuote ed errori di dispositivo non trovato. Aggiornare il tempo di spegnimento predefinito della TV a 60 minuti.
Nuove Funzionalità:
Test:
Original summary in English
Summary by Sourcery
Implement the LinkedList class and add exception classes for empty list and device not found errors. Update the default shutdown time for the TV to 60 minutes.
New Features:
Tests: