MumukiProject / mumuki-guia-java-practica-polimorfismo

Creative Commons Attribution Share Alike 4.0 International
1 stars 1 forks source link

Bug por visibilidad de métodos entre ejercicios 5 y 6 #1

Open mgarciaisaia opened 3 years ago

mgarciaisaia commented 3 years ago

👋 ¡Hola! :)

Esta solución del ejercicio 5 de la guía (el de JugoSimple) está, en lo que a la lección respecta, joyita.

Pero hay dos problemas cuando pasamos al ejercicio 5 con esa solución.

El primero es que, al hacer la solución "correcta" del ejercicio 6, falla por problemas de visibilidad de métodos:

mumuki io_matias-garcia_exercises_4480-objetos-con-tipado-estatico-practica-polimorfismo-jugo-de-frutas

¡Ups! Tu solución no se puede ejecutar Problemas que encontramos: El método `jugo()` en la clase `Manzana` debería ser público. Revisá si tiene la visibilidad correcta cerca de `int jugo() { return 80; }`. Detalles
Resultados:
/tmp/SubmissionTest.java:40: error: jugo() in Manzana cannot implement jugo() in Fruta
  int jugo() { return 80; }
      ^
  attempting to assign weaker access privileges; was public
/tmp/SubmissionTest.java:39: error: esAcida() in Manzana cannot implement esAcida() in Fruta
  boolean esAcida() { return false; }
          ^
  attempting to assign weaker access privileges; was public
/tmp/SubmissionTest.java:45: error: jugo() in Naranja cannot implement jugo() in Fruta
  int jugo() { return 105; }
      ^
  attempting to assign weaker access privileges; was public
/tmp/SubmissionTest.java:44: error: esAcida() in Naranja cannot implement esAcida() in Fruta
  boolean esAcida() { return true; }
          ^
  attempting to assign weaker access privileges; was public
/tmp/SubmissionTest.java:50: error: jugo() in Pomelo cannot implement jugo() in Fruta
  int jugo() { return 130; }
      ^
  attempting to assign weaker access privileges; was public
/tmp/SubmissionTest.java:49: error: esAcida() in Pomelo cannot implement esAcida() in Fruta
  boolean esAcida() { return true; }
          ^
  attempting to assign weaker access privileges; was public
6 errors

En el ejercicio 5 funcionaba todo bien porque a los miembros de Fruta los usa JugoSimple, que está definido en el mismo archivo. Pero acá parece que se mueve a un archivo separado (el de la Biblioteca), y entonces ahora sí necesita sí o sí que los métodos sean public. Para que el ejercicio 6 funcione, hay que tener esta solución en el Ejercicio 5.

Para peor, el problema no está en el código de la solución de este ejercicio, si no del anterior. Hay que darse cuenta de ir a corregir en el ejercicio anterior cosas que antes andaban joya, pero ahora ya no sirven tanto (y encima es un problema de visibilidad, que entiendo que medio que no es el foco del ejercicio, ni es algo a lo que se le de mucha importancia a lo largo de la lección).

Por último, la frutilla del postre es que, si te diste cuenta de que tenías que ir al ejercicio anterior a corregir algo, lo cambiás allá, volvés al ejercicio 6 y le volvés a dar Enviar (porque tu solución estaba bien), Mumuki ve que el código de la solución que mandás es el mismo que ya habías mandado, y entonces no vuelve a procesarlo. Es decir, tiene cacheado el Envío, pero cambiar la Biblioteca (que modifica el resultado del ejercicio) no invalida esa cache - y debería.

Como propuestas de solución, agregaría algún aviso en el Ejercicio 5 reforzando que los métodos tienen que ser public para evitar problemas con el ejercicio siguiente (y, de ser posible, le agregaría validaciones al ejercicio para chequear que los métodos de la solución efectivamente sean públicos). Eso, y/o agregar una nota en el Ejercicio 6 recordando que "si tenés problemas de visibilidad de métodos, revisá de vuelta el código del ejercicio anterior".

Lo de la cache lo considero un bug de Mumuki (de la plataforma), ¿no?


PD: La radio está buenísima. Ojalá lo sepan, pero en vez de confiar en que lo sepan, vengo a asegurárselos. Gracias, miles ❤️

flbulgarelli commented 2 years ago

Como propuestas de solución, agregaría algún aviso en el Ejercicio 5 reforzando que los métodos tienen que ser public para evitar problemas con el ejercicio siguiente (y, de ser posible, le agregaría validaciones al ejercicio para chequear que los métodos de la solución efectivamente sean públicos).

Sí, esto definitivamente. Hoy la única forma de resolver esto sería usando reflection en el test. Si te animás a mandarte un PR, bienvenidísimo!

Lo de la cache lo considero un bug de Mumuki (de la plataforma), ¿no?

Sí, mas bien se trata de un known issue con el que por ahora hemos decidido "convivir". Quizás en el futuro eliminemos esa caché definitivamente.

flbulgarelli commented 2 years ago

@nadiafinzi esto es justo lo que hoy hablábamos. Otro ejercicio para revisar si queremos que mantenga referencias al código previo.

mgarciaisaia commented 2 years ago

Sí, esto definitivamente. Hoy la única forma de resolver esto sería usando reflection en el test. Si te animás a mandarte un PR, bienvenidísimo!

No prometo nada, pero si me tirás algún centro sobre por dónde podría mirar para inspirarme, quizá eventualmente pueda hacer algún intento. Ahora, así, en el aire, sería tres bandas de esfuerzo para empezar a ver qué tocar.

Sí, mas bien se trata de un known issue con el que por ahora hemos decidido "convivir". Quizás en el futuro eliminemos esa caché definitivamente.

¿Hacer que la key de esa cache dependa también de la Biblioteca no es una opción? Estimo que en algún lado estarán hasheando el código, habría que sumarle un hash de la Biblioteca (que en la mayoría de los otros ejercicios sería constante, así que funcionaría igual que ahora).

mgarciaisaia commented 2 years ago

Sí, esto definitivamente. Hoy la única forma de resolver esto sería usando reflection en el test. Si te animás a mandarte un PR, bienvenidísimo!

No prometo nada, pero si me tirás algún centro sobre por dónde podría mirar para inspirarme, quizá eventualmente pueda hacer algún intento. Ahora, así, en el aire, sería tres bandas de esfuerzo para empezar a ver qué tocar.

¡Ahhhhhhh! Ahí entendí: decís que modifique el tests.java para que assertee la visibilidad via reflection de Java, porque el frameworkcito de expectations no soporta versar sobre esto.

Buen, tengo una punta ya. Veré qué onda.