0
#include <vector>
#include <list>
#include <iostream>

using namespace std;

list<int> calc_cambio(vector<int>& v,int d){
    cout<<"dentro"<<endl;
    list<int> cambio;
    for(int i=0;i<=(int)v.size();i++){
        int c=d/v[i];
        cout<<"monedas: "<<c<<endl;
        cambio.push_back(c);
        d=d%v[i];
        cout<<"cantidad por cambiar: "<<d<<endl;

    }
     cout<<"terminé el for"<<endl;
    if(d!=0){
        cout<< "no existe cambio posible"<<endl;
        list<int> vacia;
        return vacia;
    }

    cout<<"fuera"<<endl;
    return cambio;

}

void write_cambio(list<int>& c,vector<int>& v){
    int i=0;
    for(const int & x: c){

        if(x!=0){
            if(i!=0){
                cout<<" ,";
            }
            if(v[i]>=100){
                cout<<x<<"x"<<(v[i]/100)<<"€";
            }
            else{
                cout<<x<<"x"<<(v[i])<<"cent";
            }
        }
        i++;
    }
}
/*
 * 
 */
int main(int argc, char** argv) {
    vector<int> monedas = {200,100,50,20,10,5,2,1};
    float n;
    cout<<"Introduce la cantidad que quieres cambiar:";
    cin >> n;
    cout<<"aquí"<<endl;
    int devolver = (int)(n*100);
    cout<<"allí "<<devolver<<endl;
    list<int> cambio = calc_cambio(monedas , devolver);
    write_cambio(cambio,monedas);

    return 0;
}

el output fue este:

Introduce la cantidad que quieres cambiar:3.54
aquí
allí 354
dentro
monedas: 1
cantidad por cambiar: 154
monedas: 1
cantidad por cambiar: 54
monedas: 1
cantidad por cambiar: 4
monedas: 0
cantidad por cambiar: 4
monedas: 0
cantidad por cambiar: 4
monedas: 0
cantidad por cambiar: 4
monedas: 2
cantidad por cambiar: 0
monedas: 0
cantidad por cambiar: 0
Excepción de coma flotante (`core' generado)

El error me salta cuando estoy en la función calc_cambio pero en el único sitio que trabajo con flotantes es el main así que no se si estoy cometiendo un error a la hora de cambiarlo a int

PaperBirdMaster
  • 44,474
  • 6
  • 44
  • 82
mistermagius
  • 25
  • 1
  • 8
  • ¿Puede ser que estés dividiendo por 0? – XMP Dec 08 '17 at 12:51
  • @XMP He mirado el código y no se me ocurre donde puedo estar dividiendo por 0 porque en los sitios donde hago divisiones son elemento que defino anteriormente y no los modifico en ningún momento – mistermagius Dec 08 '17 at 13:06
  • Creo que donde haces el push_back deberías comprobar que el elemento no sea ya igual a 0. Porque entonces dará otra vuelta y te saltará la división entre 0. Eso encajaría con la traza que has indicado. – XMP Dec 08 '17 at 13:30
  • @XMP Gracias por la ayuda, el problema no era lo de meter un 0 pero sí estaba dando una vuelta de más – mistermagius Dec 08 '17 at 13:52

1 Answers1

1

<=

Estás cometiendo uno de los errores más comunes del uso de bucles for y contenedores. Ten en cuenta que los contenedores en C++ se indexan desde 0, por lo que si tienes ocho elementos en un std::vector sus índices irán del 0 al 7 ambos incluidos.

for(int i=0;i<=(int)v.size();i++){

Suponiendo que el std::vector<int> &v tenga tamaño ocho, el bucle ejecutará estos ciclos:

| ciclo | i |      condición      |
+-------+---+---------------------+
|     1 | 0 | 0 <= 8 -> verdadero |
|     2 | 1 | 1 <= 8 -> verdadero |
|     3 | 2 | 2 <= 8 -> verdadero |
|     4 | 3 | 3 <= 8 -> verdadero |
|     5 | 4 | 4 <= 8 -> verdadero |
|     6 | 5 | 5 <= 8 -> verdadero |
|     7 | 6 | 6 <= 8 -> verdadero |
|     8 | 7 | 7 <= 8 -> verdadero |
|     9 | 8 | 8 <= 8 -> verdadero |
|    10 | 9 | 9 <= 8 -> falso     |

El bucle debería interrumpirse antes de empezar el noveno ciclo con i conteniendo 8 pero como 8 es menor o igual a 8 se inicia el noveno ciclo y...

Divides entre cero...

... posiblemente.

El contenedor std::vector ofrece dos maneras de acceder a los datos contenidos, mediante el operador de indexado ([]) y mediante la función std::vector::at. La diferencia entre estas dos maneras de acceder es que la primera no realiza comprobaciones de rango, simplemente devuelve (mediante aritmética de punteros) el valor contenido en la posición resultante de sumar N a la primera posición del arreglo (siendo N el número dentro de los corchetes); en el caso de std::vector::at se lanza una excepción (std::out_of_range) si N no pertenece al rango del vector.

Al usar los corchetes, has accedido a memoria que no pertenece al std::vector y esta memoria posiblemente contenía un cero:

vector<int> monedas = {200,100,50,20,10,5,2,1};

posición | 0 | 1 | 2 | 3 | 4 | 5 | 6 | 7 | 8 | 9 | 10| 11| 12| ...
         +---+---+---+---+---+---+---+---+---+---+---+---+---+---
         |200|100| 50| 20| 10| 5 | 2 | 1 |???|???|???|???|???| ...
                                          ~~~ <-- aquí accedes en el bucle,
                                                  posiblemente contenga 0

Solución.

Cambia tu bucle para que la condición sea menor estricto:

for(int i=0;i<(int)v.size();i++){
//           ^ <--- Menor, no menor o igual.

Propuesta.

Ya que has marcado la pregunta como , usa el bucle for de rango:

list<int> calc_cambio(vector<int>& v,int d){
    cout<<"dentro"<<endl;
    list<int> cambio;
    for(auto valor_moneda : v){
        int c=d/valor_moneda;
        cout<<"monedas: "<<c<<endl;
        cambio.push_back(c);
        d=d%valor_moneda;
        cout<<"cantidad por cambiar: "<<d<<endl;

    }
     cout<<"terminé el for"<<endl;
    if(d!=0){
        cout<< "no existe cambio posible"<<endl;
        list<int> vacia;
        return vacia;
    }

    cout<<"fuera"<<endl;
    return cambio;

}

Otras cosas a tener en cuenta.

  1. No se aconseja el uso de using namespace std; salvo que sea estrictamente necesario (no suele serlo) consulta este hilo para saber por qué.
  2. No suele ser necesario usar std::endl, es más eficiente usar un salto de línea explícito \n, consulta este hilo para saber por qué.
  3. Puedes devolver un std::vector vacío sin necesidad de crear una variable con nombre, simplemente escribe return {}; en funciones cuyo retorno sea un std::vector.
  4. No uses variables cuyo nombre sea una sola letra, es tremendamente confuso, asigna nombres auto-explicativos, te ayudará en el futuro a entender tu propio código y será mejor por tu salud al ahorrarte las agresiones y males de ojo de tus compañeros de trabajo.
  5. Usa alias de tipos para añadir significado semántico a tus tipos y aumentar así la comprensión de tu código.
  6. Cualifica como constantes los parámetros que no vayan a ser modificados en las funciones, eso ayuda a compartir intencionalidad y da pistas al compilador para aplicar optimizaciones.
  7. Decídete por un idioma, mezclar inglés y español en el código es confuso.

Aplicando los puntos propuestos, tu código quedaría así:

#include <vector>
#include <list>

using valor_monedas = std::vector<int>;
using recuento_monedas = std::vector<int>;

/* De un vistazo vemos que la funcion calcula_cambio devuelve un
* recuento de monedas, no algo abstracto como un vector de enteros.
* Tambien vemos que recibe una coleccion de valores de monedas y que
* dicha coleccion no sera modificada en el interior de la funcion. */
recuento_monedas calcula_cambio(const valor_monedas& valores, int devolver){
    std::cout<<"dentro\n";
    //                ~~ <--- Evitamos std::endl

    recuento_monedas cambio;

    /* El bucle for de rango evitara que accedamos a valores fuera del
    * rango del vector, los nombres auto-explicativos nos ayudan a
    * comprender el proceso */
    for (const auto &valor_moneda : valor_monedas){
        int monedas = devolver / valor_moneda;
        std::cout << "monedas: " << monedas << '\n';
        cambio.push_back(monedas);
        devolver = devolver % valor_moneda;
        std::cout << "cantidad por cambiar: " << monedas << '\n';
    }

    std::cout << "terminé el for\n";

    if(devolver){ // Cualquier valor diferente a 0 es considerado verdadero
        std::cout << "no existe cambio posible\n";
        return {}; // Devuelve una lista vacia que construye en el mismo return
    }

    std::cout << "fuera\n";
    return cambio;

}

void muestra_cambio(const recuento_monedas& recuentos, const valor_monedas& valores){
    int indice=0;

    for (const auto & monedas: recuentos){
        if (monedas){
            if(indice){
                cout<<" ,";
            }
            if(valores[indice]>=100){
                std::cout << monedas << "x" << (valores[indice]/100) << "€";
            }
            else{
                std::cout << monedas << "x" <<(valores[indice]) << "cent";
            }
        }
        indice++;
    }
}


int main(int argc, char** argv) {
    valor_monedas monedas = {200,100,50,20,10,5,2,1};
    float cantidad;
    std::cout << "Introduce la cantidad que quieres cambiar:";
    std::cin >> cantidad;
    std::cout << "aquí\n";

    int devolver = cantidad * 100;
    std::cout << "allí " << devolver << '\n';

    auto cambio = calcula_cambio(monedas, devolver);
    muestra_cambio(cambio, monedas);

    return 0;
}
PaperBirdMaster
  • 44,474
  • 6
  • 44
  • 82
  • Accede a 0 porque la implementación típica de `std::vector` de los compiladores modernos suele incluir una optimización que consiste en crear un minivector en la pila (la inmensa mayoría de los vectores suele tener un tamaño inferior a 15 elementos)... en ese caso se omite la reserva de memoria y se tira de la pila de forma transparente para el usuario... lo que sucede en este caso es que esa memoria suele estar incializada a 0 y el resto de la historia ya nos la conocemos – eferion Dec 11 '17 at 09:47
  • Lo había pensado @eferion pero lo descarté porque en los compiladores en que he hecho pruebas (gcc, clang) tanto el `size` como el `capacity` me dieron `8`. – PaperBirdMaster Dec 11 '17 at 10:31
  • Paper, como es cambiaste de nombre? Lo dijiste por el Chat y me lo perdí o fue destrangis? – lois6b Jan 03 '18 at 09:36