1

I'm working on a classes that are designed to read a defined file format; said format is comprised of 2 separate files (FileA and FileB). For ease of use, the format is referred to as the FileA format, but a valid "file" must have a FileB. The files share the same path prefix, but alter in suffix; therefore my current classes appears as:

class FileReaderA:
    def __init__(self, path: pathlib.Path):
            self.a = path.with_suffix('.fa')
            self.b = FileReaderB(path)
    # methods related to operating on FileA types

class FileReaderB:
    def __init__(self, path: pathlib.Path):
            self.b = path.with_suffix('.fa')
    # methods related to operating on FileB types 

The classes are separated since FileReaderB does not depend on FileReaderA and can be used independently to read FileB types, however a valid FileA requires its associated FileB. Therefore, composition made sense, at first. Now I'm wondering if DI is the more appropriate solution, but it requires that FileReaderB be explicitly instantiated and passed to the constructor of FileReaderA, which is less than ideal form a user standpoint with this API:

>>> frb = FileReaderB(path)
>>> fra = FileReaderA(path, frb)

Thus I am forced to abstract it by using a factory:

def reader(path: pathlib.Path):
    return FileReaderA(path, FileReaderB(path))


>>> fra = reader(path)

This seems verbose and I feel as though I'm introducing more code and layers of abstraction to achieve the same result, all around the guise for better unit testing of decoupled classes.

What is wrong with just using Composition over enforcing DI?

  • Are you certain that FileReaderA depends upon FileReaderB? On the surface it sounds like you have two separate file formats. To accomplish some goal, an operation may need both FileReaderA and FileReaderB at the same time, but is this a responsibility of FileReaderA, or can it be pulled out into some other class? – Matthew Sep 9 at 14:19
  • @Matthew They are indeed 2 separate file formats, however, to convey information correctly, Record 1 of FileReaderA needs the information of Record 1 of FileReaderB. Independently their data is valid, but means nothing when disassociated. – pasta_sauce Sep 9 at 14:20
  • It may make your system easier to write if you focus on providing separate abstractions for reading A files and reading B files. Then you can write some logic that deals with them simultaneously. At which point, if you move the calculation into a new class, it may no longer care that they're files at all. The sole purpose is to break the problem down into smaller, testable chunks, this may also help with your dependency problem. – Matthew Sep 9 at 14:23
  • DI gives you flexibility to reuse the same component under different circumstances, but to me, it doesn't look like that's needed here. Making FileReaderB a constructor parameter of FileReaderA sort of implies that the B part could vary (in implementation, format), which is probably not the case. As for testing, DI lets you pass in a test double, e.g. to avoid accessing the file system in a test, but you don't get that here. You can either test the A and B readers separately (as they are), or extract the partial A-reading logic into its own class, and have FileReaderA be composed of those two. – Filip Milovanović Sep 9 at 14:25
  • @Matthew Possibly, but then I feel as though we've gone full circle; I pass a file path to Processer and it must instantiate each of the reader abstractions in its constructor; thus we've arrived at composition. – pasta_sauce Sep 9 at 14:26
2

The questions I would ask here are:

  • can one sensibly unit test a FileReaderA with an injected MockFileReaderB and achieve independence of I/O operations that way (to make such a test simpler and faster)?

  • are FileReaderA and FileReaderB both so complex that making a real unit test for FileReaderA, isolated from FileReaderB, may be beneficial? So when the test fails, you know for sure FileReaderA is broken, not FileReaderB?

I guess the answer to the first question is "no", since a FileReaderA will probably require I/O operations anyway. The second question is something only you can answer.

| improve this answer | |
  • Could you expand on why I/O operations would make testing problematic? – pasta_sauce Sep 9 at 14:19
  • If you wanted to write unit tests, dealing with in-memory streams may be much simpler than dealing with discs. – Matthew Sep 9 at 14:20
  • @pasta_sauce: unit tests with I/O are not necessarily problematic, but with I/O, tests are often faster and simpler. The question is surely, how fast you will need them to be. Have a look at Martin Fowler's site and his remarks about the speed of unit tests. – Doc Brown Sep 9 at 14:43
  • @DocBrown To the second question, it is possible to write a test that if failed, it would indicated that FileReaderA is broken and not FileReaderB. Given that answer, how does that aid in determining if composition would be best over DI? Additionally, in your comment did you mean "...but without* I/O..."? I would think that tests would be slower when it comes to I/O. – pasta_sauce Sep 9 at 14:51
  • @pasta_sauce: yes, I mant "without I/O". And as I said, you have to answer the second question to yourself, not to me. If you think DI is not necessary for making your unit tests for FileReaderA reliable, go ahead, trust your instinct. – Doc Brown Sep 9 at 14:59
0

The problem with:

self.b = FileReaderB(path)

is that you can never do:

class FileReaderB2 : FileReaderB
{
    //some overridden methods
}

and then use it in your FileReaderA

Which, some would say, is the whole point of classes.

| improve this answer | |
  • And I see that here, DI is useful. However, since FileReaderB is parsing a know binary format, I cannot foresee a need for a class that extends it and overrides methods. Others have suggested a 3rd class (lets call it Reader) that is composed of the two individual format readers; this is the way I've gone so far. What would make this method less viable? – pasta_sauce Sep 10 at 10:42
  • the obvs example is in unit tests – Ewan Sep 10 at 10:53
  • the second obvs example is if the format of the file is changed – Ewan Sep 12 at 15:22
  • Thanks for the inputs and considerations! I think after all this, composition is still fine. – pasta_sauce Sep 14 at 15:06

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.