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

Validación con Mixines cuando no se usan (shouldImplementAllMethodsInHierarchy) #211

Closed
PalumboN opened this issue Feb 13, 2024 · 4 comments
Assignees
Labels
bug Something isn't working component: validator Linter / Validator done A task is already done priority: medium A "should" issue, whenever we have the time to solve it

Comments

@PalumboN
Copy link
Contributor

PalumboN commented Feb 13, 2024

Con este repo del concurso: https://github.com/pdepmartestm/2023-tpgame-dulce-de-leche-tentacion

image

Es una jerarquía grande que va asignando atributos a medida que hereda (no me acordaba de ese feature, quizá eso lo rompa).
No hay mixines en el proyecto.

@PalumboN PalumboN added bug Something isn't working component: validator Linter / Validator labels Feb 13, 2024
@fdodino fdodino added the priority: medium A "should" issue, whenever we have the time to solve it label Feb 21, 2024
@fdodino fdodino self-assigned this Feb 21, 2024
@fdodino
Copy link
Contributor

fdodino commented Feb 21, 2024

Bueno, @PalumboN, ahí estuve viendo el issue:

  • el proyecto tiene packages duplicados: en el archivo constants.wlk define un package ENEMIES { que colisiona con la carpeta Enemies y adentro los archivos. Con lo cual todo se rompe mal y es imposible que el validador conteste algo razonable.

image

  • más allá de eso, me armé un proyecto aparte y logré replicarlo. Primero funciona correctamente con la jerarquía Enemy > GameVisual:

image

Y después cuando puse el método remove como abstracto, saltó la validación:

image

Qué se puede mejorar

  1. Cambiemos por supuesto el mensaje que no corresponde con el error (asume que la jerarquía es de mixines y se puede dar este caso de llamar a un método abstracto)
  2. Por otra parte, acá en ts deberíamos marcar únicamente la clase, mostrar todo es muy verboso y queda medio raro. Y de paso le pasamos cuáles son los métodos que tienen ese problema para darle contexto al usuario.

@fdodino
Copy link
Contributor

fdodino commented Feb 22, 2024

Algo así @ivojawer @PalumboN

image

@PalumboN
Copy link
Contributor Author

Buenísimo.

Pero el problema es el override o el super? Entiendo que debería ser el override.

Por eso yo propongo que la validación esté en el método, y no en la clase. Ahí se podría sobreescribir cada override incorrecto por separado.

@PalumboN
Copy link
Contributor Author

Ahh perdón ahí entendí. Bueno, entonces podría estar en el super :P

(Para cargar issue) 😃

PalumboN added a commit that referenced this issue Feb 28, 2024
…lMethodsInHierarchy

Fix #211 should implement all methods in hierarchy
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working component: validator Linter / Validator done A task is already done priority: medium A "should" issue, whenever we have the time to solve it
Projects
None yet
Development

No branches or pull requests

2 participants