rodrigorotondo / TP_solitario_Algo3

0 stars 0 forks source link

Etapa 1 #14

Closed TaniaFrieden closed 1 year ago

smaraggi commented 1 year ago

Hola chicos les paso la corrección del TP

Comentarios de Diseño

  1. En ColumnaDeJuego están violando varios principios al mismo tiempo. Me refiero a esta parte del código en particular:

    public Color obtenerColorPrimeraCarta(){
        return obtenerPrimeraCarta().obtenerColor();
    }
    
    public int obtenerNumeroPrimeraCarta(){
        return obtenerPrimeraCarta().obtenerNumero();
    }
    
    public int obtenerNumeroUltimaCarta(){
        return obtenerUltimaCarta().obtenerNumero();
    }
    
    public Color obtenerColorUltimaCarta(){
        return obtenerUltimaCarta().obtenerColor();
    }
    public Palo obtenerPaloUltimaCarta(){
        return obtenerUltimaCarta().obtenerPalo();
    }
    public void darVueltaUltimaCarta(){
        obtenerUltimaCarta().cambiarVisibilidad();
    }

    Están violando PoLK, y capaz tenga algo de PoLA. A una Pila le pido una Carta, y las propiedades de la Carta se las pido a la carta. Si no hay otro motivo, Pila no debería inmiscuirse en atributos de Carta directamente si no va a aportar algún enriquecimiento de comportamiento que lo justifique.

Comentarios sobre uso de Git.

  1. Veo que no usaron branches. No es obligatorio. Les recomiendo al menos usar un tag para etiquetar el commit de la etapa 1 cuando esté aprobada.

Cambios Requeridos para aprobar etapa 1.

  1. Arreglar la violación al principio de diseño PoLK. Están rompiendo el encapsulamiento de Carta sin motivo que lo justifique. Un motivo que podría justificar que Pila se meta con los atributos de carta es que hubiera una alguna lógica en la pila que necesite consultar los valores de palo y número de cartas. Pero en este caso, de todas formas Pila debería obtener los datos de las cartas, hacer las operaciones lógicas y devolver el resultado del proceso, no tiene sentido que haga de pasamanos de los atributos de Carta. Fíjense que abajo mismo implementan en la pila Carta getCarta()... con esto es innecesario tener todos estos métodos.

Solamente les pido este cambio para dar por aprobada la Etapa 1. Recuerden que la aprobación no implica que no vayan que tener que ir puliendo si a algo le falta un poco para las etapas siguientes, sino que significa que el diseño va por un camino razonable. Está bastante bien, salvando esto que les digo.

Comentario adicional

Tienen muchos métodos que son triviales. Por ejemplo:

public boolean esMismoPalo(Palo paloCarta){
        return this.palo == paloCarta;
    }

Este método no aporta mayormente nada, y reemplaza una simple comparación como ser carta1.getPalo() == carta2.getPalo();

No les exijo que lo saquen, pero sepan que no aporta nada tampoco. Es una cuestión de gusto si lo dejan o no.

smaraggi commented 1 year ago

Hola chicos, les copio una pregunta que recibí por chat para que quede en el historial de la corrección.

Me consultaron si por ejemplo que otra clase que no fuera Pila tuviera que conocer los métodos (o alguna interfaz) de Carta no sería POLK. La respuesta es que no, y acá la explicación:

En general, para que Pila tenga métodos que hagan cosas sobre las cartas sería conveniente que exista alguna lógica interna de la Pila que justifique la existencia de la función; sino se termina repitiendo código que ya existe en Carta (violación a DRY).

Pueden pensar en el principio POLA, sino, que dice esto:

"Este principio propone que una abstracción debe capturar todo el comportamiento de algún objeto, sin ofrecer sorpresas o efectos secundarios que vayan más allá del alcance de la abstracción." (POLA, diapos de la cátedra).

¿Les parece que dar vuelta una carta tiene que ver con la abstracción de la carta o con la abstracción de la pila? A la Pila le podemos agregar o quitar cartas, o quizás hasta mover la pila entera a otro lugar del tablero. Pero dar vuelta una carta es algo que tiene que ver con la carta y no interviene en principio la Pila.

Finalmente, no hay violación de POLK si en clases de nivel superior se tiene conocimiento de Carta y de sus atributos útiles al juego. Es una dependencia absolutamente necesaria. Y no está mal que una clase un poco superior tenga conocimiento de otra que parece estar algo más lejos en una jerarquía de clases, piensen que una carta podría existir por fuera de las pilas. Si, por ejemplo, el jugador pudiera hacer click y tener la carta en el aire hasta depositarla en otra pila (digamos, pegada al cursor), y no hace falta que lo hagan así, sino que les doy un ejemplo en que Carta vive afuera de una Pila; no todo tiene que pasar por Pila, y no es violación de POLK que otras clases conozcan a Carta y sus primitivas.

rodrigorotondo commented 1 year ago

Las correcciones fueron hechas!

smaraggi commented 1 year ago

Hola chicos, está muy bien encaminado el TP.

Damos por aprobada la etapa 1, cierro el issue con este comentario.

Felicitaciones.

Prosigan con la etapa 2.