diaznicolasandres1 / 95.02-algo3-algo-empire-tp-final

Proyecto final algoritmos y programacion III - POO/Patrones de diseño/MVC/TDD
1 stars 0 forks source link

Violación de encapsulamiento #6

Closed tomasBustamante closed 5 years ago

tomasBustamante commented 5 years ago

El método mePuedoMover() : boolean de la clase ArmaDeAsedio viola el encapsulamiento.

Debería seguirse el principio "Tell, don't ask" de manera tal que no se pida permiso para realizar una determinada tarea, sino que se haga directamente y, en caso de que no corresponda, se lanza una excepción o se la trata acordemente.

En este caso, si lo que se necesita es saber si el objeto se puede mover o no, se debería enviarle el mensaje mover(destino) : void y que se encargue de (o delegue) la responsabilidad del movimiento. El patrón State para determinar un booleano es un overkill.

Este comentario es un poco llamativo:

[...] los movimientos se manejan por afuera del modelo

El manejo de los movimientos debe ser parte del modelo. Lo único que no corresponde al modelo es la interfaz gráfica, siguiendo el patrón MVC.

FacuMastri commented 5 years ago

Nuestra idea es que la clase Mapa (o Juego) sea la encargada de ir moviendo cada Unidad a lo largo del Mapa. La Unidad solo se encargaría, a priori, de atacar/ser atacado.

tomasBustamante commented 5 years ago

Pero le están otorgando al ArmaDeAsedio la responsabilidad de determinar si puede moverse o no, lo cual dependerá de su estado. Por lo tanto no es posible lograr que solamente el Mapa (o Juego) se encargue de eso y al forzar esa separación terminan violando el encapsulamiento. Si en su modelo el ArmaDeAsedio sabe si puede moverse o no, entonces no debería revelar eso sino que le enviarían un mensaje para moverse y él sabrá cómo responderlo.

diaznicolasandres1 commented 5 years ago

Hola Tomas!. Entonces podríamos hacer que cuando el arma este montada el estado lanza la excepcion correspondiente y caso contrario el estado desmontada, el metodo no realiza nada si no que el mapa la mueve. Algo por el estilo Desde el mapa: try{ armaDeAsedio.mover(posicion); }catch(la excepcion lanzada desde el estado){ manejamos la excepcion; } Y aca realizar lo necesario para realizar el movimiento desde el mapa

Saludos

tomasBustamante commented 5 years ago

No estoy seguro de llegar a entenderlo, pero si tenés un método mover(posicion) : void debería encargarse del movimiento. No entiendo por qué quieren hacer que "el mapa se encargue del movimiento".

tomasBustamante commented 5 years ago

Añado que también hay una posible violación del encapsulamiento en la clase Posicion. Ahora que @JonathanGalvanPerez subió las pruebas se aprecia que su única responsabilidad es determinar si otra posición está dentro del rango y devolver un booleano. ¿Para qué será utilizado ese booleano? ¿A qué otra entidad le interesa saber eso? ¿Se puede hacer que, el motivo por el cual otra entidad necesite ese booleano sea delegado a la Posición? Ver este ejemplo con una especie de Double Dispatch en Smalltalk.