Produces a string based on rules

Question

I was recently playing around with some code, and someone mentioned that my code uses too many classes. I didn't realize I've been doing ES6 incorrectly for so long.

I'm having a hard time finding a way to organize my code in a way that reads nicely. I say this coming from a C# background.

Thanks for your time, everyone.

class Producer {
    constructor(starterString) {
        this.rules = [];
        this.createRules();
        this.producedString = starterString;
    }
    createRules() {
        this.rules.add(new RuleAddE());
        this.rules.add(new RuleAddNumberOne());
    }
    produce() {
        for(var i = 0, len = this.rules.length; i < len; i++) {
            this.producedString = this.rules[i].run(this.producedString);
        }

        return this.producedString;
    }
}

class Rule {
    constructor() {
        this.applied = false;
    }
    run() {            
    }
}

class RuleAddE extends Rule {
    constructor() {
        super();
    }

    run(stringToChange) {
        this.applied = true;
        return stringToChange + "e";
    }
}

class RuleAddNumberOne extends Rule {
    constructor() {
        super();
    }

    run(stringToChange) {
        this.applied = true;
        return stringToChange + "1";
    }
}

var producer = new Producer("aa 1hdheb");

console.log(producer.produce());

How could I improve this code with more proper structure?

Answer 1

1. You have classes but you do not effectively need them because they do not keep any state (even the applied field is actually unused). In C# you do not have much options (unless they're local) but in JavaScript you can use functions anywhere:

const ruleAddE = (text) => text + "E";
const ruleAddNumberOne = (text) => text + "1";

And so on. Now let's go to the core of your algorithm: the Producer class. Do you need e class here? From the code you posted it seems you don't and again a function is enough. Let me assume that rules is accessed from outside the class then it makes sense to keep it in-place.

constructor. I'm not sure if you want to discard starterString, repeately calling produce() will always work on latest produced string. If this is the intended behavior then you can let it as-is.

createRules. If this is a private method I like to prefix it with _. It's not a rule, not everyone agrees with this and - unlike Python - it does not change method visibility/accessibility. I like it because it's self-documenting and I won't, by chance, use externally a private method (which is an implementation detail). You do not even really need this function because code is reduced to:

this.rules = [ ruleAddE, ruleAddNumberOne ];

Note that your original code is broken because there isn't a method Array.add() but Array.push().

produce. This may be simplified, the same way you'd do in C# using some LINQ:

produce() {
    this.producedString = this
        .rules.reduce((text, rule) => rule(text), this.producedString);

    return this.producedString;
}

---

2. In short, your code might be:

const ruleAddE = (text) => text + "E";
const ruleAddNumberOne = (text) => text + "1";

class Producer {
    constructor(starterString) {
        this.producedString = starterString;
        this.rules = [ ruleAddE, ruleAddNumberOne ];
    }

    produce() {
        this.producedString = this.rules
            .reduce((text, rule) => rule(text), this.producedString);

        return this.producedString;
    }
}

---

Note that for undefined and null inputs the result is pretty odd (undefined is converted to a literal string) while you may expect an empty string in this case. If that's your case then change declaration to:

this.producedString = starterString || "";

We're not using Rule class hierarchy but, in general, remember that in JavaScript there isn't a (easy) syntax to enforce an object with a specific base class then, unless, it adds something, you can simply drop it.

---

3. To answer the question about "do you need a class for Producer?": if you don't need to call produce() multiple times on the same object then you don't even need a class and everything may be reduced to:

const ruleAddE = (text) => text + "E";
const ruleAddNumberOne = (text) => text + "1";

function produce(starterString) {
    const rules = [ ruleAddE, ruleAddNumberOne ];
    return rules.reduce((text, rule) => rule(text), starterString);
}

In general in JavaScript you can use classes when you want to carry state and behavior on the same object but for everything else you may well use plain functions (and if you use modules you do not even need to use classes as namespaces).

4. If you're using a class only to give a chance to the caller to specify a different set of rules:

function produce(starterString, rules = [ ruleAddE, ruleAddNumberOne ]) {
    return rules.reduce((text, rule) => rule(text), starterString || "");
}