Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Case 1] Solving packaging problem by inverting dependencies #4

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

vzurauskas
Copy link

This seems to be the simplest solution. I changed no code, only structure and naming. If the default implementation of the algorithm, DefaultPvModuleLayouts, depends on lower level details in a subpackage, then they are not really lower level details. In fact, DefaultPvModuleLayouts is supposed to be in a lower level, because it depends on everything and nothing depends on it. Therefore:

  • Pull layout specification implementations up into pvmodulelayouts package.
  • Push DefaultPvModuleLayouts into a subpackage. I propose to call it defaultlayouts for the default algorithm. Presumably you will need alternative algorithms, they can be put into other subpackages, and they can depend on the same layout specifications in the parent package.

If this is a library, then this is enough, and you can have even stricter rules than the three proposed. Instead of "packages should never depend on sub-packages", you can introduce "packages can only depend on superpackages (no sibling dependencies)". If it's a runnable app, however, then you need a place which ties the whole app together, there is no way around that. I usually introduce an app package at the root of the project, representing the application fully assembled from all disparate parts from all packages, but since your package structure already has app, I introduced appconfig, similar as in your solution PR.

@grimsa
Copy link
Owner

grimsa commented Jul 18, 2022

If the default implementation of the algorithm, DefaultPvModuleLayouts, depends on lower level details in a subpackage, then they are not really lower level details. In fact, DefaultPvModuleLayouts is supposed to be in a lower level, because it depends on everything and nothing depends on it.

While such technique could be useful in some case (probably if domain concepts were of somewhat equal weight, or if some domain experts saw it that way), I don't think this would be a good idea in general.

I think my example (case 1) can be generalized into something like this:

  • An interface for the whole algorithm (let's call it Algorithm)
  • Implementation of said interface which implements the whole thing, except for one point of variability (AlgorithmImpl)
  • An interface that models the variability of the implementation in a single step of the larger algorithm (AlgorithmStep)
  • Multiple implementations for the variations of that single step (AlgorithmStepImpl1 and AlgorithmStepImpl2)

In such case this:

- com.company
    - Algorithm interface
    - algorithm package
        - AlgorithmImpl class
        - AlgorithmStep interface
        - algorithmstep package
            - AlgorithmStepImpl1
            - AlgorithmStepImpl2

would seem preferable to what I think you proposed:

- com.company
    - Algorithm interface
    - algorithm package
        - AlgorithmStep interface
        - AlgorithmStepImpl1
        - AlgorithmStepImpl2
        - algorithmimpl package
            - AlgorithmImpl class

The reason is that reading top-down should reveal progressively more details, and if you are interested in high-level algorithm implementation you'd have to go to com.company.algorithm package in the first example vs. com.company.algorithm.algorithmimpl package in the alternative (i.e. go past the AlgorithmStep concept and its implementations as they are not relevant when seeking higher-level algorithm implementation details).

@grimsa grimsa changed the title Solving packaging problem by inverting dependencies [Case 1] Solving packaging problem by inverting dependencies Jul 23, 2022
@vzurauskas
Copy link
Author

vzurauskas commented Jul 24, 2022

I'm not convinced yet. I think the best approach depends on specific situation. Do several algorithms use the same step, or is a step specific to an algorithm? In former case I'd say the step is more abstract because algorithms depend on it and it doesn't depend on algorithms; in latter case the algorithm is more abstract because the steps are only its parts.

Although now that I think more about it, I think I see your point. If client code cares only about algorithms and not individual steps, then the steps can be package private and then they are just an implementation detail, even if multiple algorithms use the same step.

On the other hand, if step is conceptually more abstract than algorithm, then perhaps it's wrong for client code not to care about steps. Should it use steps to compose algorithms? Then the package structure would be entirely different.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants