Primero me voy a centrar en el estilo de programación:
1. Las clases mejor pasarlas como referencia.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í:
int donde_esta( const string& nombre, vector< ranking >& v )
2. Separa el código con espacios y tabula correctamentevector<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
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 absurdasComo suena, esto
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í:
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
if(p[i].tipo_competicion=='s')
{
v[lugar].pruebas_puntuables++;
}
o si prefieres ganar un poco más de rendimiento:
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 amigosun 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.
// 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
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í:
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 amigasEn 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:
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:
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 tareaActualmente, 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 )
#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;
}
Autor
En línea
