0

According to the answer in this post Should my classes have separate constructors just for unit testing?

ad hoc constructor is not good and should be avoided.

Unfortunately, I have the same problem in my code.

        readonly AppletFirmware _appletFirmware;
        readonly byte[] _motorControllerFirmware;
        readonly ISAMBATransport _transport;
        ISAMBADevice _sambaDevice;
        IApplet _sambaApplet;
        IApplicationProperties _appProps;

        public FBSambaBootloader(byte[] appletBytes, byte[] motorControllerBytes, ISAMBATransport transport)
        {
            _appletFirmware = new AppletFirmware()
            {
                FileSource = new MemoryStream(appletBytes),
                AppletAddress = APPLET_ADDRESS,
                AppletMailboxAddress = APPLET_MAILBOX_ADDRESS,
            };
            _motorControllerFirmware = motorControllerBytes;
            _transport = transport;

            _appProps = new ApplicationProperties();
            _appProps.Init(SAMBA_PROGRESS_LOG_KEY);

            _sambaDevice = new SAMBADevice(_transport);
            _sambaApplet = new Applet(_sambaDevice, _transport, _appletFirmware);
        }

        #if DEBUG
        public FBSambaBootloader(byte[] appletBytes, byte[] motorControllerBytes, 
                ISAMBATransport transport,
                ISAMBADevice sambaDevice,
                IApplet sambaApplet,
                IApplicationProperties appProps)
        {
            _appletFirmware = new AppletFirmware()
            {
                FileSource = new MemoryStream(appletBytes),
                AppletAddress = APPLET_ADDRESS,
                AppletMailboxAddress = APPLET_MAILBOX_ADDRESS,
            };
            _motorControllerFirmware = motorControllerBytes;
            _transport = transport;

            _sambaDevice = sambaDevice;
            _sambaApplet = sambaApplet;
            _appProps = appProps;
            _appProps.Init(SAMBA_PROGRESS_LOG_KEY);
        }
        #endif

        public async Task<bool> Start(Action<int, int> onProgress, uint offset = 0)
        {
            await _sambaDevice.SetNonInteractiveMode();
            await _sambaApplet.InitAndConfigureApplet();

            _sambaApplet.OnFirmwareUpdateProgressChanged += (senader, args) =>
            {
                onProgress?.Invoke(args.BytesProgrammed, args.TotalBytesToProgram);
            };
            _sambaApplet.OnFirmwareUpdateProgressChanged += (senader, args) => LogProgress(args);

            var address = START_ADDRESS + offset;
            var bytes = _motorControllerFirmware.Skip((int)offset).ToArray();
            var bytesProgrammed = await _sambaApplet.AppletProgramFlash(address, bytes);
            if (bytesProgrammed < _motorControllerFirmware.Length)
            {
                Debug.WriteLine("\nProgramming failed");
                return false;
            }
            else
            {
                Debug.WriteLine("\nProgramming Successful, please reset the device.");
                await _sambaDevice.RunFromAddress(START_ADDRESS);

                return true;
            }
        }

And in this way, it seems my class has many dependencies, but in fact, most dependencies are created internally by the class itself, it has all the information these dependencies need, and that is why I think it is a good place to take the responsibility of creating these dependencies. So I would like to know how to make this class testable while I can keep it from using ad hoc constructor.

Thanks.

  • Some dependencies are bound to this class and will never change to use different implementation, such as AppletFirmware _appletFirmware;, so I think it is not necessary to inject an AppleFirmware. And having many dependencies in the constructor looks bad. – Haoyuan Tang Mar 28 '19 at 1:54
  • 1
    You can tell at a glance that the class will be hard to test. 'Newing' up objects inside constructor is a code smell. New them up in your app root, and inject instead. – Eternal21 Mar 28 '19 at 11:43
  • What do you want to test exactly? – Stop harming Monica Mar 28 '19 at 14:09
  • @Goyo The Start() method – Haoyuan Tang Mar 28 '19 at 22:34
  • @Eternal21This class has some information I need to new Applet, like APPLET_ADDRESS and APPLET_MAILBOX_ADDRESS, they are bound to this class and will never be changed or used by other classes. If I move my dependencies out to the app root, how should I instantiate them with the information this class has? And will it end up having more duplicate code since every time I want to create an instance of FBSambaBootloader, I need to instantiate all its dependencies again, even though the code of creating these dependencies will always be the same? – Haoyuan Tang Mar 28 '19 at 22:41
5

I have the same problem in my code.

A key idea in test-driven-development; if the tests are hard to write, they are telling you that your design needs to be improved.

most dependencies are created internally by the class itself, it has all the information these dependencies need, and that is why I think it is a good place to take the responsibility of creating these dependencies.

That's not quite the right heuristic. The important concern is that you have dependencies that need to be changed when you switch from one environment to another (in this case, from prod to dev). So those dependencies, one way or another, need to be configurable.

There are a number of different ways that can be done, but the most common is dependency injection, which is just a fancy way of saying "pass the dependencies as arguments".

Miško Hevery has written a lot about the testability problems of constructors, and how to avoid them. Mark Seemann has also written good material on how object graphs are composed.

Another approach that makes sense in some circumstances is to first create the object in its inert, testable form, and then in the production code path replace the inert dependencies with the real ones that you need.

|improve this answer|||||
  • More specifically, "pass the dependencies as constructor arguments." – Robert Harvey Mar 28 '19 at 4:27
  • @RobertHarvey I disagree. Trying to artificially constrain DI to only apply to constructor arguments achieves nothing useful. – David Arno Mar 28 '19 at 5:52
0

After spending a couple of days, I have found a solution in Mark Seemann's book https://www.manning.com/books/dependency-injection-in-dot-net?. The most important and hardest thing to use dependency injection is to let go the control of dependencies, that means dependencies should be injected(in most cases, constructor injection is the best way to do this), instead of letting the class itself to specify which concrete class it wants to use.

The main questions of using Di are where we should create our dependencies, and how we know what dependencies are needed, these problems can be solved by using Composition Root, which is a concept related to DI. Let the App entry point be the only place that takes the responsibility of instantiating dependencies, and composing the object graph with the help of DI containers.

I was struggling with having too many constructor parameters when I first used DI to solve the question I asked. Since almost every dependency is injected. This can also be fixed by combining some relevant dependencies into a coarse-grained dependency, and inject the coarse-grained dependency.

Though I am still a new learner of DI, I can solve every problem I have encountered so far in my project by using DI (mostly constructor injection, sometimes property if the dependency is optional or created at runtime) and Abstract Factory (to solve cyclic reference and create dependencies require runtime parameters).

@VoiceOfUnreason 's answer helps me a lot, and also thanks to everyone who answered my question, it is inspiring.

|improve this answer|||||

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.