Mensajes

El tráfico de red se puede optimizar... pero mi consejo es ir por partes... primero haz que funcione y luego lo optimizas.

[1. Las clases mejor pasarlas como referencia.]

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;
}

También podrías tratarlo como un vector usando estructuras.

O bien, si eres más purista y no te gustan los condicionales del compilador en medio del código:

Código (cpp) [Seleccionar] Expandir


#ifdef _WIN32
char* limpiar_pantalla_cmd = "CLS";
#else
char* limpiar_pantalla_cmd = "clear";
#endif

void func( )
{

  // ...

  system( limpiar_pantalla_cmd );

  // ...

}

Código (cpp) [Seleccionar] Expandir

for (f = 0; f < 5;f++)
{
   for (c= 1; c < 5;c++)

el primero es un bucle para las filas ( por eso la f por nombre de variable ) que las recorre de la 0 a la 4. El segundo es un bucle para columnas... y las recorre de la 0 a la 4... y eso para cada valor de f.

Dicho esto, si, estarías recorriendo toda la matriz.

Y dicho esto también me permito añadir lo siguiente: si te pierdes con dos bucles y cuatro líneas de código ponte las pilas porque esto no es ni tan siquiera la punta del iceberg. Aprende a usar el depurador de c++ y a seguir la traza de los programas... te ayudará a comprender mejor cómo funciona el código... y haz muuuuchas prácticas

C++ no necesita limpieza de buffer ni esas bobadas que tenía C

Corrección: no está de más, antes de hacer una lectura del buffer de teclado asegurarse de que el usuario no ha escrito nada antes que pueda afectar a la lectura... como todo buffer almacena información y a no ser que la leas o descartes sigue ahí esperándote cual bomba de relojería.

Código (cpp) [Seleccionar] Expandir


struct partidas
{
 char nom_jug[40];
 char ced_jug[8];
};

struct partidas partida1, partida2;

Es lo mismo, más claro y con menos código que:

Código (cpp) [Seleccionar] Expandir


struct partidas1
{
 char nom_jug[40];
 char ced_jug[8];
 
} partidas1[500];

struct partidas2
{
 char nom_jug[40];
 char ced_jug[8];
} partidas2[500];

Aunque si lo que quieres es, por ejemplo almacenar los datos de las últimas 10 partidas necesitarías quizas algo así:

Código (cpp) [Seleccionar] Expandir


struct partidas
{
 char nom_jug[40];
 char ced_jug[8];
};

struct partidas partidas[10];

Con eso tendrías un array que te permitiría almacenar hasta 10 partidas.

Ahora la cosa, te lo dejo como tarea porque no voy a resolverte toda la práctica, es almacenar en ese array las diferentes partidas que vayas jugando.

Recuerda que lo primero es inicializar el array para que no muestre basura ( este consejo es gratis ), y luego almacenar los resultados en la posición en que correspondan.

Código (cpp) [Seleccionar] Expandir

         if(*computadora<*opcion)
                       printf("Mi numero es meno\n");
               else if(*computadora>*opcion)
                       printf("Mi numero es mayor\n");
               else
                       printf("Acertaste");
                       exit(1);

ese exit( 1 ) se va a ejecutar siempre.

Si quieres que un if o un else abarque más de una instrucción tienes que ponerlo entre llaves.

Más cosas... estás almacenando el dato del usuario en opcion y aparte usas esa variable para comprobar que no se pasa de 7 intentos... no va a funcionar.

Y además, personalmente, prefiero no usar exit salvo causas de fuerza mayor... yo lo dejaría así:

Código (cpp) [Seleccionar] Expandir

int main()
{
 int num,com,i;
 int acertado = 0;
 int *numero,*computadora,*opcion;
 numero = #
 computadora = &com;
 opcion = &i;
 printf("Juego que adivina el numero de la computadora<----------\n");
 srand(time(NULL));
 *computadora = (rand()%50)+1;
 printf("Tienes 7 oportunidades para adivinar el numero\n");
 for(*opcion=1;(*opcion<=7) && (acertado == 0);(*opcion)++)
 {
   printf("%d oportunidad---->",*opcion);
   scanf("%d",numero);
   if(*computadora<*numero)
     printf("Mi numero es meno\n");
   else if(*computadora>*numero)
     printf("Mi numero es mayor\n");
   else
   {
     printf("Acertaste");
     acertado = 1;
   }
 }

 if ( acertado == 0 )
 {
   printf("Lastima se te acabaron las oportunidades\n");
   printf("%d",*computadora);
 }
 
 return 0;
}

Verás que también he eliminado el uso de ii... ya tienes esa información en la variable *opcion, luego es redundante... aparte de que no incrementabas nunca el valor de dicha variable.

Deberías aprender a usar los depuradores para corregir los errores de tu código.

Un saludo

pues usa QtCreator... no hace falta que uses las librerías de Qt, puedes programar también con la stl si quieres... y desde luego funciona muuuuucho mejor que Devcpp

[eferion]

a y b entiendo que son matrices dinámicas, no?

Código (cpp) [Seleccionar] Expandir


int** a, b;

o las estás declarando con un tamaño estático y rezas para que el usuario no pida un tamaño superior al que has definido ??

Lo pregunto porque las soluciones serían ligeramente diferentes.

es que tambien me parece demasiado repetitivo el codigo, ingresarle para una matriz, luego ingresarle para otra.. seria bueno hacerlo con una funcion como dijo eferion???

Yo solo hice una apreciación y es que aprender a dividir el código en funciones no es siempre una tarea fácil... practicarlo con ejemplos sencillos ayuda luego a desenvolverse con códigos más complejos... pero no deja de ser una decisión tuya si pones funciones o todo junto.

El código puede funcionar igual de bien... otra cosa es lo que tu practiques y, en consecuencia, aprendas, al hacer estas prácticas.