7

The question might sound a little weird, and I guess it is. I'm came up with the question while browsing through some design patterns. I came to the notorious state / strategy pattern and I came up with this code:

public class Account
{
    public double Balance { get; private set; }

    private IAccountState state;

    public Account()
    {
        state = new ActiveAccountState();
    }

    public void Deposit(double number)
    {
        state = state.Deposit(() => Balance += number);
    }

    public void Withdraw(double number)
    {
        state = state.Withdraw(() => Balance -= number);
    }

    public void Freeze()
    {
        state = state.Freeze();
    }

    public void Close()
    {
        state = state.Close();
    }
}

It is simplistic and not production ready of course, but I'm not trying for it to be that. It is a simple example. This code actually shows well the problem I'm having. Basically I have an IAccountState interface which represents the state / strategy. Now this could be considered a dependency and by allowing the Account class to KNOW how to manufacture the ActiveAccountState I enable the Account class to have more than 1 responsibilities. I could create an AccountStateFactory and make that a dependency on the account class. This would allow me to easily inject this newly created Factory dependency with the use of the Account class' constructor. But this way I, well, expose the implementation detail which is probably only interesting to the Account class and to none else. I guess that would make unit testing a tad bit easier.

My question would be how to know if something should be a dependency? Sometimes it is dead simple (ie. services, factories, providers etc.) but then there are times when I'm confused (like in this state / strategy example). Thanks in advance!

3
  • "the notorious state / strategy pattern" - Just curious - what's so notorious about the State & Strategy patterns? :) Especially Strategy - among the classical go4 patterns, it's one of the fairly straightforward ones. (I'll admit that State can be somewhat tricky in unexpected ways.) Jun 11, 2018 at 14:04
  • Read this book, thank me later: amazon.com/Dependency-Injection-NET-Mark-Seemann/dp/1935182501
    – Eternal21
    Jun 12, 2018 at 19:13
  • Every GoF pattern is notorious in my book x) Thanks for the book recommendation
    – masm64
    Jun 13, 2018 at 14:07

6 Answers 6

19

how to know if something should be a dependency?

Whenever you need something done but you don't want to do it here.

The big thing I'd like you to understand about reference passing (or Dependency Injection if you insist on being fancy) is that using a known good default is ok. You are not forbidden from using new. You are forbidden from using new without providing a way to override whatever that new makes you point at.

This is poorly understood in languages (like Java) that do not have named parameters. C# has them so you guys have no excuse. Josh Bloch gave Java users his builder pattern to fix this so if you know that, you also have no excuse.

Named parameters make it easy to have optional arguments. If you have that you can have default values easily. Your state should be an overridable default value. That lets you use DI without suffering with helpless classes that can't build themselves.

The philosophy behind this different approach to DI is called convention over configuration. Try it. I bet you'll find it makes these ideas much less theoretical and more useful.

There are other issues that you didn't ask about and since this isn't Code Review I won't get into them all, but I just can't ignore the double Balance one. Please tell me it doesn't represent dollars and cents. Double works well on discrete inches (base 2) and nondiscrete/continuous measurements (no base) but not on base 10 currencies. IEEE floating points and accountants rarely get along.

4
  • The balance part is there just for it to be there. This is just a simple example but thanks for pointing it out. For argument's sake what would you suggest though?
    – masm64
    Jun 10, 2018 at 9:48
  • 3
    @user3357969 an inter value of the given smallest unit of interest (usually cent, but sometimes smaller values). Jun 10, 2018 at 11:57
  • 1
    @user3357969 C# has a decimal type that is just crying to be used here. Jun 10, 2018 at 14:21
  • thanks! this code is not used in production and was just a stupid simple example hence the double, no need to overthink it :D but it is still good that you've mentioned it so that if someone comes in and sees the example this part has a suggestion, too
    – masm64
    May 28, 2020 at 8:51
6

You have not provided state/strategy separation or reduced the responsibilities of Account. To a caller of Account there is no separation between Account and ActiveAccountState. Any call to methods of Account will also call methods of ActiveAccountState. The expected result will thus depend on both the Account and ActiveAccountState class. Unit tests for Account will be sensitive to changes in ActiveAccountState.

If you really separate them by injecting an IAccountState instance into Account, then you can test Account using a mocked implementation of IAccountState. These tests will no longer be sensitive to changes to ActiveAccountState. This will complicate the creation of Account instances, and a factory class or a dependency injection container might be helpful.

How to know? You might try writing your code test-first. It's no fun writing complicated tests, so you will start optimizing your class design for ease of testing. Code that is easy to test is easy to maintain.

BTW, why is Balance a member of Account ? Surely Balance should be owned by the account state.

5
  • Interesting point. Why do you think that the balance should be a part of the AccountState instead of the Account? In my view it is a property of the Account and not the strategy.
    – masm64
    Jun 9, 2018 at 22:50
  • I strongly support the TDD argument! Jun 9, 2018 at 23:11
  • @user3357969 -- so you named the strategy "IAccountState" ? Jun 9, 2018 at 23:17
  • The IAccountState is basically the state / strategy pattern in action
    – masm64
    Jun 9, 2018 at 23:31
  • maybe it is just the example, but yeah Balance doesnt make any sense
    – Ewan
    Jun 10, 2018 at 10:09
2

ActiveAccountState is clearly a dependency, but the question is whether you should inject this dependency or create it inside Account. I don't see any benefit to injection in this case.

Account creates an ActiveAccountState instance, but that does not mean Account now "knows how to manufacture the ActiveAccountState". This knowlede is presumably encapsulated inside the constructor of ActiveAccountState. Account is just using the public interface, just as it is using the public interface to perform the state transitions.

In general you want to expose as few internals as possible. It seems to me ActiveAccountState is purly an internal mechanism inside Account, and should not be exposed to clients of Account. MaRequiring a dependency to be injected means you expose this dependency to the client.

1

I for myself apply the Single responsibility pattern to this problem too:

A class providing business behavior should not be responsible for instantiating its dependencies.

So when in doubt favor DI.

1

My question would be how to know if something should be a dependency?

Starting from a solid OO class design, dependencies tend to become self evident.

IAccountState is inventing a dependency by inappropriately decoupling behavior and state of an otherwise coherent class. Yet you virtually hit the nail on the head: "I, well, expose the implementation detail which is probably only interesting to the Account class and to none else"

An abstract class in this case can solve the class fracturing while insisting on a strategy pattern and so eliminate the artificial dependency problem. And wherever there is a constructor or method - concrete, abstract or otherwise - there is a seam for dependency injection.

So I enthusiastically disagree with shying away from inheritance because "you have limited opportunity for DI ... as you bake the logic ... through its inheritance chain". You should have designed opportunities for DI, not a free-for-all violating the tenants of object orientation.


A good class exposes functionality and hides state

The key is identifying that single responsibility for each class. Think functionality first, then the state necessary to support it. This is a more reliable path to good SRP application.


Diligent design embiggens dependencies

Don't allow exuberance for interfaces hobble SRP application and rip apart state and/or functionality prematurely.

Inheritance and composition are complementary. Let this evolve during analysis and design. In fact I expect to see this just as any machine is made of distinct parts.

Don't approach this from a solution looking for a problem stance.

0

Always use DI when you have separated your concerns using composition.

The very fact that you are using composition implies that you will want to swap out the dependency, if only with a mock for testing.

If you use inheritance instead then you have limited opportunity for DI as you bake the logic into the concrete class through its inheritance chain.

So in your example you might have say Business and Current accounts with different withdrawal logic managed by different injected AccountState objects.

If you were using inheritance you would create separate BusinessAccount and CurrentAccount classes which overrode the withdrawal method

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

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