El for no me hace su funcion? (Solucionado)

[eferion]

Veo que te bailan un poco los conceptos. vamos por partes.

1. Constructores (parte1)

Al igual que los seres vivos ( con perdón de las reencarnaciones ) solo nacen una vez y en ese proceso ya nacen con todo, los objetos sólo se pueden crear una vez.

Cada vez que tu llamas a un constructor estás creando un objeto nuevo y diferente de los ya existentes.

El siguiente código crea tres camiones diferentes:

Código (cpp) [Seleccionar] Expandir


//Instancia de camion y separado por cada constructo     
            Camion *nuevoCamion1 = new Camion(idCam, cilindrajeCam, nPuertasCam, anioCam);
            Camion *nuevoCamion2 = new Camion(precioCam);
            Camion *nuevoCamion3 = new Camion(marcaCam, modeloCam, colorCam);

Imagino que tú ahí estás intentando asignar todas las propiedades al mismo elemento... eso se hace con los getters y los setters que has implementado.

Código (cpp) [Seleccionar] Expandir


Camion* nuevoCamion = new Camion( );
nuevoCamion->setId( idCam );
nuevoCamion->setCilindraje( cilindrajeCam );
nuevoCamion->setNPuertas( nPuertasCam );
// ...

Fíjate de paso que no he dicho "setIdCamion" ni "setCilindrajeCamion"... si la clase se llama camión es redundante y totalmente innecesario que pongas el sufijo "Camion" a todos los miembros de la clase... pero eso lo tratamos más adelante para no desviarnos.

El caso es que quizás es una buena práctica que el constructor de una clase reciba únicamente aquella información que es necesaria desde un primer momento para el buen funcionamiento del código. En tu caso, la clase Camion puede funcionar perfectamente si todos sus elementos tienen valores por defecto, luego con gestionar el constructor por defecto te sobra y te basta.

No tengas miedo a que esta forma de pensar te obligue a tener 10 líneas más de código... créeme si te digo que hay malas prácticas que generan muchas más líneas de código que además suele ser bastante ilegible.

Además, no he podido evitar fijarme en la siguiente línea:

Código (cpp) [Seleccionar] Expandir


temporalListaDeCamiones[temporalCantidadDeCamiones] = new Camion(listaDeCamiones[i]->getIdCamion(),
                                                                                  listaDeCamiones[i]->getCilindrajeCamion(),
                                                                                  listaDeCamiones[i]->getnPuertasCamion(),
                                                                                  listaDeCamiones[i]->getanioCamion(),
                                                                                  listaDeCamiones[i]->getPrecioCamion(),
                                                                                  listaDeCamiones[i]->getMarcaCamion(),
                                                                                  listaDeCamiones[i]->getModeloCamion(),
                                                                                  listaDeCamiones[i]->getColorCamion());

Resulta que tú aquí estás intentando hacer una copia de un objeto ya existente. Los chicos que diseñaron C++ eran bastante listos y ya pensaron en esta posibilidad y la incorporaron al lenguaje... estamos hablando del constructor copia.

El constructor copia se implementa por defecto en tooooodas las clases de C++... esto quiere decir que mientras la clase en cuestión no utilice memoria dinámica en sus miembros el constructor que se genera por defecto es perfectamente válido... aunque eso no quita que te apetezca implementarlo tu explícitamente para aprender, por optimización, por seguridad o por lo que sea.

El constructor copia, si lo defines explícitamente, debería tener la siguiente firma.

Código (cpp) [Seleccionar] Expandir


class Camion
{
  public:
    Camion( const Camion& );
};

Y en su implementación deberías realizar la tarea de copiar los datos de un objeto en otro:

Código (cpp) [Seleccionar] Expandir


Camion::Camion( const Camion& otro )
{
  idCamion = otro.idCamion;
  cilindrajeCamion = otro.cilindrajeCamion;
  // ...
}

Con esto, tu línea puede quedar reducida a algo tan simple como:

Código (cpp) [Seleccionar] Expandir

temporalListaDeCamiones[temporalCantidadDeCamiones] = new Camion( *listaDeCamiones[i] );

o incluso, si temporalListaDeCamiones fuese un vector, el código quedaría aún más simple:

Código (cpp) [Seleccionar] Expandir

temporalListaDeCamiones.push_back( new Camion( *listaDeCamiones[ i ] );

Resumiendo, tu clase debería tener el constructor por defecto y, si te apetece implementarlo explícitamente, el constructor copia... el resto de constructores puedes obviarlos:

Código (cpp) [Seleccionar] Expandir


class Camion
{
  public:
    Camion( );
    Camion( const Camion& otro );
};

2. Constructores (parte 2)

Cuando tú reservas memoria con un new, la memoria que el sistema te da viene "tal cual", es decir, no hay un proceso de limpieza que deje la memoria lista como si fuese a estrenar. Mas bien te encuentras con que la memoria que te ha dado el sistema tiene basura no es sino la información que otra aplicación, en su momento, escribió en esa posición y ahora ya no la usa.

Es tarea del que programa una clase inicializar toda la memoria utilizada por dicha clase. Esto quiere decir, en otras palabras, que has de dar valores por defectos a todas las variables miembros, al menos a aquellas que no sean clases.

El sitio adecuado para hacer esta tarea es en el constructor ( o constructores ). Recuerda que la llamada a un constructor cualquiera, por ejemplo al constructor copia, no implica una llamada al constructor por defecto.

Código (cpp) [Seleccionar] Expandir


Camion::Camion( )
{
  idCamion = 0;
  cilindrajeCamion = 0;
  // ...
}

Con este trabajo te aseguras que todos los objetos que se creen van a tener valores válidos SIEMPRE, que es el objetivo a conseguir. Un objeto con valores no válidos se vuelve inestable y da problemas, quedas avisado.

3. Clases (parte3)

En este apartado me voy a centrar en la nomenclatura utilizada para los miembros de la clase.

Si bien es algo meramente estético no deja de ser importante para facilitar la lectura del código.

Como te comenté anteriormente no tiene sentido que un método de la clase Camion contenga el sufijo Camion. Si por ejemplo tu dices:

Código (cpp) [Seleccionar] Expandir

camion->setNPuertas( X );

Se presupone que lo que vas a indicar es el número de puertas del camión... luego el sufijo "Camion" sobra.

Con esta premisa, los setters deberían tener más o menos esta forma:

Código (cpp) [Seleccionar] Expandir


class Camion
{
  public:
      void setId(int);         //Devuelve el Id del Camion.
      void setCilindraje(int); //Devuelve el cilindrage del camion
      void setNPuertas(int);   //Devuelve el numero de puertas del camion
      void setanio(int);       //Devuelve el año del camion
      void setPrecio(double);  //Devuelve el precio del camion
      void setMarca(string);   //Devuelve la marca del camion
      void setModelo(string);  //Devuelve el modelo del camion
      void setColor(string);   //Devuelve el color del camion
};

Aunque se les podría dar una vuelta de tuerca más.

Si has leído todo hasta aquí ya te habrás enterado de que existe el constructor copia. En tu código actual, cuando tu haces:

Código (cpp) [Seleccionar] Expandir

camion->setMarca( loquesea );

estás creando una copia de loquesea y esa copia será la que se use dentro de la función... esto supone una carga adicional y absurda, pues se crea un objeto copia con cada llamada.

Es una buena costumbre que las clases se pasen como referencias constantes. De esta forma evitas tener que crear un objeto temporal perfectamente evitable.

El cambio es relativamente sencillo:

Código (cpp) [Seleccionar] Expandir


class Camion
{
  public:
      void setId(int);         //Devuelve el Id del Camion.
      void setCilindraje(int); //Devuelve el cilindrage del camion
      void setNPuertas(int);   //Devuelve el numero de puertas del camion
      void setanio(int);       //Devuelve el año del camion
      void setPrecio(double);  //Devuelve el precio del camion
      void setMarca( const string& );   //Devuelve la marca del camion
      void setModelo( const string& );  //Devuelve el modelo del camion
      void setColor( const string& );   //Devuelve el color del camion
};

Y además tiene la ventaja de que las llamadas a las funciones son exactamente iguales que antes!!!

Ponerle la etiqueta const indica que el objeto que se pasa como argumento no va a ser modificado... una referencia es similar a un puntero y aquí tienes un ejemplo:

Código (cpp) [Seleccionar] Expandir


void func( int& numero )
{
  numero = 10;
}

void main( )
{
  int num = 0;

  std::cout << num << std::endl;
  func( num );
  std::cout << num << std::endl;
}

El resultado de ejecutar este programa es el siguiente:

Código [Seleccionar] Expandir


0
10

4. Principio de responsabilidad única

Cada clase y cada método que programes debería regirse por este principio, por tu bien.

El principio se basa en que cada función debería dedicarse a una única tarea. Una clase que permite imprimir por pantalla no debería controlar el estado de la tarjeta de sonido, por ejemplo.

En tu caso, la clase Camion tiene dos tareas diferentes:

* Gestiona los datos de un camion
* Gestiona una lista de camiones

Esta parte de tu código en la clase Camion

Código (cpp) [Seleccionar] Expandir


  private:
      Camion**listaDeCamiones;
      int cantidadDeCamiones;

Debes eliminarla. Si quieres gestionar una lista de camiones, estupendo... pero que no sea la clase Camion la encargada de dicha gestión. Si es necesario que de dicha gestión se encargue una clase, creas una clase nueva.

Otra forma de violar este principio lo encontramos en este otro ejemplo que te vas a encontrar más veces de las que piensas:

Código (cpp) [Seleccionar] Expandir


class Persona
{
  public:
    void SetDatos( )
    {
      std::cout << "Introduce el nombre: ";
      std::cin >> nombre;
      std::cout << "Introduce su edad: ";
      std::cin >> edad;
    }

  private:
    std::string nombre;
    int edad;
};

En este caso, la clase Persona no solo gestiona los datos de una persona, también se encarga de comunicarse con el usuario para pedirle datos... mala solución.

Lo mismo esto te resulta familiar. Tu clase listaDeCamiones infringe este principio.

5. Contenedores

Insisto, si no es un requisito del ejercicio, usa contenedores en vez de arreglos. Salvo que quieras o tengas que practicar, no es necesario reinventar la rueda con cada programa... no es práctico.

Los contenedores te permiten almacenar listas de elementos de formas diversas: ordenados, sin ordenar, sin duplicados, con índice, ...

Además con los contenedores reduces las probabilidades de acceder a posiciones de memoria que no debes, te proporcionan información sobre el número de elementos que tienen, pueden crecer dinámicamente sin que tú tengas que preocuparte por ello...

Como norma general, todo son ventajas.

6. Otras cuestiones

Deberías leerte y reorganizar tu código. La última secuencia de código que has puesto deberían formar parte de la clase listaDeCamiones... ya que es esta clase la que gestiona la lista de camiones.

Por el momento creo que ya es bastante para asimilar, dedica el tiempo necesario a arreglar el código y no intentes ir rápido... las prisas no son buenas.

[nolasco281]

Entendí la mayoría de las cosas que me dijiste, ya lo he leído como 30,35 veces

Y la funcion listaDeCamiones::ingresarCamion() ya la probe y esta bien ya no tengo problema.

Código (cpp) [Seleccionar] Expandir

Camion* nuevoCamion = new Camion( );
           nuevoCamion->setId(idCam);
           nuevoCamion->setCilindraje(cilindrajeCam);
           nuevoCamion->setNPuertas(nPuertasCam);
           nuevoCamion->setanio(anioCam);
           nuevoCamion->setPrecio(precioCam);
           nuevoCamion->setMarca(marcaCam);
           nuevoCamion->setModelo(modeloCam);
           nuevoCamion->setColor(colorCam);
           
           //Añadiendo los camiones a nCamiones a arreglo y lo actualiza
           //recorar nuevoCamion instancia de Camion                                        
           Camion[nCamiones++] = nuevoCamion;

Y mis ultimas dos dudas.

1. Pero para ir guardando los registros del cada camion. estaria bien asi ya que todavía no lo estoy mostrando solo guardanlos.

Código (cpp) [Seleccionar] Expandir

Camion[nCamiones++] = nuevoCamion; //Tambien esta donde le mando los argumentos a los metodos para que se observe mejor.

2. Una anotación que me llamo mucho la atención es de la clase persona. Eso me hace pensar, no sé si estoy en lo correcto que debería de crear la de encabezado que gestiona los datos de una persona. y la de implementación que se encarga de comunicarse con el usuario. .

En mi caso sería una para que me gestione los datos de listaDeCamiones la .h y la otra que se comunique con el usuario .cpp

Y no sé si estoy mal lo dices para mantener la robustez de un software y la legibilidad del mismo.

Muchas gracias por su ayuda enserio mil gracias me han ayudado a mejorar en un 100% en cuanto al manejo de código y a las buenas practicas. Siempre las tomo y más cuando te ayudan a agilizar y sobretodo que otras personas lo entiendan.

agradezco a primeramente a eferion y a yoel_alejandro. por tomarce todo ese tiempo para ayudarme y con eso doy por cerrado este tema.

y me dare la tarea de los demas hacerlo yo, ya que no veo mucha dificulta haciendo el primero. tanto con vectores como con arreglo de apuntadores. para entender este tema mejor.

Y gracias de nuevo un saludo a todos.

[eferion]

1. Pero para ir guardando los registros del cada camion. estaria bien asi ya que todavía no lo estoy mostrando solo guardanlos.

Código (cpp) [Seleccionar] Expandir

Camion[nCamiones++] = nuevoCamion; //Tambien esta donde le mando los argumentos a los metodos para que se observe mejor.

Si tú en tu código tienes algo tal que:

Código (cpp) [Seleccionar] Expandir

class Camion ...

Ni puedes ni debes usar la palabra Camion para identificar un array, vector o arreglo de camiones.

Que haya nombres que colisionen ( es decir, que sean iguales ) y sirvan para cosas diferentes causa confusión a la hora de revisar el código y de encontrar errores.

Además, lo de nCamiones++... eso solo es necesario si usas arreglos al estilo de C, con contenedores no suele ser necesaria esa variable.

2. Una anotación que me llamo mucho la atención es de la clase persona. Eso me hace pensar, no sé si estoy en lo correcto que debería de crear la de encabezado que gestiona los datos de una persona. y la de implementación que se encarga de comunicarse con el usuario. .

En mi caso sería una para que me gestione los datos de listaDeCamiones la .h y la otra que se comunique con el usuario .cpp

A lo que me refería con el ejemplo de la clase Persona es que hay que separar actividades que no tienen nada que ver entre sí; me explico:

Tu diseñas una clase, por ejemplo una llamada "Camion"... y le pones un método para crear instancias de esta clase que le pide al usuario vía consola los datos requeridos para su creación. Pasa el tiempo y te llega un encargo para que ahora pueda funcionar con una interfaz de ventanas... ¿que haces en este caso? Al final te toca modificar una clase que ya funciona debido a un mal diseño.

Código (cpp) [Seleccionar] Expandir


// Diseño malo

class Camion
{
  public:
    Camion( );
    Camion( const Camion& );
    virtual ~Camion( );
   
    void SetId( int id );
    void SetMatricula( const std::string& matricula );

    int GetId( ) const;
    std::string GetMatricula( ) const;

   void PideDatos( ); // funcion conflictiva
};

En cambio imagínate que en vez de optar por ese diseño "rápido" y problemático, sacas de "Camion" la función que pide los datos al usuario y la dejas en una clase independiente... en este caso para cambiar la interfaz con el usuario no hace falta tocar para nada una clase que funciona como es el caso de Camion. Y no solo eso, si además has diseñado la clase con cierto cuidado puedes hasta hacer uso de polimorfismo para que tu código pueda funcionar con varias interfaces diferentes sin tener siquiera que recompilar.

Código (cpp) [Seleccionar] Expandir


// Diseño bueno

class Camion
{
  public:
    Camion( );
    Camion( const Camion& );
    virtual ~Camion( );
   
    void SetId( int id );
    void SetMatricula( const std::string& matricula );

    int GetId( ) const;
    std::string GetMatricula( ) const;
};

class Interfaz
{
  public:
    // Funcion virtual pura, esta clase sirve para poder hacer polimorfismo
    virtual Camion* PideDatos( ) = 0;
};

class InterfazDOS
  : public Interfaz
{
  public:
    // Este metodo contiene la logica necesaria para comunicarse con el usuario via consola
    virtual Camion* PideDatos( ) override;
};

class InterfazWindows
  : public Interfaz
{
  public:
    // Este metodo contiene la logica necesaria para comunicarse con el usuario via windows
    virtual Camion* PideDatos( ) override;
};

Interfaz ObtenerInterfazUsuario( )
{
  // Estas directivas de precompilador se pueden cambiar por variables o lo que sea
  // a gusto del consumidor
  #ifdef _WIN32
    return new InterfazWindows( );
  #else
    return new InterfazDOS( );
  #endif
}

void main( )
{
  Interfaz* interfaz = ObtenerInterfazUsuario( );

  // Le pedimos al usuario los datos del camion y nos da igual si
  // lo hace a traves de una ventana de Windows o de una consola.
  Camion* camion = interfaz->PideDatos( );
}

Espero que con el ejemplo haya quedado este tema un poco más claro.

Un saludo.

[nolasco281]

Aclarado cuando termine el programa lo comparto para quien le sirva.  Y si es un poco confuso pero hai voy saludos y gracias a todos.