Implementación INT utilizando listas vinculadas -- ++ campo con linked-list camp codereview Relacionados El problema

Int implementation using linked lists


3
vote

problema

Español

Esta es la primera pieza de código que he escrito por mi cuenta desde que comencé a aprender hace un par de meses. El código funciona y toma un 99887766555443311

Antes de seguir adelante, se apreciaría considerablemente si estoy haciendo algo mal, si mi código está debidamente estructurado o si estoy siguiendo un estilo bueno / malo.

  def bubbleSort(alist):     n = len(alist)     for i in range(n, 1, -1):         for j in range(i-1):             if alist[j] > alist[j+1] :                 alist[j],alist[j+1] = alist[j+1],alist[j] 2 

Original en ingles

This is the first piece of code I've written on my own since I started learning a couple of months ago. The code works and it takes an int as input and creates a linked list with each node representing a digit. I've added a function that sums two linked listed ints and returns a third list with the result. The rest of the code is also distributed in functions.

Before I move on, it would be greatly appreciated to know if I'm doing something wrong, if my code is properly structured or if I'm following a good/bad style.

#include <iostream>  using std::cout; using std::cin;  struct intNode {     int num;     intNode *next; };  int deallocateList(intNode *&s); int intConversionHelper(int n); int lenList(intNode *&s); int sumListContent(intNode *&s); intNode *sumLists(intNode *&s1, intNode *&s2); int printNode(intNode *&s); int intLen(int n); intNode *intToList(int n); void append(intNode *&s, int n);    int main() {     int num1;     int num2;     cout << "Give me number 1: ";     cin >> num1;      cout << "Give me number 2: ";     cin >> num2;      intNode *list1 = intToList(num1);     intNode *list2 = intToList(num2);     cout << "\nPrinting ints as linked lists: \n";     printNode(list1);     printNode(list2);      cout << "\nSum of the two lists (as a list too!)\n";     intNode *sumList = sumLists(list1, list2);     printNode(sumList);      deallocateList(list1);     deallocateList(list2);     deallocateList(sumList); }  int printNode(intNode *&s) {     intNode *tmp = s;      while (tmp != NULL) {         cout << tmp->num;         tmp = tmp->next;     }     cout << "\n"; }  int intLen(int n) {     int len = 1;     while (n > 9) {         len++;         n /= 10;     }     return len; }  intNode *intToList(int n) {     intNode *intList = NULL;     int num;     int dec = 10;      num = n % 10;     append(intList, num);     int count = 1;     while (count < intLen(n)) {         num = n / dec % 10;          dec = dec * 10;         append(intList, num);         count++;     }     return intList;      }  void append(intNode *&s, int n) {     if (s == NULL) {         s = new intNode;         s->num = n;         s->next = NULL;     }     else {         intNode *nNode = new intNode;         nNode->num = n;         nNode->next = s;         s = nNode;     } }  intNode *sumLists(intNode *&s1, intNode *&s2) {     int sum1 = sumListContent(s1);     int sum2 = sumListContent(s2);     return intToList(sum1 + sum2); }  int sumListContent(intNode *&s) {     intNode *tmp = s;     int sum = 0;     int len = lenList(s);     while (tmp != NULL) {         sum += intConversionHelper(len) * tmp->num;         len--;         tmp = tmp->next;     }     return sum; }  int lenList(intNode *&s) {     intNode *tmp = s;     int len = 0;     while (tmp != NULL) {         len++;         tmp = tmp->next;     }     return len; }  int intConversionHelper(int n) {     float value = n;     for (int i = 1; i < n; i++) {     value = value * 10;     }     value = value / n;     return value; }  int deallocateList(intNode *&s) {     intNode *tmp = s;     while (s != NULL) {         tmp = tmp->next;         delete[] s;         s = tmp;     } } 
     
     
     

Lista de respuestas

6
 
vote
vote
La mejor respuesta
 

Aquí hay algunas observaciones y sugerencias que pueden ayudarlo a mejorar su código.

Uso de objetos

El programa se escribe mucho más en el estilo de procedimiento de C en lugar de en el estilo orientado a objetos de C ++. La lista en sí podría ser un objeto (como podría los nodos individuales), con la mayoría de los procedimientos como funciones de ese objeto. Esto reduciría el acoplamiento y facilitaría la comprensión del programa. No menos importante, en lugar de requerir que el usuario llame explícitamente deallocateList() , el destructor del objeto podría llamarse automáticamente.

Asegúrese de que todas las rutas devuelvan un valor

Tanto el printNode() y deallocateList()2 Funciones Reclamar para devolver un int Pero ninguno de ellos realmente lo hacen. Devuelve un valor 99887766555443344 o cambie la firma de la función a void .

Match new y delete

Como se mencionó en comentarios a esta pregunta, new8 debe coincidir con delete y no 99887776655443310 es una operación diferente.

Usa MEJOR NAMING

La función llamada printNode()1 no es bien nombrado. Un mejor nombre puede ser printNode()2 ya que en realidad imprime la lista completa y no solo un solo nodo.

Evite usar los punteros crudos

En C ++ moderno, no tendemos a usar los punteros crudos tanto como en C. En su lugar, usamos punteros u objetos inteligentes. En este caso, diría que si creó objetos, sería mejor devolver un objeto de 99887776655443313 y 99887766555443314 en lugar de un puntero a un nodo.

Uso printNode()5 Donde sea práctico

En casi todas sus funciones, un parámetro es una referencia a un puntero a un printNode()6 es el nodo inicial de una lista. En todos los casos, esa lista no se modifica por la función, por lo que puede ser declarada (y debe) declararse como printNode()7 .

Comprender referencias y punteros

Casi todas sus funciones Tome un 99887766655443318 como argumento. Lo haría altamente lo recomiendo que use los punteros o referencias, pero no referencias a los punteros.

Uso printNode()9 En lugar de deallocateList()0

Modern C ++ usa deallocateList()1 en lugar de deallocateList()2 . Consulte Esta respuesta por qué y cómo es útil.

Minimizar la duplicación del código

El 99887766655443323 parece esto:

  deallocateList()4  

Sin embargo, la mayor parte del código es funcionalmente idéntica si deallocateList()5 es 99887776655443326 o no. Lo reescribiría así:

  deallocateList()7  

Evite las conversiones numéricas inútiles

En el deallocateList()8 TENGAMOS NEISTRIBLICAMENTE UNA CONVERSIÓN PARA deallocateList()9 que luego se convierte en un int0 cuando se devuelve el valor. Simplemente use int1 y evite las conversiones.

Repensar su interfaz

En este momento, solo los números que se ajustan en un int22 son representables dentro de este código, restringiendo gravemente el uso de la lista y las funciones, como int3 . En su lugar, considere agregar dígitos a la vez sin convertir cada lista a un int4 primero.

 

Here are some observations and suggestions that may help you improve your code.

Use objects

The program is written much more in the procedural style of C rather than in the object-oriented style of C++. The list itself could be an object (as could the individual nodes), with most of the procedures as functions of that object. This would reduce coupling and make the program easier to understand. Not least, instead of requiring the user to explicitly call deallocateList(), the object's destructor could automatically be called.

Make sure all paths return a value

Both the printNode() and deallocateList() functions claim to return an int but neither of them actually do. Either return a meaningful int value or change the function signature to void.

Match new and delete

As mentioned in comments to this question, new should be matched with delete and not delete[] which is a different operation.

Use better naming

The function named printNode is not well named. A better name might be printList since it actually prints the entire list and not just a single node.

Avoid using raw pointers

In modern C++, we don't tend to use raw pointers as much as in C. Instead, we either use smart pointers or objects. In this case, I'd say that if you created objects, it would be better to return an object from sumLists() and intToList() rather than a pointer to a node.

Use const where practical

In almost all of your functions, one parameter is a reference to a pointer to an intNode which is the beginning node of a list. In all cases, that list is unmodified by the function and so can (and should) be declared as const.

Understand references and pointers

Almost all of your functions take an intNode *& as an argument. I would highly recommend that you use either pointers or references but not references to pointers.

Use nullptr rather than NULL

Modern C++ uses nullptr rather than NULL. See this answer for why and how it's useful.

Minimize duplication of code

The current append looks like this:

void append(intNode *&s, int n) {     if (s == NULL) {         s = new intNode;         s->num = n;         s->next = NULL;     }     else {         intNode *nNode = new intNode;         nNode->num = n;         nNode->next = s;         s = nNode;     } } 

However, most of the code is functionally identical whether s is NULL or not. I'd rewrite it like this:

void append(intNode *&s, int n) {     s = new intNode{n, s}; } 

Avoid pointless numeric conversions

In the intConversionHelper() routine, we inexplicably have a conversion to float which is then converted back to an int when the value is returned. Just use int and avoid the conversions.

Rethink your interface

Right now, only numbers that fit in an int are representable within this code, severely restricting the use of the list and functions such as sumLists. Instead, consider adding digit at a time without converting each list to an int first.

 
 

Relacionados problema

4  Eliminar nodos alternativos de una lista vinculada  ( Delete alternate nodes of a linked list ) 
Si la lista vinculada es 1- & gt; 2- & gt; 3- & gt; 4 entonces la salida debe ser 1- & gt; 3. Si la lista vinculada es 1- & gt; 2- & gt; 3- & gt; 4- & gt; 5,...

5  Cola de bloqueo con la exactitud de la lista doblemente vinculada  ( Lock free queue with doubly linked list correctness ) 
Necesito una cola de bloqueo que se implementa mediante la lista doblemente vinculada. es mi código correcto? ¿Hay algún error? ¿Hay alguna mejoras a realiz...

1  DEQUEUE () en la implementación de la cola que utiliza una lista circular vinculada  ( Dequeue in queue implememtation that uses a circular linked list ) 
Utilizo una lista de enlaces circulares para implementar un queue , y solo sostiene un 99887776665544332 (Tenga en cuenta que 99887776655443333 enlaces a...

3  Lista doblemente vinculada en óxido usando los punteros crudos  ( Doubly linked list in rust using raw pointers ) 
Estoy practicando la oxidación escribiendo una lista doblemente vinculada usando los punteros crudos, 9988776655544330 Para asignar datos en el montón, 998...

14  Implementando una lista relacionada adecuada para un entorno profesional  ( Implementing a proper linked list for a professional environment ) 
Tengo algunas preocupaciones: ¿Es normal que la clase tenga al menos un nodo? En otras palabras, esta implementación no puede tener una lista vinculada va...

5  Destructor para una lista vinculada  ( Destructor for a linked list ) 
El código completo se encuentra aquí: https://gist.github.com/4521540 < / p> Es un maniquí List en C ++. Mi preocupación es para liberar la memoria. No s...

0  Agregando dos números extremadamente grandes usando una estructura de datos personalizada  ( Adding two extremely large numbers using a custom data structure ) 
Tengo una implementación para agregar 2 números extremadamente grandes, mayor que el techo proporcionado por long , por ejemplo, 1238913893838383813813813813...

10  Lista vinculada de objetos de conejito  ( Linked list of bunny objects ) 
El ejercicio es el ejercicio de la lista de conejitos vinculados; El último ejercicio para principiantes de aquí . Estoy buscando comentarios sobre absolut...

7  Simulando una lista enlazada lineal usando una matriz  ( Simulating a linear linked list using an array ) 
Un programa para simular una lista enlazada lineal usando una matriz: Especificaciones: A Y: cree un nuevo nodo con el valor de datos y, y agregue este...

4  ADT Pila con Linkedlist  ( Adt stack with linkedlist ) 
Actualmente estoy preparando para mi examen y estoy tratando de implementar algunos tipos de datos abstractos con listas vinculadas como preparación para el e...




© 2022 respuesta.top Reservados todos los derechos. Centro de preguntas y respuestas reservados todos los derechos