Entrega:

Antes que nada muy bien que presentaron en fecha y cumpliendo todas las reglas, esto es importante y esperamos lo mismo para cada una de las entregas.

Ahora vamos a pasarles algunos comentarios puntuales:

- Archivos de eclipse subidos al repo. Los archivos de la idea (en este caso .project y .classpatch) es recomendable mantenerlos en cada pc, ya que diferentes personas del mismo equipo pueden estar usando diferentes configuraciones o IDEs por lo que el repositorio debería ser agnóstico de las mismas. Algo que puede ayudar a la hora de subir el código por primera vez, es hacer un mvn eclipse:clean (o su equivalente en otras IDEs) y luego de comitear y regenerar las configuraciones un svn:ignore de las mismas.

- Muy buenos los diagramas, pero no hay Javadoc :( El javadoc es importantísimo para poder entender un proyecto. Es muy importante realizarlo. Si por algún impedimento (como por ejemplo el tiempo) no se puede hacer (aunque esto es un problema que no debe ocurrir y se tiene que manejar en otras esferas, por ejemplo políticas o administrativas) , es importante al menos que las clases y los métodos principales lo tengan. No se olviden que muchas veces nos va a tocar el rol de ser "guardianes de la calidad y el código" si nosotros no nos ponemos firmes y no tenemos ojo crítico para rechazar la idea de no escribir comentarios, después van a pedir que no hagamos tests, y que la aplicación tenga una calidad inferior en pos de reducir costos y tiempo.

- Una cuestión técnica. El GenericDao tiene un méotdo findById que recibe un id, y su clase.
1) No se cierren a que el id va a ser un long
2) Porque recibe la clase? Si el DAO usa Generics y ES de una clase en particular. Si tengo ManzanasDao, le voy a pedir que me encuentre una pera por ID? No.
El problema de saber la clase sobre la que trabaja el DAO que usa generics es muy común. Acá hay un link para que vean. Presentan una solución hacker, pero necesaria y que queda muy elegante y cómoda de usar.

http://community.jboss.org/wiki/GenericDataAccessObjects

- No hace falta que haya un paquete test, por lo generar los tests se ponen en el mismo paquete que las clases que están testeando, pero en la carpeta test en vez de la carpeta main.

- CategoriaResource está conociendo PiezaEnMemoriaDao, manejense al nivel de la interfaz.

- En los test están levantando un contexto de spring para testear. No está mal, pero tampoco descarten la opción de generar sus objetos y dependencias a mano.
Por ejemplo:

        srv = new PiezaServiceImpl();
        srv.setDao(new PiezaDaoMock());

A veces da un poco más de claridad sobre lo que se está testeando, y se ahorran la verbosidad y tener que tocar varios archivos para un test.

Lo que no nos gustó mucho es que mezclen ambos approaches, por ejemplo en ConsultaPiezasPorCategoriaTest donde tienen mocks y sacan cosas del contexto.

- Tambien con respecto a los tests, si bien por el momento no hay grandes lógicas ni funcionalidades, al hacer TDD pueden ir haciendo test más chicos que abarquen cada clase. No está del todo mal, pero tengan cuidado de no terminar teniendo 3 tests monstruo para toda la aplicación.

- PiezaEnMemoriaDao no tienen test, por más que sea temporal deberían garantizar que responde como esperan. Por otro lado, al hacer TDD deberían arrancar el proceso de desarrollo con un test.

- Los resources de REST deberían tener un test que garantice que hacen lo que ustedes definieron que van a hacer.

- Con respecto al modelo de datos, nuestra sugerencia es que lo dejen completamente de lado, se enfoquen en el modelo de objetos, y luego dejen que Hibernate haga sus magias. En caso que no les guste el output o quieran tunearlo, pueden configurar Hibernate. La ventaja de ir en este sentido es que no se van a tener que pelear con mapeos extraños de forma innecesaria.

Una aclaración más, tengan cuidado como se maneja esto, porque como cliente yo podría decir que al haber recibido el modelo, ya puse a mi equipo de DBAs a trabajar en el mismo, lo cual los condicionaría en iteraciones posteriores para cambiarlo y realizar los mapeos de hibernate lo cual seguramente les agregue complejidad.

- Tengan en cuenta que aun no hay logs, es algo que pueden decidir hacer como una tarea independiente o como una parte implícita en cada tarea.

- Como último punto de la entrega, al ejecutar mvn jetty:run la aplicación no levanta, por lo que no pudimos probar los servicios REST. Les pido que en las próximas iteraciones agreguen aunque sea un pequeño txt indicando las instrucciones para deploy. Sobre este problema, cuentennos como están levantando la aplicación así podemos probarlo.

Estaría bueno, si el martes van todos a clase hacer una pequeña reunion de 10 minutos en forma de retrospectiva de la iteración, para que nos cuenten un poco como vienen, que piensan que salió bien y que podemos mejorar en las siguientes.

Una última cosa, las tareas para la iteración 2 están bien, pero hay una que no se pideSi quieren hacerlo no hay problema, pero sepan que como clientes (o product owners) no nos va a agregar ningún tipo de valor.
En vez de eso queremos que elijan otras tareas, y si tienen ganas aparte pueden hacer (gratis) la interfaz web.

Por favor miren el punto que está con negrita porque es lo que necesitamos para cerrar la entrega.

Saludos
Seba y Nico


Correccion de la entrega:

Ok, ahora si levanta Jetty.

Muy bueno que ya tengan los servicios REST in place. Espero que la clase del jueves les haya servido para sacarse dudas y entender algunas cosas desde un punto de vista más teórico.

Para de issues para ver

1) /piezas me da un listado de Ids. Está bueno esto? O mejor me da un listado de piezas?
2) Esto es para pensar muy a futuro. Cuando haya 100.000 piezas en el sistema, está bueno que las pueda listar todas juntas con /piezas? Hay estrategias de paginación que se usan, tenganlo como un item muy low priority en el backlog y en caso de que el cliente considere que es importante, lo atacamos.
3) Si pido una pieza que no existe tira un error 500 y muestra el stacktrace. Debería ser un error 404 (resource not found)
4) Si pido /piezas/XXXXX y XXXXX no es un número me tira de vuelta 500 con stacktrace. Debería devolver un error 400 de HTTP (bad request)
5) Si pido una categoría que no existe pasa lo mismo que en el punto 3)
6) Otra tarea low priority. El xml que devuelve al consultar piezas de una categoría no quedó muy lindo. Tienen que resolver el tema de las referencias circulares, y que no esté tan acoplado a su implementación (por ejemplo <ar.edu.utn.frba.tacs.