Dependency Injection Vs. Closures. Which is the better model for SockBot to use for providers?

[yamikuronue]

The bindX pattern is pretty weird for new developers. We should instead pass the forum instance into the constructor or provide a static method to set it once so we have a pure class-based implementation.

[AccaliaDeElementia]

hmm... agreed it's a weird pattern.

the reason i went with it is i wished to have support for multiple logins operating at the same time.

for example bots A and B

each bot will have its own Forum instance, each of which has a reference to the Post class. If A has more rights on the provider than B then A needs to make requests that would fail if B makes the same requests.

I'm not opposed to adopting a different provider model, but i'm not seeing a way to solve that business requirement without using closures (the bindX pattern) or abusing the internals of nodejs's require() functionality (a complete non starter in this case, too prone to failure in the face of implementation differences between versions)

[yamikuronue]

Why can't the Forum instance be injected into the constructor upon creation, so that Post class A has a dependency on Forum A injected and Post class B has a dependency on Forum B injected?

[AccaliaDeElementia]

Yami, do you have the time to whip up a super simple example of what you're thinking of with this DI suggestion?

your suggestion intrigues me, but my brain is currently refusing to bend enough to wrap around the idea... silly brain, you don't have a say in what i do or do not do..... oh, wait, you do..... whoopsies.

[yamikuronue]

Yeah, something like so: f2e135c

[yamikuronue]

Or, for the closure issue, d72eddd

[AccaliaDeElementia]

that closure suggestion solves the issue on instances of Post, what about for the static methods?

... the more i look at this the more i think.... "Why exactly are we jumping through all these hoops to support multiple bots per process in core? isn't that something i should have YAGNI'd in the design phase?"

i mean multiple instances per process are cool and all, but what do we really use that for anyway?

[AccaliaDeElementia]

hmm. I can see there's more thought to be had here. both the closure method (the bindX methods) and Dependency Injection have their pros and cons in this matter.

For now I'm going to defer this issue as we have some features to work on right now to add functionality for sockMafia, and some other projects, but we should circle round on this one in a bit to see if we can figure out if there's a way to accomplish what's needed in the sockbot API in a cleaner manner than it is currently implemented.

[AccaliaDeElementia]

@yamikuronue i think this issue can be closed now, we still have a discussion to be had about dependency injection compared to closures for architecture, but the current implementation seems to be functional with other providers cropping up using the sockbot architecture.

Examples include:
SockDrawer/Sockbot-Slack for using sockbot in Slack
yamikuronue/sockbot-irc for using SockBot in IRC

Do you agree with my assessment here?

[yamikuronue]

I know with 100% certainty that the maintainers of those other providers are willing to change implementations. Both of them simply copied the existing architecture since it was supported.

[AccaliaDeElementia]

well, yes i'm sure she is.

would she be interested in leading the architecture change for SockBot 7.0 or higher? :-)