actum / gulp-dev-stack

Actum dev stack based on gulp
MIT License
11 stars 7 forks source link

Merge init and factory functions? #177

Closed zmrhaljiri closed 7 years ago

zmrhaljiri commented 7 years ago

Idea

What about to use only init() with the logic distinguishing single/multiple calls within? Would be good idea to do the following checks?

(Also in that case return fn(container, ...args) could be stored in a separate function to not repeat itself)

Pros

Cons

janpanschab commented 7 years ago

There are cases where you want to initialize only one instance and pass multiple containers/inputs. I have use it several times.

zmrhaljiri commented 7 years ago

I see. Then I would suggest to pass an additional parameter to the init() function and then add id to the check. This way I will use only one file containing the shared code inbetween current init() and factory() functions. What do you think?

janpanschab commented 7 years ago

I don’t understand. Give me an example, please.

kettanaito commented 7 years ago

@zmrhaljiri I completely agree to have one init method controllable through custom options. This is simple, intuitive, and decreases the chance of using init/factory wrong, as opposed to now.

Please correct me if I get you wrong, I think you are talking about something similar:

export default function init(component, containers, options) {
  const { factory } = options;
  if (factory) {
    return component(containers);
  } else {
    return Array.from(containers).forEach(container => component(container));
  }

  // or
  return options.factory ? component(containers) : Array.from(containers).forEach(container => component(container));
}
DavidSlavik commented 7 years ago

@janpanschab Can I ask you for some feedback?

janpanschab commented 7 years ago

@zmrhaljiri @kettanaito @DavidSlavik @vbulant I don’t see a big difference between these two approaches, but why not. Maybe only renaming the factory function would solve the problem.

In general I don’t like monster functions with too many arguments and huge options. It could be really nice small two functions. But definitely I don’t want to block this. As I said. It is not a big change IMHO.

DavidSlavik commented 7 years ago

From my perspective using of two functions what is similar, but different is more readable than joined functionality with many arguments. Actually init and factory functions are called with many parameters and I don't want to add another one. App.js file is more readeble, because I'm able to read if component can be many time on page or once. I prefer actual solution.

@ronaldruzicka @omladek please give me feedback.

omladek commented 7 years ago

I am fine with the current state as it is (separate init and factory and render).

ronaldruzicka commented 7 years ago

I am also fine with the current solution with init and factory as separate functions.

DavidSlavik commented 7 years ago

Ok, we leave it is now.