Skip to content
This repository has been archived by the owner on May 7, 2023. It is now read-only.

[FEATURE] Add four node questions #32

Merged
merged 6 commits into from May 27, 2018
Merged

[FEATURE] Add four node questions #32

merged 6 commits into from May 27, 2018

Conversation

flxwu
Copy link
Contributor

@flxwu flxwu commented May 24, 2018

What does your PR belong to?

  • Questions / Answers
  • Website
  • General / Things regarding the repository (like CI Integration)

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • Enhancement (non-breaking improvement of a question)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have checked that the changes are working properly
  • I have checked that there isn't any PR doing the same
  • I have read the CONTRIBUTING document.

@fejes713
Copy link
Collaborator

Thanks a lot for dedicating time to this project and writing those questions. Everything seems okay to me, they could be a bit shorter but I think this is fine too.

There's just one thing I would like to point out before merging:

Good to hear section should represent short bullet points so we would need to change the following:

Take a look into the other questions regarding Promises!

This is just a convention. Node could use the convention that you suggest as well - with the exception that you wouldn't be able to return an error object as an intended value on success as you noticed, which may or may not be a problem, depending on your particular requirements.

Apart from those minor changes, everything is perfect. 👍
Thanks a lot for contributing.

Copy link
Contributor

@skatcat31 skatcat31 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not exactly certain these relate to NodeJS at all... While we can have ECMAScript/JS flavors of it as the question that is fine, but tagging the Node is doing a disservice to them...

@@ -0,0 +1,37 @@
### How can you avoid callback hells?

This comment was marked as spam.

@@ -0,0 +1,61 @@
### What is an error-first callback and why should I use it?

This comment was marked as spam.

This comment was marked as spam.

@@ -0,0 +1,22 @@
### In which states can a Promise be?

This comment was marked as spam.

@@ -0,0 +1,28 @@
### What are Promises?

This comment was marked as spam.

@fejes713
Copy link
Collaborator

@skatcat31 Any solutions to this problem then? I am sure we will run into those problems pretty frequently. I never planned on adding other languages so everything associated with JS and Node went to js and node categories. We're also using JS as an example in every code.

But as we discussed earlier, contributors will come and we will introduce new categories very soon.

To me, it is stupid to put this into general for some reason. We're also going to have way more basic questions that are going to be in general category. For example: What happens when a user enters an address into search bar.

@skatcat31
Copy link
Contributor

skatcat31 commented May 24, 2018

We could have a Programming category that deals with mainly abstracted high level ideas and tag probably most of them as advanced. This would include questions about recursion, datatypes, and such that aren't really langauge specific. As for these Node questions, I'd be more to say they should be tagged as JS

Reason I say that is a node question is more along the lines of

When is it appropriated to use a synchronous file-system call versus an asynchronous file-system call?

That's a question that is really server specific, and in this case fits perfectly into the node category(or a server category if we choose to implement that)

But we can really target it at node by asking

When is it appropriate to use fs.readFileSync instead of fs.readFile?

@flxwu
Copy link
Contributor Author

flxwu commented May 25, 2018

Yeah right so maybe these have to be put into javascript or general and rather questions like the one @skatcat31 mentioned for node, i.e. streams and so on or regarding npm/yarn

EDIT
I think that the promise questions can clearly be put to javascript, but callback hells and error-first callbacks... I'd leave them in node, simply because you're most likely to be asked that question in a node interview I think, and the error-first callback question is about the node.js convention

@fejes713
Copy link
Collaborator

@flxwu can you fix mentioned things so we can merge then?

@skatcat31
Copy link
Contributor

@flxwu I'd be fine with that, but we should direct the question about Callback Nesting a little bit more directly as Node by changing it a bit to read:

NodeJS uses a callback pattern in many instances where if an error were returned it will pass it as the first argument to the callback. What are the advantages of this pattern and what ways can you avoid nesting callbacks inside of each call?

Advantages include:

  • Not needing to process data if there is no need to even reference it
  • Having a consistent API leads to more adoption
  • Ability to easily adapt a callback pattern that will lead to more maintainable code

Avoiding nesting:

  • Properly returning from a callback to composition chain(hardest, most advanced, will show an older developer/functional programing)
  • Promisification using a Promisifer(like Bluebird.fromCallback)
  • Declaring all callbacks in the chain separately and delegating a control function to properly scope and assign data

The problem is that question is so nuanced I'm not sure it's easy to just explain... We could do a medium article about more information and point to it under the question and note how this specific example is just one of many ways to approach the problem and just stick with that...

I could literally write paper after paper about error handling in just NodeJS or Closure based languages of which PHP, Python, C/++, Scala, Java, Erlang, Elm, Haskell, so many others all apply that it could be a career

@flxwu
Copy link
Contributor Author

flxwu commented May 27, 2018

@fejes713 , yeah, working on it 👍

@fejes713
Copy link
Collaborator

Once you're done, I am merging this right away. Then we are just 2 questions away from our 1st milestone (50 questions for launch)

@flxwu
Copy link
Contributor Author

flxwu commented May 27, 2018

@skatcat31 Actually, after googling around a bit, the term callback hell is originated by JavaScript developers and specifically means callback nesting in JS, as in Python i.e. you usually don't use callbacks but rather event loop coroutines or kind of "implicit" callbacks... and regarding the fact that we are focussing on web, it makes more sense to me to just make avoiding JavaScript callback hells as a JS question

or do you strongly disagree? @skatcat31

and further, we are lacking node.js questions haha

@fejes713
Copy link
Collaborator

and further, we are lacking node.js questions haha

This is very true and sad at the same time. I would personally stick with felix's opinion on this one.

Down the road, we will eventually make a general category and try to generalize node/js questions that are similar to other languages 👍

@flxwu
Copy link
Contributor Author

flxwu commented May 27, 2018

@fejes713 Your opinion regarding the current questions? Just changed the error-first callback question title to @skatcat31 's suggestion

Copy link
Collaborator

@fejes713 fejes713 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now it is way better 👍 Good job

@fejes713 fejes713 merged commit f462aca into master May 27, 2018
fejes713 added a commit that referenced this pull request May 27, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants