OpenUserJS / OpenUserJS.org

The home of FOSS user scripts.
https://openuserjs.org/
GNU General Public License v3.0
858 stars 305 forks source link

_template.js usage #464

Closed Martii closed 9 years ago

Martii commented 9 years ago

So with the most recent JSHint fixes in #458 ... policy needs to be re/established with not including the defaults in applicable .js...

As I have stated before aRes, aReq, aNext should always be included... however #458 removes things such as tasks, etc.

It is great to optimize however sometimes it's necessary to keep things in for certain backend host provider solutions such as isPro, isDev and isDbg... e.g. those stay in for #430 and further changes in the backend.

As sizzle mentioned sometimes there is a slight derivation from the STYLEGUIDE.md regarding == and ===... I do know that the metadata block parsing routine relies on this a bit in the past.

Anyhow... the real policy discussion needed now is whether or not to keep existing controllers with nearly the full compliment of what is outlined in the ./controllers/_template.js.

This is team biz because it needs some defining for the DOCs.

Zren commented 9 years ago

not including the defaults in applicable .js

?

removes things such as tasks, etc.

Meh. I wrote the _template.js to teach others how to make consistent controllers. The task variable is only needed when you are batching asynchronous tasks before sending a response. It's not needed when you're doing a synchronous response.

whether or not to keep existing controllers with nearly the full compliment of what is outlined in the ./controllers/_template.js.

Keep consistent with the coding style in it. You can stray from it as need persists. There are enough eyes on new controllers being made that we don't need a huge lists of rules on their creation. So long as it's well written code there's no problem.

Martii commented 9 years ago

I wrote the _template.js to teach others how to make consistent controllers.

And I appreciate that.

There are enough eyes on new controllers being made that we don't need a huge lists of rules on their creation.

Structure is required with any project... so the "huge" set of rules that you are clearly overly dramatizing isn't as huge as you think. If we didn't need structure we wouldn't have any sort of STYLEGUIDE, CONTRIBUTING and it would all be spaghetti code... so your opinion is notated once again on this... but the DOCs will have something on it. I'm not against removing unused elements but I'm also not for it either... e.g. I'm at 0.

Part of the STYLEGUIDE is using braces {} which can prevent errors with semicolons as well as make it easier for some (not for myself in my non-OUJS code because I only use them when I need them and it's not that difficult to put them in or take them out imho). Also some styling is good for teams so we can all read the code. I've already told sizzle years ago it's hard as hell to read some of his code and I usually match his condensed code nature in his... in this case I'm matching OUJS styles because that's what's needed for teamwork.

jerone commented 9 years ago

Definition of the word "template":

"A document or file having a preset format, used as a starting point for a particular application so that the format does not have to be recreated each time it is used" - http://www.thefreedictionary.com/template

It's a starting point. Why keep unused code in there, and confuse other developers why a peace of code is not being used and reserves resources.

I like the template file as a starting point, but I learn more from existing code which actually does things.

Points discussed with my opinion:

Martii commented 9 years ago

I like the template file as a starting point, but I learn more from existing code which actually does things.

And I was exactly the opposite on this with the controllers. This is why it is better to describe things for potential newcomers that learn differently. In particular it has been mentioned in #262 that:

Identify misplaced controllers (the ones dealing with scripts in controllers/user.js jump to mind) and move them to the appropriate file

... This is still a little vague to me especially since all of you have constantly used the word "controller" when there are what I have recently been calling "handlers" in each "controller" (e.g. I get to convert someones terminology to something that makes sense to me and perhaps others) ... Zren barked at me one time for creating a new controller when that didn't make much sense to take it out of the admin controller... e.g. he wanted a new handler inside of it.

Thank you both for your input and I have enough to fill in a short diddy in the Docs when I get around to that.

Anyhow

Zren commented 9 years ago

Zren barked at me one time for creating a new controller when that didn't make much sense to take it out of the admin controller... e.g. he wanted a new handler inside of it.

Are you talking about #318, where you wanted to do a if (schema modelName exists) { get document from db } else if (modelName == "npmlist") { create a new process to run a command and send the response } and then I told you to make a new route/controller to do that?

This is still a little vague to me especially since all of you have constantly used the word "controller" when there are what I have recently been calling "handlers" in each "controller"

Model View Controller is a software pattern (MVC). (Edit: Noticed you mentioned MVC elsewhere so you're probably familiar with it)

The Model is the mongoose schema/documents. The View is the Mustache templates. The Controller is the ExpressJS route handlers functions. This is a common pattern with webserver frameworks.

We currently bundle a bunch of route handlers in the same javascript module since they manipulate the same Model. user.js sorta bundles a lot more controllers than just those that manipulate the User model. One of the patterns I'm trying to introduce is 1 route handler per file in order to prevent things like user.js getting too big.

Edit: Actually, I'm not quite sure if the "Controller" refers to the file, or the functions themselves. So calling a route handle a controller might be incorrect, but whatever.

Martii commented 9 years ago

Are you talking about #318

Yes. I don't know if "handler" is the correct nomenclature but it's what I'm using now... I'm open to better terminology because clearly it is lacking a descriptive nature at wikipedia when Jerone presented it the first time months ago... this would have eased some confusion at first back then but we worked it out.

Martii commented 9 years ago

One of the patterns I'm trying to introduce is 1 route handler per file in order to prevent things like user.js getting too big.

Which is mostly synonymous with #262 (and open to do preferably incrementally btw save for vote/flag/remove) although this might be interesting with some of the handlers being split up into little pieces filename wise.