some refactor-love ?

[ahmadnassri]

I'm quite tempted to do a full code re-factor on this entire project, specifically: separate each endpoint into its own self-contained in-project-module

benefits of course are:

• readability
• separate tests per module
• abstract some utility methods

before I attempt such a task, what's the likely hood of this being an accepted PR, or do you like to keep things as they are now?

[kyrias]

I don’t know about @espadrine, but it sounds like a very good idea to me.

[ahmadnassri]

@espadrine let me know your thoughts, happy to take on this one ...

[espadrine]

Moving code into separate modules is not a good idea. For historical reasons, quite a few of those endpoints can match the same URL, for varied URLs. The only reason they work is then the order in which they are defined. Obscuring that order would make debugging harder.

At the bottom of the file, there are a bunch of utility functions. Those can probably be factored in a different file, say, util-server.js. I don't feel like it would be of any use, but if well done, I wouldn't refuse it.

Testing is of course imperfect (as in, manual). The CLI and backend engine have tests, but the server does not. What would be helpful however isn't tests here; it would be automated error reporting. We know that a list of badges should work. Having a program that goes through them and reports what the vendor server sent back if the badge failed would be great. However, it should absolutely not complexify the existing logic of the server. That's also a nice-to-have; it wouldn't help development (copying the vendor URL and filling the incriminating values would still have to be done). That work would be useless, but you can do it if you want.

To be honest, I didn't bother doing so because contributions are not an issue. We've had loads from day 1, and they aren't too hard to review, even though some mistakes get made. The hard work and blood is in the backend, package management, puzzle work of keeping everything working together. server.js is like an amusement park, messy but fun, and it gets work done.

Something that would have helped me (and, I believe, contributors) a lot is this: #276 but the patch wasn't finished. If you simply finished that patch, that would help the project a lot. Anything else would be like a third wheel.

[ahmadnassri]

The only reason they work is then the order in which they are defined. Obscuring that order would make debugging harder.

you can keep the routing info in the main file, with the same order and abstract away the resolving function itself into a self-contained module, write tests for the function itself in isolation.

doing so would also help in writing an error reporting system much like you described.

I'll definitely take a look at #276, is certainly the starting point to clean up syntax before any type of re-factoring.

I'll pitch this:

• I'll get 276 cleaned up and done.
• group / abstract utility functions
• then start a branch with a suggested re-structure i'm describing and showcasing the values gained with tests.

[ahmadnassri]

how do you feel about standard, can add to npm test that and apply coding style fixes? takes care of #276

(there's also semistandard if you prefer to keep those semi-colons)

[jirutka]

I totally agree with @ahmadnassri. File server.js has 4 175 LOC – this is insane!

Moving code into separate modules is not a good idea. For historical reasons, quite a few of those endpoints can match the same URL, for varied URLs. The only reason they work is then the order in which they are defined.

Not a good idea? Just opposite, this is very strong reason to refactor this mess.

[espadrine]

File server.js has 4 175 LOC – this is insane!

Does the file size make it harder for you to write patches? I am honestly asking. I tend to jump around easily by relying on my editor's regex find and on grep, and my editor is far from having trouble with that file size, but I know everyone has their own workflow.

Not a good idea? Just opposite, this is very strong reason to refactor this mess.

I'm not sure what you mean. We could complexify the regexes to ensure that they never overlap, but that sounds like a lot of work; is that what you are suggesting?

[jirutka]

Does the file size make it harder for you to write patches? I am honestly asking.

It’s not just about the file size, but also code structure, code-quality etc. Since you’re honestly asking, then yes, it’s so bad that it really discourages me from contributing.

[espadrine]

Could you detail what you would like to see changed in the code structure?

What would you like to contribute?

[paulmelnikow]

This has been picked up in #948. Please join us there!