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

_template.js usage #464

Closed
Martii opened this issue Dec 2, 2014 · 7 comments
Closed

_template.js usage #464

Martii opened this issue Dec 2, 2014 · 7 comments
Labels
DOC Pertains inclusively to the Documentation operations. team biz This is similar to a meta discussion.

Comments

@Martii
Copy link
Member

Martii commented Dec 2, 2014

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.

@Martii Martii self-assigned this Dec 2, 2014
@Martii Martii added DOC Pertains inclusively to the Documentation operations. team biz This is similar to a meta discussion. labels Dec 2, 2014
@Martii Martii changed the title _template usage _template.js usage Dec 2, 2014
@Zren
Copy link
Contributor

Zren commented Dec 2, 2014

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
Copy link
Member Author

Martii commented Dec 2, 2014

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
Copy link
Contributor

jerone commented Dec 2, 2014

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:

  • I'm +1 on keeping the template file, but not taking it literally.
  • I'm +1 on removing unused variables.
  • I'm +1 on keeping unused arguments.
  • I'm -1 on using == in favor of ===. It's always possible to convert to the same datatype.

@Martii
Copy link
Member Author

Martii commented Dec 2, 2014

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

@Martii Martii closed this as completed Dec 2, 2014
@Martii Martii removed their assignment Dec 2, 2014
@Zren
Copy link
Contributor

Zren commented Dec 2, 2014

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
Copy link
Member Author

Martii commented Dec 2, 2014

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
Copy link
Member Author

Martii commented Dec 2, 2014

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.

@OpenUserJS OpenUserJS locked as resolved and limited conversation to collaborators Apr 12, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
DOC Pertains inclusively to the Documentation operations. team biz This is similar to a meta discussion.
Development

No branches or pull requests

3 participants