Invoke PipedCompressionProgram directly rather than through subclassing

[rhpvorderman]

This is basically this Jack Diederich video in practice: https://www.youtube.com/watch?v=o9pEzgHorH0

Subclassing to derive settings is not the best way in this case.

By moving all the program specific arguments to a dedicated _ProgramSettings class, we can use a _ProgramSettings class for each different program. These settings classes are now in one central place in the code, rather than spread out over multiple class definitions.

• Less code
• Easier to see which programs are used
• Easier to add new programs

I also made _PipedCompressionProgram a private class.

EDIT: Changed to reflected changes after review.

[rhpvorderman]

@marcelm, since you like deleting lines here is another one. This is the last major refactor that I had in mind.

[marcelm]

I like the direction this is going. Subclassing to customize behavior could be considered an anti-pattern, from which I’ve tried to get away myself recently. I liked this talk by Brandon Rhodes. Relevant here would be the part about composition vs specialization.

However, I’m not a huge fan of using **kwargs. You have this comment:

Rather than using a dict, use a NamedTuple with _asdict to enforce presence
of certain members and type checking.

The namedtuple itself is typechecked, but that is mostly lost when the _program_settings() function is invoked, which has just Dict[str, Any] as return type.

Another solution would be to pass ProgramSettings directly to PipedCompressionProgram and then avoid calling _asdict, but assign the attributes individually instead. This adds a couple of lines, but makes it clearer what is going on.

One could argue that this increases coupling between ...Program and ...Settings, but the coupling is already there because the constructor to PipedCompressionProgram expects exactly those fields that ProgramSettings contains.

[rhpvorderman]

However, I’m not a huge fan of using **kwargs.

I found this made writing the tests less messy (I initially implemented it this way), but I agree with your arguement. So I have changed it back and used a dataclass instead.

EDIT: @marcelm, all changes are done and reviewable now.
EDIT2: I have seen the talk by Brandon Rhodes too. It is great. I went on to watch a few of his videos on design. It has greatly influenced me. I now also try to communicate with data rather than behavior as much as possible.

[rhpvorderman]

@marcelm, it is finished now. I saw a comment that you were waiting on this in the other PR. I have rewatched a bit more of Brandon Rhodes. What is done here is composing two classes together, which is totally legit, even if this creates some coupling.

[marcelm]

Nice, I like it! Thanks for addressing my comments.

[rhpvorderman]

Thanks for merging! According to github, we went from 1144 LOC on __init__.py on 1.9.0 to 650 LOC now on main. I am very happy with that. Coverage has also skyrocketed as a result.