Environment variables in the constructor are a code smell
TLDR: don’t use env vars in class constructors.
Seeing environment variables in a class constructor is a code smell and an anti-pattern.
For example in TypeScript for Node.js, you might see something like this:
Anti-pattern:
class Foobar {
private niceInjectableDependency: IDependency;
private importantConfig: string;
constructor(niceInjectableDependency: IDependency) {
this.niceInjectableDependency = niceInjectableDependency;
this.importantConfig = process.env.IMPORTANT_CONFIG; // code smell
}
}
This might seem reasonable, as we’re still kind of able to inject the
importantConfig
into the class, just not via the constructor. Unfortunately
this injectability is nowhere near as good as the usual kind with function call
arguments.
The reason using env vars in the constructor is an anti-pattern is that we’ve now created a dependency on global state. Env vars are global state (as is the current date and time, but that’s another discussion).
Because env vars are global state, having functions reach out and grab env vars behind the scenes will cause the same problems that any unmanaged global state will cause.
We can’t see that this class has a hidden dependency on global state when we see
new Foobar()
in the code. We have to know about this and make allowances to
work around it if necessary.
This class is now harder to test. You can set values on process.env
in
Node.js, but again it’s global, and it will make your tests difficult to
parallelise.
We also can’t choose to have this class behave differently at runtime, which is
what config is for, after all. Maybe we want to be able to configure the
importantConfig
to be something different, but we can only do it globally
now. Do we have to run separate processes for each different configuration we
want? That would be difficult to manage and tricky to debug, and could explode
into a large number of processes if we have several of these global env var
dependencies across different classes.
We might want to run several instances of this class with different configurations. Perhaps this class has a nice implementation of publishing a message to a queue, but when we have more queues to deal with later, we find we can’t just instantiate more instances of this class with each configured to publish to a different queue.
A factory pattern would be nice for this class, but again we’re not able to use the factory pattern here because of the uncontrollable global state at run time.
In short, don’t use env vars in class constructors.
Config is a dependency like any other, and should be injected into the class constructor. A factory pattern can handle this in a clear and maintainable way. Other set up code that is responsible for configuration might have the option of taking it from env vars, but it shouldn’t be hard-coded into classes whose responsibility is not about env vars.
This is related to the more general “Constructor does Real Work” flaw, which is worth reading about.