2
\$\begingroup\$

I have a question about dependency injection and best practices when using simple classes which are meant to be created often, which have dependencies on external services. Here's a (vastly) simplified version of what I'm working on:

app.js

import {Factory} from './factory.js';
import {Convert} from './convert.js';

let convert = new Convert();
let factory = new Factory({ convert });

// assume this is being imported and executed somewhere
export function run() {
    let avengers = [
          factory.person({ first: 'Iron', last: 'Man' })
        , factory.person({ first: 'Captain', last: 'America' })
        , factory.person({ first: 'Scarlet', last: 'Witch' })
        , factory.person({ first: 'Black', last: 'Panther' })
    ];
    console.log(avengers);
}

convert.js

export class Convert {
    toString(val) {
        if (!val)
            return '';
        return val.toString();
    }
}

person.js

export class Person {
    constructor({
        convert,
        first,
        last
    }) {
        this.convert = convert;
        this.first   = this.convert.toString(first);
        this.last    = this.convert.toString(last);
    }
}

factory.js

import {Person} from './person.js';

export class Factory {

    constructor({ convert }) {
        this.convert = convert;
    }

    person(obj) {
        obj.convert = this.convert;
        return new Person(obj);
    }
}

My main interest is that class Person has a dependency on a function located on an instance of class Convert. My question is this -- is this taking dependency injection too far? Am I better off just importing an instantiated version of Convert into Person.js and referencing it directly? I know that will work, but I was trying to avoid coupling the two classes without a nice way to override. I definitely don't like what I have written here, since Convert will be defined on each Person, which is worse than coupled classes in my opinion.

I had started working on modularizing one of my libraries to decouple classes and use dependency injection, but my pattern broke down when I ran across an instance similar to this. Any suggestions are definitely welcome.

Side note, I know in this example I could just use the Convert methods inside the Factory instead of inside Person -- that doesn't work for my actual project, unfortunately.

\$\endgroup\$
1
\$\begingroup\$

Am I better off just importing an instantiated version of Convert into Person.js and referencing it directly? I know that will work, but I was trying to avoid coupling the two classes without a nice way to override. I definitely don't like what I have written here, since Convert will be defined on each Person, which is worse than coupled classes in my opinion.

There is nothing wrong with requiring a data conversion service. The problem is coupling the conversion service with the Person class. This coupling should occur in the Factory class instead.

Furthermore, you haven't coupled the classes together. ECMA Script is a dynamically typed language. At worst you've coupled the Person class to an object supporting the same public interface as the Convert class.

Solving this dilemma is pretty simple. The data conversion should happen in the factory:

factory.js

import {Person} from './person.js';

export class Factory {

    constructor({ convert }) {
        this.convert = convert;
    }

    person(obj) {
        obj.first = this.convert.toString(obj.first);
        obj.last = this.convert.toString(obj.last);

        return new Person(obj);
    }
}

person.js

export class Person {
    constructor({ first, last }) {
        this.first = first;
        this.last = last;
    }
}

This keeps your Person constructor clean and straight forward. The messiness of converting data types is encapsulated in the Factory class where it belongs, and the Person class no longer has a dependency on the Convert class.

\$\endgroup\$
  • \$\begingroup\$ I agree, but unfortunately I can't move all of my logic to the factory in my actual code, as the class I'm actually creating needs to make use of the dependency (Convert in this case) inside a function which lives on the dependent class (Person) -- sorry, I probably over-simplified the question a bit by moving the use of the dependency to the constructor, but in reality it's in quite a few other places. That being said, I will most likely accept this as the answer, as I think it's as close as I can get. \$\endgroup\$ – dvlsg May 27 '15 at 23:34

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.