error estructura al compilar

[indict]

Hola, he echo una ejercicio de estructuras que consiste en lo siguiente:
le introduces
el nombre, s si la competición es puntuable n en caso que no lo sea y los puntos correspondientes que ha generado dicha competición.

Quiero generar una lista que me muestre
el nombre, el total de pruebas realizadas, el total de pruebas puntuables (que será la suma de pruebas de tipo "s") y el total de puntos. Lógicamente podré introducir varias veces un mismo usuario y se irán acumulando los puntos y el número de competiciones.

He diseñado un programa para ello, pero me marca error al final de escribir los usuarios.

Código (cpp) [Seleccionar] Expandir

#include<iostream>
#include<vector>
#include<algorithm>
using namespace std;

struct usuario{
      string nombre;
      char tipo_competicion; // s en caso que la competición sea puntable n en caso contrario
      int puntos;
     
      };
     
struct ranking{
      string nombre;
      int total_pruebas;
      int pruebas_puntuables;
      int puntos;
     
      };
     
int donde_esta(string nombre,vector<ranking>&v){
   for (int i = 0 ; i < v.size() ; i++){
       if (v[i].nombre == nombre){return i;} }
       ranking r;
       r.nombre=nombre;
       r.total_pruebas=0;
       r.pruebas_puntuables=0;
       r.puntos=0;
       }
       
vector<ranking>classificacion(vector<usuario>p){
vector<ranking>v;
for(int i=0;i<p.size();i++){
       int lugar=donde_esta(p[i].nombre,v);
       v[lugar].total_pruebas=v[lugar].total_pruebas+1;
       v[lugar].puntos=v[lugar].puntos+p[i].puntos;
       if(p[i].tipo_competicion=='s'){
                                      v[lugar].pruebas_puntuables=v[lugar].pruebas_puntuables+1;}
                                     
       else if(p[i].tipo_competicion=='n'){v[lugar].pruebas_puntuables=v[lugar].pruebas_puntuables+0;}
       }
       return v;
       }
               
bool ordenar_pruebas(ranking c1, ranking c2){
    if(c1.total_pruebas==c2.total_pruebas){
    if(c1.puntos==c2.puntos){return c1.nombre<c2.nombre;}  
                          else{return c1.puntos>c2.puntos;}}
    else{return c1.total_pruebas>c2.total_pruebas;}            
                                         
    }          
   
bool ordenar_puntos(ranking c1, ranking c2){
    if(c1.puntos==c2.puntos){return c1.nombre<c2.nombre; }  
    else{return c1.puntos>c2.puntos;}
    }                          
                                               
int main(){
   int n;
   cout<<"introduzca el numero de elementos de la estructura"<<endl;
   cin>>n;
   vector<usuario>p(n);
   
cout<<":::::::::::::::::::::::::::::::::::::::::::::::"<<endl;
cout<<endl;

for(int i=0;i<n;i++){
       cout<<"nombre >> competicion (s)-> PUNTUABLE, (n)-> NO PUNTUABLE >> puntos "<<endl;
       
       cin>>p[i].nombre;
       cin>>p[i].tipo_competicion;
       cin>>p[i].puntos;
       
       }
       
vector<ranking>v=classificacion(p);

cout << "-----------------------"<< endl;

sort(v.begin(),v.end(),ordenar_pruebas);

cout<<"LISTA"<<endl;

for(int i=0;i<v.size();i++){
       cout<<v[i].nombre<<" "<<v[i].total_pruebas<<" "<<v[i].pruebas_puntuables<<" "<<v[i].puntos<<endl;
       }
   
   system("pause");
   return 0;
}
   
   

Gracias por la ayuda

[ivancea96]

Mm estuve echandole un vistazo, y vi que falla aquí:

Código (cpp) [Seleccionar] Expandir


vector<ranking>classificacion(vector<usuario>p){

vector<ranking>v;

for(int i=0;i<p.size();i++){
       int lugar=donde_esta(p[i].nombre,v);

     /**Aqui falla**/  v[lugar].total_pruebas=v[lugar].total_pruebas+1;/**Aqui falla**/

       v[lugar].puntos=v[lugar].puntos+p[i].puntos;
       if(p[i].tipo_competicion=='s'){
                                      v[lugar].pruebas_puntuables=v[lugar].pruebas_puntuables+1;}
       else if(p[i].tipo_competicion=='n'){v[lugar].pruebas_puntuables=v[lugar].pruebas_puntuables+0;}
       }
       return v;
       }


Aparentemente, a base de pauses, vi que ahi da el error. No me paré más, porque bueno, es bastante lioso todos los vectores.

Un par de cosas que vi, que podrías revisar:

Código (cpp) [Seleccionar] Expandir

/*Linea 76*/ vector<ranking>v=classificacion(p);

Código (cpp) [Seleccionar] Expandir

/*Linea 32*/ vector<ranking>v;
Esos 2 vectores se llaman igual, "v". No se si eso tendrá que ver.
Siento no ser de más ayuda. Si encuentras por ahí algún error, pues bien, sinó, siento estorbar xd

EDITO: En el título pones error al compilar. Yo lo compilé con Code::Blocks, y me dió error en tiempo de ejecución. Me compiló con varios warnings, nada más.
Te pongo los warnings, por si te interesan:

Código [Seleccionar] Expandir


In function 'int donde_esta(std::string, std::vector<ranking>&)':|
|43|warning: comparison between signed and unsigned integer expressions [-Wsign-compare]|
||In function 'std::vector<ranking> classificacion(std::vector<usuario>)':|
|66|warning: comparison between signed and unsigned integer expressions [-Wsign-compare]|
||In function 'int main()':|
|167|warning: comparison between signed and unsigned integer expressions [-Wsign-compare]|
||In function 'int donde_esta(std::string, std::vector<ranking>&)':|
|57|warning: control reaches end of non-void function [-Wreturn-type]|
||=== Build finished: 0 errors, 4 warnings (0 minutes, 5 seconds) ===|



Cabe destacar el Warning que dice que la función "int donde_esta(std::string, std::vector<ranking>&)", que si el "if" no se cumple, no retorna ningún valor. No se como lo tienes tú pensado, posiblemente no tenga nada que ver ese warning, pero bueno. Sugiero xD

[amchacon]

Código (cpp) [Seleccionar] Expandir

vector<ranking>classificacion(vector<usuario>p){
vector<ranking>v;
for(int i=0;i<p.size();i++){
       int lugar=donde_esta(p[i].nombre,v);
       v[lugar].total_pruebas=v[lugar].total_pruebas+1;
       v[lugar].puntos=v[lugar].puntos+p[i].puntos;
       if(p[i].tipo_competicion=='s'){
                                      v[lugar].pruebas_puntuables=v[lugar].pruebas_puntuables+1;}

       else if(p[i].tipo_competicion=='n'){v[lugar].pruebas_puntuables=v[lugar].pruebas_puntuables+0;}
       }
       return v;
       }
Estás accediendo a posiciones del vector v... Pero el vector v está vacío.

¿Quizás querías decir p?

[eferion]

Primero me voy a centrar en el estilo de programación:

1. Las clases mejor pasarlas como referencia.

Código (cpp) [Seleccionar] Expandir

int donde_esta(string nombre,vector<ranking>&v)

Este tipo de definiciones, por defecto, intenta evitarlas... estás pasando string por valor en vez de por referencia, esto implica duplicar la cadena en memoria cada vez que llamas a la función y el rendimiento se puede resentir y muchísimo.

Es mejor pasar una referencia constante, así:

Código (cpp) [Seleccionar] Expandir

int donde_esta( const string& nombre, vector< ranking >& v )

2. Separa el código con espacios y tabula correctamente

Código (cpp) [Seleccionar] Expandir


vector<ranking> classificacion(vector<usuario> p)
{
  vector<ranking>v;
  for(int i=0;i<p.size();i++)
  {
    int lugar = donde_esta(p[i].nombre, v);
    v[lugar].total_pruebas = v[lugar].total_pruebas + 1;
    v[lugar].puntos = v[lugar].puntos + p[i].puntos;
    if(p[i].tipo_competicion == 's')
    {
      v[lugar].pruebas_puntuables = v[lugar].pruebas_puntuables + 1;
    }
    else if(p[i].tipo_competicion == 'n'
    {
      v[lugar].pruebas_puntuables = v[lugar].pruebas_puntuables + 0;
    }
  }
 
  return v;
}

Queda algo más claro que

Código (cpp) [Seleccionar] Expandir

vector<ranking>classificacion(vector<usuario>p){
vector<ranking>v;
for(int i=0;i<p.size();i++){
       int lugar=donde_esta(p[i].nombre,v);
       v[lugar].total_pruebas=v[lugar].total_pruebas+1;
       v[lugar].puntos=v[lugar].puntos+p[i].puntos;
       if(p[i].tipo_competicion=='s'){
                                      v[lugar].pruebas_puntuables=v[lugar].pruebas_puntuables+1;}

       else if(p[i].tipo_competicion=='n'){v[lugar].pruebas_puntuables=v[lugar].pruebas_puntuables+0;}
       }
       return v;
       }

Y la claridad es algo que los que te ayudan agradecen, ya que tienden a dejarse menos la vista para comprender tu código

3. Evita líneas absurdas

Como suena, esto

Código (cpp) [Seleccionar] Expandir


if(p[i].tipo_competicion=='s')
{
  v[lugar].pruebas_puntuables=v[lugar].pruebas_puntuables+1;
}
else if(p[i].tipo_competicion=='n')
{
  v[lugar].pruebas_puntuables=v[lugar].pruebas_puntuables+0;
}

Se puede simplificar tranquilamente y quedar así:

Código (cpp) [Seleccionar] Expandir


if(p[i].tipo_competicion=='s')
{
  v[lugar].pruebas_puntuables = v[lugar].pruebas_puntuables+1;
}


Ya que X + 0 = X... nos podemos ahorrar la comprobación y la suma... porque si no, ya puestos, ¿que pasa cuando tipo_competicion no es ni 's' ni 'n'?

Y esto aún se podría simplificar un poco más, tal que

Código (cpp) [Seleccionar] Expandir


if(p[i].tipo_competicion=='s')
{
  v[lugar].pruebas_puntuables++;
}

o si prefieres ganar un poco más de rendimiento:

Código (cpp) [Seleccionar] Expandir


if(p[i].tipo_competicion=='s')
{
  ++v[lugar].pruebas_puntuables;
}


Los preincrementos pueden llegar a ser bastante más eficientes que los postincrementos.

4. Los iteradores son tus amigos

un bucle for te sirve perfectamente para recorrer un array... sin embargo manejar ese array con índices te va a obligar a escribir bastante más texto... con el consecuente incremento de probabilidades de error.

Menos texto, si es con cabeza, suele significar muchas veces código más legible y fácil de mantener.

Además la mayoría de funciones de la stl utilizan los iteradores para manipular los vectores, por lo que aprener a manejarlos te va a permitir usar la stl en muchísimas situaciones.

Código (cpp) [Seleccionar] Expandir



// He puesto postincrementos en vez de preincrementos porque creo que son más fáciles de ver cuando estás aprendiendo.
vector< ranking >::const_iterator it;
for ( it = p.begin( ); it != p.end( ); ++it )
{
  int lugar = donde_esta( it->nombre, v );
  v[ lugar ].total_pruebas++;
  v[ lugar ].puntos += it->puntos;
  if ( it->tipo_competicion == 's' )
  {
    v[ lugar ].pruebas_puntuables++;
  }
}

En vez de

Código (cpp) [Seleccionar] Expandir


for(int i=0;i<p.size();i++)
{
  int lugar=donde_esta(p[i].nombre,v);
  v[lugar].total_pruebas=v[lugar].total_pruebas+1;
  v[lugar].puntos=v[lugar].puntos+p[i].puntos;
  if(p[i].tipo_competicion=='s')
  {
    v[lugar].pruebas_puntuables=v[lugar].pruebas_puntuables+1;
  }
}

5. Una función, un return.

Cuando aprendas a depurar, te darás cuenta que tener un solo return en una función te facilita mucho la vida. Con un solo breakpoint podrás controlar la salida de la función para saber si es correcta o no. Sin embargo si tienes 20 returns repartidos por la función la tarea se empieza a complicar.

Hay a gente a la que le gusta llenar las funciones de returns... yo solo lo hago si la función con returns queda muchísimo más sencilla y clara que sin ellos.

En el caso de tu función y aplicando esta norma quedaría así:

Código (cpp) [Seleccionar] Expandir


bool ordenar_pruebas(ranking c1, ranking c2)
{
  bool to_return = true;

  if (c1.total_pruebas==c2.total_pruebas)
  {
     if(c1.puntos==c2.puntos)
        to_return = c1.nombre<c2.nombre;
     else
        to_return = c1.puntos>c2.puntos;
  }
  else
    to_return = c1.total_pruebas>c2.total_pruebas;

  return to_return;           
}

6. Las clases también son tus amigas

En vez de estructuras, prueba a usar clases y aprovecha sus características. Las clases y las estructuras en c++ son iguales salvo porque, por defecto, las clases tienen sus miembros privados mientras que en las estructuras son públicos.

En cualquier caso, por convención, es más natural usar clases en el código normal de c++ y dejar las estructuras para intercambio de datos vía sockets por ejemplo.

Además, una característica imprescindible de las clases y estructuras en c++ son los constructores, que te permiten inicializar las clases para que tengan valores válidos al crearlas. Esto te evita tener que inicializarlas tu a mano cada vez que instancias una clase.

Si tu pones:

Código (cpp) [Seleccionar] Expandir


class ranking
{
  public:
    ranking( const string& nombre )
    {
      this->nombre = nombre;
      total_pruebas = 0;
      pruebas_puntuables = 0;
      puntos = 0;
    }

    string nombre;
    int total_pruebas;
    int pruebas_puntuables;
    int puntos;
};

entonces:

Código (cpp) [Seleccionar] Expandir


int donde_esta(string nombre,vector<ranking>&v)
{
  for (int i = 0 ; i < v.size() ; i++)
  {
    if (v[i].nombre == nombre)
      return i;
  }

  // Esto que viene ...
  ranking r;
  r.nombre=nombre;
  r.total_pruebas=0;
  r.pruebas_puntuables=0;
  r.puntos=0;
  //  ... se sustituye por ...
  ranking r( nombre );
  // ... una sola linea!!! 
}

También sería interesante que las variables fuesen privadas y creases funciones para acceder y modificar sus valores, pero tampoco me voy a poner a redactar aquí un libro de c++

7. Una función, una tarea

Actualmente, la función donde_esta hace demasiadas cosas. Por un lado busca un elemento en el vector y, si no se encuentra, intenta añadirlo.

Bueno, en primer lugar la función está mal hecha porque no añade nunca el elemento en el vector y, aunque lo hiciese, no devuelve la posición de dicho elemento, estos son los errores que tienes actualmente en el código.

Pero hablando de la función en si, una función que se llama donde_esta, debería centrarse únicamente en localizar un elemento y devolver su posición. Si el elemento no se encuentra debería devolver un índice no válido para poder identificar el error.

Si luego quieres añadir un nuevo elemento y que te de su posición prueba a crear una función nueva que sea nuevo_elemento, por ejemplo.

El que una función haga más cosas de las que debe te genera el problema de que mantener esa función en el futuro va a ser complicado, ya que ahora tienes muy reciente cómo funciona y que hace... pero dentro de 6 meses la memoria no será tan buena y tener una función que se llama, por ejemplo, despertador, y que aparte de sonar la alarma se encarga de preparar el desayuno, levantar las persianas, hacer la cama, encender la caldera, ... complicado de entender.

Dicho esto, una propuesta quizás sería: ( voy a aplicar todos los consejos que te he indicado )

Código (cpp) [Seleccionar] Expandir


#include<iostream>
#include<vector>
#include<algorithm>

using namespace std;

class usuario
{
  public:
    string nombre;
    bool tipo_competicion; // su funcion es booleana... o es competicion o no lo es, mejor bool
    int puntos;

    // Constructores SIEMPRE para inicializar la memoria.
    // * nombre no hace falta inicializarlo porque es una clase y ya tiene su propia inicializacion,
    //   aunque podria ponerse si quieres.
    usuario( )
      : tipo_competicion( false ),
        puntos( 0 )
    {
    }
};

class ranking
{
  public:
    string nombre;
    int total_pruebas;
    int pruebas_puntuables;
    int puntos;

    // Esta forma de inicializar las variables es mas optima que la que te he explicado antes.
    // Tambien quizas un poco mas complicada de entender, pero como he dicho, es mejor.
    ranking( const string& nombre_alumno )
      : nombre( nombre_alumno ),
        total_pruebas( 0 ),
        pruebas_puntuables( 0 ),
        puntos( 0 )
    {
    }

    // Aprovechando las caracteristicas de las clases, metemos el sumador aqui
    void suma( const usuario& usuario )
    {
      ++total_pruebas;
      puntos+= usuario.puntos;
      if ( usuario.tipo_competicion )
        ++pruebas_puntuables;
    }
};

vector< ranking >::iterator donde_esta( const string& nombre,
                                        vector< ranking >& v )
{
  vector< ranking >::iterator it;
  for ( it = v.begin( ); it != v.end( ); ++it )
  {
    if ( it->nombre == nombre )
      break;
  }

  return it;
}

vector< ranking >::iterator nueva_entrada( const string& nombre,
                                           vector< ranking >& v )
{
  ranking r( nombre );
  v.push_back( r );

  return donde_esta( nombre, v );
}

vector< ranking > classificacion( const vector< usuario >& p)
{
  vector< ranking > v;
  vector< usuario >::const_iterator it_usuario;

  for( it_usuario = p.begin( ); it_usuario != p.end( ); ++it_usuario )
  {
    vector< ranking >::iterator it_ranking = donde_esta( it_usuario->nombre, v );
    if ( it_ranking == v.end( ) )
      it_ranking = nueva_entrada( it_usuario->nombre, v );

    it_ranking->suma( *it_usuario );
  }

  return v;
}

bool ordenar_pruebas( const ranking& c1,
                      const ranking& c2)
{
  bool to_return;

  if( c1.total_pruebas == c2.total_pruebas )
  {
    if( c1.puntos == c2.puntos )
      to_return = c1.nombre<c2.nombre;
    else
      to_return = c1.puntos>c2.puntos;
  }
  else
    to_return = c1.total_pruebas>c2.total_pruebas;

  return to_return;
}

bool ordenar_puntos( const ranking& c1,
                     const ranking& c2)
{
  bool to_return;

  if( c1.puntos == c2.puntos )
    to_return = c1.nombre<c2.nombre;
  else
    to_return = c1.puntos>c2.puntos;

  return to_return;
}

int main()
{
  int n;
  cout << "introduzca el numero de elementos de la estructura" << endl;
  cin >> n;
  vector< usuario > p(n);

  cout << ":::::::::::::::::::::::::::::::::::::::::::::::" << endl;
  cout << endl;

  for(int i=0;i<n;i++)
  {
    // Para separar un poco mas el texto.
    cout << endl;
   
    // Es mejor guiar al usuario por pasos
    cout << "nombre: ";
    cin >> p[i].nombre;

    cout << "competicion (s/n): ";
    char c;
    cin >> c;
    // si no se introduce una s, se interpreta que no es de competicion.
    // si quieres forzar a que el usuario introduzca s o n te toca programarlo
    p[i].tipo_competicion = ( c == 's' );

    cout << "puntos: ";
    cin >> p[i].puntos;
  }

  vector< ranking > v = classificacion(p);
  cout << "-----------------------" << endl;

  sort( v.begin(), v.end(), ordenar_pruebas );

  cout << "LISTA" << endl;

  vector< ranking >::const_iterator it;
  for( it = v.begin( ); it != v.end( ); ++it )
  {
    cout << it->nombre << " "
         << it->total_pruebas << " "
         << it->pruebas_puntuables << " "
         << it->puntos << endl;
  }

  system("pause");
  return 0;
}

[indict]

Gracias por este amplio tutorial y por la explicación de todas las pautas que debo formalizar.
Ahora iré asimilando toda esta nueva información.

Un saludo