4 ms·

Unfortunately the repo has some truly awful antipatterns. The catalog service is a masterclass in how not to write Node.

anonytrary 7y ago
I think the controller/service abstractions are completely unnecessary and cause unwanted indirection. Otherwise, this looks like a typical CRUD service -- with custom logging, database model schemas, authentication, and routes that utilize all of the above.
- ozfive 7y ago
  So fix it!!! It's open source. Put some work in, or your complaints fall on deaf ears.
  - anonytrary 7y ago
    You're assuming quite a lot. First, I'm not saying it's broken, so what is there to "fix"? Second, I'm not using this library, so why would you expect me to submit a PR? People aren't expected to contribute to libraries just because they noticed a smell in the code. Open source is about contributing to stuff you use. You don't have to (and shouldn't be expected to) fix everything you see broken in in the world.
ozfive 7y ago
Are you even going to write a line of code to fix any of the 'awful anti-patterns' you mention or am I just going to get downvoted on Hacker News. For Christ's sake, this is why open source is failing. You all like to complain more than you actually do anything at all to fix things. Exactly the reason I never open source a line of code. There is no true community it's just a bunch of wankers that would rather complain about anti-patterns then make things better. Fuck.
preommr 7y ago
seems pretty normal, what's so bad about it?
- dfcowell 7y ago
  Thanks for the question - for clarity, this is the file I'm referring to in this reply: https://github.com/Viveckh/Veniqa/blob/develop/shopping-server/services/catalogService.js https://github.com/Viveckh/Veniqa/blob/develop/shopping-serv... 1. The service is a leaky abstraction, it knows about HTTP status codes and presumably the response body format. The point of separating concerns is to make code reusable, HTTP response codes don't make sense outside of a HTTP call. Similarly, any calling code needs to know that `sortRule` is expecting a MongoDB sort parameter. 2. The control flow is super unclear due to mixing async/await and Promise API coding styles. The `.catch()` call on the promise chain means that the `catch` block will never be hit and the API will always return OK, with the error object exposed to the client in the `responseData` - likely exposing implementation details about the server via the stacktrace on the error object. There are other nits I could pick, but those are the most important issues and my other criticisms are more a matter of preference.

anonytrary 7y ago

I think the controller/service abstractions are completely unnecessary and cause unwanted indirection. Otherwise, this looks like a typical CRUD service -- with custom logging, database model schemas, authentication, and routes that utilize all of the above.

ozfive 7y ago
So fix it!!! It's open source. Put some work in, or your complaints fall on deaf ears.
- anonytrary 7y ago
  You're assuming quite a lot. First, I'm not saying it's broken, so what is there to "fix"? Second, I'm not using this library, so why would you expect me to submit a PR? People aren't expected to contribute to libraries just because they noticed a smell in the code. Open source is about contributing to stuff you use. You don't have to (and shouldn't be expected to) fix everything you see broken in in the world.

ozfive 7y ago

Are you even going to write a line of code to fix any of the 'awful anti-patterns' you mention or am I just going to get downvoted on Hacker News. For Christ's sake, this is why open source is failing. You all like to complain more than you actually do anything at all to fix things. Exactly the reason I never open source a line of code. There is no true community it's just a bunch of wankers that would rather complain about anti-patterns then make things better. Fuck.

preommr 7y ago

seems pretty normal, what's so bad about it?

dfcowell 7y ago
Thanks for the question - for clarity, this is the file I'm referring to in this reply: https://github.com/Viveckh/Veniqa/blob/develop/shopping-server/services/catalogService.js https://github.com/Viveckh/Veniqa/blob/develop/shopping-serv... 1. The service is a leaky abstraction, it knows about HTTP status codes and presumably the response body format. The point of separating concerns is to make code reusable, HTTP response codes don't make sense outside of a HTTP call. Similarly, any calling code needs to know that `sortRule` is expecting a MongoDB sort parameter. 2. The control flow is super unclear due to mixing async/await and Promise API coding styles. The `.catch()` call on the promise chain means that the `catch` block will never be hit and the API will always return OK, with the error object exposed to the client in the `responseData` - likely exposing implementation details about the server via the stacktrace on the error object. There are other nits I could pick, but those are the most important issues and my other criticisms are more a matter of preference.