salluzziluca / TP1-Algo3

1er TP de la materia Algoritmos y Programacion III de la Facultad de de Ingeniería de la Universidad de Buenos Aires
0 stars 0 forks source link

Etapa 3 #37

Closed dessaya closed 1 year ago

dessaya commented 1 year ago

Muy bueno! El TP está aprobado (9). Para el 10 les propongo que solucionen algunos (al menos 2?) de los siguientes items:


No estaba puesto como requisito, pero estaría bueno que dibujen un diagrama de clases y/o secuencia. Recuerden que no debe ser exhaustivo, solo incluir en el diagrama los conceptos más relevantes.


Me sale el siguiente error en los archivos de test:

Must declare a named package because this compilation unit is associated to the named module 'view'

El problema es que esos archivos les falta la línea package <xyz>;, es decir que son parte del "default package". Lo estándar es que todos los archivos sean parte de algún package no-default.


¿Por qué silencian las advertencias con @SuppressWarnings? Tiene que estar bien justificado, por algo existen...


Hay dependencias cíclicas entre las clases de controller y view. Por ejemplo, en controller.Juego hay un import view.BuilderEscenaTablero, y en view.BuilderEscenaTablero hay un import controller.Juego.

Recuerden que en general si dibujan un diagrama con las dependencias debería formar un grafo en el que las flechas van todas en una misma dirección; y en general si hay 2 paquetes (en este caso controller y view) y hay al menos una dependencia entre ambos, no debería haber ninguna dependencia en el sentido contrario.


    public boolean estaVivo() {
        return this.vida <= 0;
    }

No debería llamarse estaMuerto?


        Juego juego = new Juego(primaryStage, builderEscenaInicio, builderEscenaTablero, builderMazos, builderEscenaInstrucciones);
        builderEscenaInicio.subscribe(juego, juego);
        builderEscenaInstrucciones.subscribe(juego);
        juego.empezarJuego();

Si el Juego ya tiene una referencia al builderEscenaInicio, por qué las invocaciones a subscribe no ocurren automáticamente en el constructor de Juego, o en empezarJuego()?

O sea, que quede así:

        Juego juego = new Juego(primaryStage, builderEscenaInicio, builderEscenaTablero, builderMazos, builderEscenaInstrucciones);
        juego.empezarJuego();

y en Juego() o en empezarJuego():

public Juego(...) {
        ...
        builderEscenaInicio.subscribe(this, this);
        builderEscenaInstrucciones.subscribe(this);
}
salluzziluca commented 1 year ago

Buenas, muchas gracias por la corrección y por la nota, ambos logramos llegar a promocion! :D Con respecto a los comentarios, ahi estoy resolviendo varias de las cosas que nombrás aca, la mayoria no requieren mucha explicacion y con el propio commit se van a entender. Con respecto a los warnings, pensamos que estos eran propios de intellij y los suprimimos porque no nos parecien (por lo menos en este momento). Revisando ahora algunas recomendaciones "CanBeFinal" las agregué, pero otras como las de que el for normal puede ser reemplazado por el for each generan ciertas roturas en el codigo (ya que el for moderno usa un iterador y a la hora de modificar sobre el item que se está iterando, carta actual, se rompe, o por lo menos eso interpreto yo de la excepción). De todas formas, no pensabamos que fueran cruciales, pensamos que eran mas recomendaciones de estilo y prevención.

salluzziluca commented 1 year ago

Ahi corregí varias de las cosas que nombrabas aca, decime si hace falta algo mas, muchas gracias

dessaya commented 1 year ago

Perfecto, con eso ya les queda el 10. Felicitaciones!