Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Aplicar consecuencias al mover para CasoDeUso9 #51

Merged
merged 3 commits into from
Nov 22, 2023

Conversation

RodriguezNazareno56
Copy link
Owner

@RodriguezNazareno56 RodriguezNazareno56 commented Nov 22, 2023

La idea es la propuesta por @juan-sosa "Habia pensado para ese caso que llegar a la meta sea una Consecuencia y aplicar una estrategia parecida a la que implemento Naza con FieraSalvaje". La implementación que hice es bastante fea, y sin duda require de un refactor pero nos permite pasar el caso de uso 9.

Básicamente existe una consecuencia triunfo

public class Triunfo implements Consecuencia {
    @Override
    public void afectarGladiador(Gladiador gladiador) throws TriunfoException {
        gladiador.triunfar();
    }
}

llama al método triunfar de Gladiador y lanza TriunfoException si posee el último equipamiento (el que al incrementarse ya no puede incrementarse) o te retrocede la mitad de posiciones:

    public void triunfar() throws TriunfoException {
        // El ultimo equipamiento (llave) al incrementarse se retorna asi mismo.
        if (equipamiento == equipamiento.incrementar()) {
            throw new TriunfoException("Campeon");
        } else {
            this.retroceder(casillero.getPosicion()/2);
        }
    }

Cosas que no me gustan:

  • El paquete casilleros: Creo que en general precisa ser refactorizado con fuerza
  • El casillero tiene una concecuencia, cuando no tiene ninguna ahora tiene una llamada SinConcecuencia que es una clase anémica (espantoso)
  • Si la concecuencia Triunfo puede lanzar un TriunfoException al hacer uso del metodo concecuencia.afectarGladiador(gladiador) entonces este debe pasar a ser afectarGladiador(Gladiador gladiador) throws TriunfoException propagándose así a todo lugar donde se aplique una consecuencia. Osea si hago un FieraSalvaje.afectarGladiador(gladiador) tengo que tratar la posible TriunfoException. Creo que en general no esta nada bueno.
  • Finalmente el metodo avanzar de gladiador creo que tiene muchas responsabilidades, muchos chequeos, aumentar la energia, el señority, setearse el casillero, afectar al gladiador con la concecuencia. Si bien es verdad que mucho de esto esta delegado en otros metodos deberiamos considerar implementar algo asi como una clase Movible. Siento que el gladiador cada vez esta mas cargado de responsabilidades.

Copy link

codecov bot commented Nov 22, 2023

Codecov Report

Attention: 10 lines in your changes are missing coverage. Please review.

Comparison is base (6228317) 85.55% compared to head (e2855ae) 81.40%.
Report is 2 commits behind head on master.

Files Patch % Lines
...main/java/edu/fiuba/algo3/Gladiador/Gladiador.java 28.57% 5 Missing ⚠️
...in/java/edu/fiuba/algo3/Concecuencias/Triunfo.java 33.33% 2 Missing ⚠️
...va/edu/fiuba/algo3/Gladiador/TriunfoException.java 0.00% 2 Missing ⚠️
...a/edu/fiuba/algo3/casilleros/CasilleroInicial.java 0.00% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##             master      #51      +/-   ##
============================================
- Coverage     85.55%   81.40%   -4.15%     
- Complexity       88       91       +3     
============================================
  Files            26       29       +3     
  Lines           180      199      +19     
  Branches          5        5              
============================================
+ Hits            154      162       +8     
- Misses           21       32      +11     
  Partials          5        5              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@RodriguezNazareno56
Copy link
Owner Author

Finalmente el caso de uso 9

    public void setUp() {
        CasillerosFactory casillerosFactory = new CasillerosFactory();
        ICasillero casillero = casillerosFactory.construirCasilleros(10);
        ....
     }
     
    @Test
    public void verificarQueSiLlegaALaMetaSinLaLlaveEnElEquipamientoRetrocedeALaMitadDeLasCasillas() throws Exception {
        // TODO: falta implementar
        this.gladiador.avanzar(10);
        // Empieza en casillero 0, avanza 10 hasta el 9, vuelve a la mitad (5)
        assertEquals(5, this.gladiador.getPosicion());
    }
}

@RodriguezNazareno56 RodriguezNazareno56 merged commit f97e87c into master Nov 22, 2023
8 of 10 checks passed
@RodriguezNazareno56 RodriguezNazareno56 deleted the AplicarConcecuenciasAlMover branch November 23, 2023 00:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant