Refactor towards three-layer architecture

[alandotcom]

Create app container
Pass services down into lower layers (lower layers don't require singletons from src/services)

TODO:

• Move common utils (e.g., services/log) to common

[alandotcom]

cc @gip -- config is now passed down from top layer

[MatthewPhinney]

Is this doable with with _.partial, as in app.js (for consistency)?

[alandotcom]

I just grepped for addLedger and it's not used

[sentientwaffle]

It isn't used internally, which is why it is exported.

[alandotcom]

Where is it used? (Curious)

[sentientwaffle]

https://github.com/ripple/five-bells-connector-lib-ripple

[clark800]

I’m pondering the DI structure. One thing I've noticed is that refactoring things like this can be quicker if you use keyword arguments because for example if you want to delete one of them, you can just delete it where it is used and where it is passed in and you don’t have to change all the intermediate functions that were just passing it through. So we might want to consider using a "context" object to hold all the dependency injection objects. It also gives you a logical separation between the DI parameters and non-DI parameters.

[clark800]

I'm not sure if we're getting much advantage to passing the backend around as a parameter because the config already specifies the backend, so wherever you need the backend you could just call a function that loads the proper backend based on the config. Or am I missing something? Also, I haven't figured out what the ledgers parameter is used for yet. I'm wondering if there is a way we can avoid passing that around too.

[alandotcom]

@clark800 I think maybe I can followup with a later commit to avoid needing to pass some things down. The config specifies a backend type, which is then require'ed in services/backend.

[emschwartz]

Could you provide more context for the purpose of this refactoring? Is there a specific issue that motivated it or is it just to comply with the 3-layer architecture design?

[clark800]

Also, we could even eliminate a lot of the config parameters by passing it at the module level like this

const makeExports = (config) => {
  getX: (param) => {
    ... // have access to config here even though it isn't a parameter to the getX function
  }
}

module.exports = makeExports

[alandotcom]

@clark800 I like that approach.
@emschwartz discussion is here interledger-deprecated/five-bells-ledger#112

[MatthewPhinney]

Is there a specific issue that motivated it or is it just to comply with the 3-layer architecture design?

@emschwartz my understanding was that this was partly motivated by the desire to unit test different quoter backends. With this refactor, we can pass the config as a param into the instantiation of the app, allowing us to switch quoter backends. See here. Is this correct @LumberJ?

[emschwartz]

Sounds like a use case for https://github.com/justmoon/reduct

On Wed, Mar 23, 2016 at 3:15 PM, Matthew Phinney [email protected]
wrote:

Is there a specific issue that motivated it or is it just to comply with
the 3-layer architecture design?

@emschwartz https://github.com/emschwartz my understanding was that
this was partly motivated by the desire to unit test different quoter
backends. With this refactor, we can pass the config as a param into the
instantiation of the app, allowing us to switch quoter backends. See here
https://ripplelabs.atlassian.net/browse/RILP-284. Is this correct
@LumberJ https://github.com/lumberj?

—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#122 (comment)

Evan Schwartz | Software Architect | Ripple
[image: ripple.com] http://ripple.com

[alandotcom]

@MatthewPhinney Yes, that's one of a few things

[clark800]

Maybe a cleaner way of doing it:

module.exports = function (config) {

  function getX (param) {

  }

  function getY (param) {

  }

  return {getX, getY}
}

[alandotcom]

Blocked by interledger-deprecated/five-bells-notary#44

[clark800]

LGTM