consejos con optimizacion de codigo snake

[rulovive]

que tal amigos.... al grano:
hace poco terminé un juego snake en consola en borland c++. hice una biblioteca conio independiente del entorno (mi programa corre donde quiera). y el juego no tiene errores -que yo haya descubierto-.... el caso es que este juego es una prueba para cierto empleo y antes de enviar mi codigo y el ejecutable quise que un amigo me dijera sus opiniones...
en resumen el me dijo que la logica estaba bien, que el juego corre perfecto pero... en cuestion de optimizacion y posicion del codigo estaba horrible. coloco el codigo:

Código [Seleccionar] Expandir

# include<iostream.h>
# include<conio1.h>
# include<windows.h>
#include<stdio.h>
#include<time.h>

#define ARRIBA 72
#define ABAJO 80
#define DERECHA 77
#define IZQUIERDA 75


void nocursor()                                     //FUNCTION TO DISSAPEARS THE CURSOR OF THE WINDOW
{
HANDLE hStdout = GetStdHandle(STD_OUTPUT_HANDLE);
CONSOLE_CURSOR_INFO cci;
cci.dwSize=20;
cci.bVisible=FALSE;
SetConsoleCursorInfo(hStdout,&cci); }
void sicursor()                                     //FUNCTION TO APEARS THE CURSOR OF THE WINDOW
{
HANDLE hStdout = GetStdHandle(STD_OUTPUT_HANDLE);
CONSOLE_CURSOR_INFO cci;
cci.dwSize=20;
cci.bVisible=TRUE;
SetConsoleCursorInfo(hStdout,&cci); }


class vibora{
char comptecla,tecla,comida,*head;
int x,y,cx,cy,*hx,*hy,tamano;
public:
int timer,timer1,vel;
bool perder;
vibora(int _x,int _y): x(_x),y(_y){}
void inicializar();
void mover();
void borrar();
void pintar();
void generarcomida();
void comer();
};

void vibora::inicializar()
{tamano=3;
perder=false;
comida='©';
tecla=IZQUIERDA;
comptecla=IZQUIERDA;
timer1=clock()/100;

head=(char *) malloc(tamano *sizeof(int));
head[0]='@';
for (int i = 1; i <= tamano-1; i++)
head[i]='o';

hx=(int *) malloc(tamano *sizeof(int));
hy=(int *)malloc(tamano *sizeof(int));
for(int i=0; i<=tamano-1; i++)
{hx[i]=x+i; hy[i]=y;}

borrar();
pintar();
generarcomida();
}

void vibora::pintar()
{
for (int i = 0; i <= tamano-1; i++)
{gotoxy(hx[i],hy[i]); cout<<head[i];}
}

void vibora::generarcomida()
{int i=0;
cx=2+rand()%37;
cy=2+rand()%9;

do{
   if(cx==hx[i]&&cy==hy[i])
{cx=2+rand()%39;
cy=2+rand()%9;
i=0;}
   else
i++;
}while(i<=tamano-1);

gotoxy(cx,cy); cout<<comida;
}

void vibora::comer()
{
if(hx[0]==cx&&hy[0]==cy)
  {tamano++;
  hx=(int *)realloc(hx, tamano *sizeof(int));
  hy=(int *)realloc(hy, tamano *sizeof(int));
  head=(char *)realloc(head, tamano *sizeof(int));
  head[tamano-1]='o';
  generarcomida();}
}

void vibora::mover()
{
timer=clock()/100;
if(kbhit()) tecla=getch();   //registra pulsacion
if(tecla!=ARRIBA&&tecla!=ABAJO&&tecla!=DERECHA&&tecla!=IZQUIERDA) tecla=comptecla; //si la tecla pulsada es diferente de la de direccion, copia el ultimo movimiento
if((hx[1]==x-1&&tecla==IZQUIERDA )||(hx[1]==x+1&&tecla==DERECHA)||(hy[1]==y-1&&tecla==ARRIBA)||(hy[1]==y+1&&tecla==ABAJO))tecla=comptecla;  //si quiero ir para el lado contrario al que voy, no lo dejo, sino que copio el ultimo movimiento

if((tecla==IZQUIERDA&&comptecla!=DERECHA)||(tecla==DERECHA&&comptecla!=IZQUIERDA)||(tecla==ARRIBA&&comptecla!=ABAJO)||(tecla==ABAJO&&comptecla!=ARRIBA))comptecla=tecla;  //si pulso tecla de direccion y no es contraria, copio a comptecla


if(timer1==timer)
{
if(tecla==IZQUIERDA)
x--;
if(tecla==DERECHA)
x++;
if(tecla==ARRIBA)
y--;
if(tecla==ABAJO)
y++;







if(x==1||x==40||y==1||y==12)     //si choca en la orilla pierde...
for (int i = 0; i <= 2; i++)
  {borrar();
  Sleep(500);
  pintar();
  Sleep(500);
  perder=true;}
else                             //...pero si no choca en la orilla...
{
for (int i = 1; i <= tamano-1; i++)    //...reviso eslabon por eslabon...
if(x==hx[i]&&y==hy[i])    //...y si las coordenadas estan en un eslabon
for (int i1 = 0; i1 <= 2; i1++)
{borrar();
Sleep(500);                //...pierdo...
pintar();
Sleep(500);
perder=true;}
else                         //...pero si no estan en el eslabon...
   if(i==tamano-1&&perder==false)            //...compruebo si ya reviso todos y si ya lo hizo...
   {borrar();
for(int i=tamano-1; i>=1; i--)
{hx[i]=hx[i-1];
hy[i]=hy[i-1];}                     //... se mueve
hx[0]=x;
hy[0]=y;
pintar();
timer1=timer+vel;}
}

}

}

void vibora::borrar()
{
for (int i = 0; i <= tamano-1; i++)
{gotoxy(hx[i],hy[i]); cout<<" ";}
}


void main()
{
randomize();
bool nuevamente=true;
char resp;
for (int i = 1; i <= 40; i++)  //limite superior
{gotoxy(i,1); cout<<"*";}
for (int i = 2; i <= 12; i++)  //limite izquierdo
{gotoxy(1,i);cout<<"*";}
for (int i = 2; i <= 40; i++)  //limite inferior
{gotoxy(i,12); cout<<"*";}
for (int i = 2; i <= 12; i++)  //limite derecho
{gotoxy(40,i);cout<<"*";}





do{                    //do que sirve solo para comenzar y recomenzar el juego despues de perder

 
//este do sirve para elegir nivel de cada partida
  do{ gotoxy(1,13); cout<<"Bienvenido a Snake. Selecciona un nivel para comenzar a jugar (1-5): ";
  sicursor();
  cin>>resp;
  if(resp!='1'&&resp!='2'&&resp!='3'&&resp!='4'&&resp!='5')
{cout<<"Opcion no permitida...Debes seleccionar una opcion del 1 al 5."; Sleep(2000);
gotoxy(1,13);cout<<"                                                                                                                                                     ";
}
  else
{nocursor();
cout<<"Elegiste el nivel "<<resp; Sleep(1000);
gotoxy(1,13);cout<<"                                                                                                                                                     ";
gotoxy(11,13);cout<<"EL JUEGO COMIENZA EN";
gotoxy(20,14); cout<<"3"; Sleep(1000);
gotoxy(20,14); cout<<"2"; Sleep(1000);
gotoxy(20,14); cout<<"1"; Sleep(1000);
gotoxy(19,14); cout<<"YA"; Sleep(1000);
gotoxy(1,13);cout<<"                                                                                                                                                     ";
}         
  }while(resp!='1'&&resp!='2'&&resp!='3'&&resp!='4'&&resp!='5');


  vibora n(20,6);
  n.inicializar();
  switch (resp)
{case '1': n.vel=9; break;
case '2': n.vel=7; break;
case '3': n.vel=5; break;             //switch para la velocidad elegida
case '4': n.vel=3; break;
case '5': n.vel=1; break;}



 
  do{                                //bucle del juego en proceso
n.mover();
n.comer();
}while(n.perder==false);




  gotoxy(15,13);cout<<"HAS PERDIDO"; Sleep(1500);
  gotoxy(1,15); cout<<"Si quieres jugar de nuevo presiona 's', de lo contrario, presiona cualquier otra tecla: ";
  sicursor();
  cin>>resp;
  if(resp=='s'||resp=='S')
{
nocursor();
nuevamente=true;
gotoxy(15,13); cout<<"               ";
gotoxy(1,15); cout<<"                                                                                           ";
for(int i=2; i<=39; i++)
   for(int j=2; j<=11; j++)       //for que sirve para limpiar el campo de juego al perder
   {gotoxy(i,j);cout<<" ";}
Sleep(1000);
}
  else
  nuevamente=false;
 
}while(nuevamente==true);  //do que termina cuando le respondes que ya no quieres jugar


cout<<endl<<"Has elegido salir. ";
system("pause");
}


Si se fijan tengo una clase vibora, sus metodos y la funcion main que es donde va el bucle del juego... este compañero y amigo me dijo que los metodos de la clase definen (o deberian definir) de manera muy especifica y delimitada las propiedades y parametros del objeto, pero no colocar la mayoria del flujo del programa en esas partes... es decir. el me dijo que todos los metodos deberian ser parecidos a lo que hice con el metodo inicializar() y/o borrar(), no como en el metodo de mover(), donde se supone que solo deberia recibir instrucciones y ejecutarlas, mas no tomar  valores ni procesarlos ni nada ahi mismo...
ustedes que opinan? mi codigo es optimo de la forma que está? o podria sacar muchas partes de codigo de los metodos y colocarlo en main o en alguna funcion para que los metodos solo ejecuten instrucciones y parametros recibidos de dichas funciones??
NOTA: Aclaro que este codigo es 100% mio y que en realidad la critica no fue hacia este codigo sino hacia uno mucho mas largo y engorroso (un tetris) en el que el metodo llamado eliminacion tenia aprox el 70% del codigo total del programa... pero como ese codigo no tiene sangrias ni comentarios ni nada (lo hice hace como 4 meses y ni quiero meterme al codigo del metodo eliminacion porque es un reborujo que ni yo entiendo...por culpa de no ponerle los comentarios), puse mejor el del snake, que ilustra un poco mas ordenada la logica que use para el tetris, ya que es igual en casi todo el flujo del codigo... me podrian decir si debo modificarlo o asi esta bien?

[eferion]

El código francamente es mejorable:

* En C++ se suele emplear const en vez de defines

* Deberías tabular el código para hacerlo legible.

* La clase víbora no debería encargarse de cosas que no dependiesen de la propia vibora, como "generar la comida"

* Ya que usas coordenadas, ¿qué tal usar una clase para encapsular las coordenadas?

* ¿Por qué usas malloc en vez de new?

* ¿Y los correspondientes free?

* La interfaz del usuario debería ser independiente del código del juego.

Código (cpp) [Seleccionar] Expandir



class Punto
{
 public:
   Punto( int x, int y );

   int X( ) const;
   int Y( ) const;

 private:
   int _x;
   int _y;
};

class Tamano
{
 public:
   Tamano( int ancho, int alto );

   int Ancho( ) const;
   int Alto( ) const;

 private:

   int _alto;
   int _alto;
}

class Rectangulo
{
 public:
   Rectangulo( const Punto& punto, const Tamano& tamano );

   Punto EsqSupIzq( ) const;
   Punto EsqSupDer( ) const;
   Punto EsqInfIzq( ) const;
   Punto EsqInfDer( ) const;

   Tamano Tamano( ) const;

   // Verifica si el punto se encuentra dentro del rectangulo o no
   bool EstaDentro( const Punto& punto );

 private:

   Punto _origen;
   Tamano _tamano;
};

class Vibora
{
 public:
   enum Direccion
   {
     Arriba, Abajo, Derecha, Izquierda
   };

   Vibora( const Punto& inicio, Direccion direccion );

   // Obtiene las coordenadas del cuerpo de la serpiente... la primera coordenada es la cabeza
   std::vector< Punto > Cuerpo( ) const;

   // Permite cambiar la direccion en la que se mueve la serpiente
   void SetDireccion( Direccion direccion );

   // Mueve la serpiente en la direccion establecida
   void Mover( );

   // Se llama cuando la serpiente come una fruta, hace que su tamaño crezca
   void Comer( );

 private:

   std::vector< Punto > _cuerpo;
   Direccion _direccion;
};

class Juego
{
 public:
 
   // El parametro indica el tamaño del tablero de juego
   Juego( Tamano tamano, int nivel );

   // Inicia el algoritmo de juego
   void Jugar( );

 private:

   // Posicion de la comida ( si no se admite mas de una por vez se puede quitar el vector )
   std::vector< Point > _comida;

   // Serpiente del juego
   Serpiente _serpiente;

   // Tamaño del tablero de juego
   Tamano _tablero;

   // Para controlar la velocidad de juego
   int _nivel;

   // Comprueba si el usuario ha pulsado una tecla para cambiar de direccion
   void ComprobarTecla( );

   // Pinta el tablero de juego
   void PintarTablero( );

   // Pinta la serpiente
   void PintarSerpiente( );

   // Coloca una nueva pieza de comida
   void CrearComida( );
};

El diseño que te presento tiene una estructura más definida y organizada, además permite reutilizar código.

[amchacon]

¿Por que dices que es independiente del SO? Yo ahí veo la windows.h y la conio.h.

Para resolver el problema de la portabilidad necesitas compilación condicional:
http://foro.elhacker.net/programacion_cc/duda_sobre_portabilidad-t393915.0.html

[rulovive]

gracias por responder chicos ^^
amchacon: las librerias son estandarizadas, la conio1 la descargue para el dev y la windows si esta ahi pero me refeia mas bien a que lo puedo correr en cualquier compu que no tenga instalado borland :p

en cuanto al codigo del compañero eferion. muchas gracias ^^ tratare de estudiar el tuyo y acomodar el propio. si tuvieran alguna otra sugerencia estaria agradecido

[amchacon]

conio no es estandar ni windows.h

[vangodp]

Este de aqui es el snake de paueky para miniwin pero lo hice con SFML 

Código (cpp) [Seleccionar] Expandir


#include <SFML/Graphics.hpp>
#include <SFML/Window.hpp>
#include <windows.h>
#include <list>
#include <iostream>
using namespace std;

const int XTAM = 66, YTAM = 40;
const int SZ = 12;

struct Punto{
    int x, y;
};


void cuadrado ( Punto &p,  sf::RenderWindow &laventana ){
    sf::RectangleShape rectangle;
   
    rectangle.setPosition ( p.x*SZ, p.y*SZ );
rectangle.setSize ( sf::Vector2f ( SZ-1, SZ-1) );  // *_*   

rectangle.setFillColor(sf::Color::Red);

laventana.draw(rectangle);
}

bool colision( const Punto &cabeza, const list<Punto>& cola ){
    if( cabeza.x >= XTAM || cabeza.x < 0 ){
        return true;       
    }
    if( cabeza.y >= YTAM || cabeza.y < 0 ) {
        return true;
    }
    list<Punto>::const_iterator it;
    for( it = cola.begin(); it != cola.end(); it++ ){
        if( cabeza.x == it->x && cabeza.y == it->y ){
            return true;           
        }
    }
    return false;
}

Punto aleatorio(){
    Punto p = { rand() % XTAM, rand() & YTAM } ;
    return p;
}


int main() {
    sf::RenderWindow window ( sf::VideoMode ( XTAM * SZ, YTAM * SZ, 32 ), "SFML APP" );
    window.setVerticalSyncEnabled(true);
    window.setFramerateLimit(60);
    Punto cabeza = {XTAM/2, YTAM/2}; //cabeza, punto salida
    Punto comida = aleatorio();
    Punto temp = {0, 0}; 
    int vx = 1, vy = 0;              //Velocidad
    int engorda = 0;
    bool choque = false;
    list<Punto> cola;
    int retraso = 0;
    while ( window.isOpen() && !choque ) {
        retraso++;
                   
        sf::Event tecla;
while ( window.pollEvent ( tecla ) ) {
switch ( tecla.type ) {
                case sf::Event::Closed:
                    std::cout << "Fin de juego!" << std::endl;
window.close();
break;
case sf::Event::KeyPressed:
                    if(tecla.key.code == sf::Keyboard::Up )
                        vx = 0, vy = -1;
                    else if(tecla.key.code == sf::Keyboard::Down )
                        vx = 0, vy = +1;
                    else if(tecla.key.code == sf::Keyboard::Left )
                        vx = -1, vy = 0;
                    else if(tecla.key.code == sf::Keyboard::Right )
                        vx = +1, vy = 0;
                    else if(tecla.key.code == sf::Keyboard::Space )
                        engorda = 1;
                    break;
default:
                    break;
}

}       
window.clear ();
if ( retraso == 7 ) {
            cola.push_front(cabeza);   
            if ( engorda > 0 ){
                engorda --;           
            }else{
                cola.pop_back();
            }
           

cabeza.x = cabeza.x + vx;
cabeza.y = cabeza.y + vy;

if ( colision( cabeza, cola) ) {
                choque = true;
} else if (cabeza.x == comida.x && cabeza.y == comida.y){
                engorda = 1;
                comida = aleatorio();
                while ( colision(comida, cola) || comida.x == cabeza.x && comida.y == cabeza.y ){
                    comida = aleatorio();
                }
}

retraso = 0;
}

if ( !choque ) {

list<Punto>::const_iterator it;

for ( it = cola.begin(); it != cola.end(); it++ ) {
temp = *it;
cuadrado ( temp, window );
}

cuadrado ( cabeza, window );
cuadrado (comida, window );
window.display();
}
    }//END BUCLE

    //return EXIT_SUCCESS;
}


Se que el código es una chapuza XDD
pero funciona ^^
los controles son WASD y la barra de espacio agranda la serpiente es solo para probar.
Tampoco hice mucho, solo la hice andar y poco mas XDD