Correccion - Caripela Libro - Entrega 1 - Grupo 4

- los try/catch vacíos de BuscarAmigoGrupoTest.setUp no están buenos.. es preferible relanzar una excepción y si falla algo que se rompa, o explicar con comentarios porque se tomó esta decisión

- guarda con las variables con mayúscula (por ejemplo Duenio). son convenciones que nos facilitan mucho leer el código. Se entienden igual pero llaman la atención.

- idem anterior con los métodos, la mayúscula se reserva por convención para los métodos de clase o estáticos.

convencines útiles:

variableOInstancia

instancia.metodoDeInstancia

NombreDeUnaClase

Clase.MetodoEstatico

Clase.CONSTANTE

Una no tan adoptada pero que a veces se ve:

_variablePrivada o _parametro

- usar excepciones chequeadas parece que les hizo un poco infeliz el uso de los métodos, para la próxima tengan en cuenta la posibilidad de usar excepciones no chequeadas.

- el método ComentarFotoTest.

EtiquetarFotoNoAmigoTest catchea una excepcion y adentro hace un assert, que pasa si ni siquiera lanza la excepción?

se podrían hacer 2 métodos, uno que valide que lance la excepión y otro que aparte cuando lanza la excepción valide que no haya comentarios, o directamente en un método hacer un return en el catch y un fail fuera. (tambien hay un assertEquals para el ==)

try {

FotoEtiquetada.Comentar(Comentador,"Mensaje de Prueba para una foto");

} catch (ComentarFotoException e) {

// TODO Auto-generated catch block

assertTrue(FotoEtiquetada.getComentarios().size() == 0);

return;

}

fail();

- assertTrue(false) se puede remplazar por fail()

- hay muchos catchs pegados que hacen lo mismo, les vendría bien manejar una jerarquía de excepciones para ahorrar código.

Más allá de eso, una práctica común es tener unas excepciones propias de las cuales hay 2 grandes familias que extienden BusinessExceptio y NonBusinessException entonces por ejempo NoExisteUsuarioPotencialException extiende de BusinessException

- hay una excepcion NoVerificaClaveConfirmacion, quizás sería más legible si se llamara NoVerificaClaveConfirmacionException

- falta testear que al etiquetar una foto se mande el mensaje y se mande SOLO UNA VEZ A CADA USUARIO el mensaje

- en RegistrarUsuarioTest.RegistrarUsuario2VecesTest si no lanza excepcion el test se rompe, para verificar esto JUnit4 nos da una herramienta excpected

@Test(expected=NoSePuedeRegistrarUsuarioRegistrado.class)

public void RegistrarUsuario2VecesTest() throws Exception{

CaripelaCarpeta.getInstancia().RegistrarUsuario(Apellido,Email);

assertTrue(CaripelaCarpeta.getInstancia().EstaRegistrado(Apellido,Email));

CaripelaCarpeta.getInstancia().RegistrarUsuario(Apellido,Email);

assertTrue(CaripelaCarpeta.getInstancia().EstaRegistrado(Apellido,Email));

}

- falta testear el strategy de 5 amigos en común, en caso de que no los tengan y no se agregue el amigo automáticamente.

En líneas generales está bien la entrega, muy buena documentación.

Lo que si, cuando ustedes dispongan antes de hacer la 2da entrega, corrijan los puntos que están resaltados.