Make using of ImTools concurrently with DryIoc about imtools HOT 13 CLOSED

DryIoc.Internal makes all public types internal including the types from FEC and ImTools.

Thinking about this Today and I see two ideal choices:

1. Make FEC and ImTools sources in DryIoc into the implementation detail - internalize the public types, optionally strip the unused code.
   Pros:

• No confusion with standalone FEC and IT
• Still no external deps to manage
• Easy to tune for DryIoc needs
• Less code and dll size if the stripping is in place

Cons:

• Breaking DryIoc major version
• More management from my side - harder to incorp the fixes (though it maybe less fixes because of stripped code)

2. Make FEC and IT the explicit dll deps.
   Pros:

• Familiar dll story
• Easy to incorp new fixes
• Non-breaking?

Cons:

• Harder to tune for DryIoc
• Dll deps hell
• Not sure how to proceed with the source packages - breakes here?

@detoxhby What are your thoughts?

from imtools.

Thanks for the ideas, I will think about the options how to solves this in a balanced manner!

In the meantime, what about my original idea?

In the external libs (FEC, IT), introducing a preprocessor #if directive based macro that would include .Embedded after every single namespace. Could be able to manage the refactor with regex-based replace.

For example:

namespace ImTools
#if IMTOOLS_EMBEDDED
    .Embedded
#endif
{
   ...

Pros:

• no maintenance costs: would be able to include the unmodified source into DryIoc
• insignificant refactor needs:
  • relatively easy find in path + replace using ImTools by using ImTools.Embedded
  • set IMTOOLS_EMBEDDED symbol for compilation
• no change in distribution options: people still be able to include the sources directly into their code without any change or the need to set a precompile symbol (becase the default behavior would produce the corrrect namespace)
• ⚠️ no external changes: DryIoc users would've never used the extra libs directly from DryIoc, so would not break legal code

Cons:

• ❗️ external changes: would unintentionally break working code just by the fact the mistaken references are would no longer be available

No easy way to deal with the con other than jumping at least a minor version.
I know the semver would make it necessary to jump a major, but I don't think it is a direct API change, it is just an incorporating dependency change which is outside the libraries' functionality.
Another option is to introduce a new NuGet package (like DryIoc.Embed) which would behave this way without making the useful types internal.

from imtools.

I have some serious deadlines currently for the next few weeks

Maybe I have time to look next week.
I am thinking about minor versions for FEC and IT, and probably major for DryIoc. I'd like to have an oportunity and mark some things Obsolete in DI. Given that prev release of DI 4.1 was stretching a line of minor version - now is time for fair and square major change.

from imtools.

✔️ Alright, this is not a rush, take the time necessary for a correct major version change!

Feel free to contact me if you need more information or a second-eye with ideas or realization details.

from imtools.

implemented in DryIoc v5

from imtools.

Thanks for the open issue.
It may be a real PITA actually.

What is your setup?

• DryIoc source or dll package?
• ImTools source or dll package?
• Are they referenced from the different package
• Did you know about DryIoc.Internal package?

from imtools.

DryIoc source or dll package?
ImTools source or dll package?

DLL for both packages.

Are they referenced from the different package

This is the problem: cannot reference it as the compiler gets confused as both assemblies contains the same namespace.

Did you know about DryIoc.Internal package?

I did not, but checking out the nuspec file it will still include all the external lib sources and would cause the conflict.
Please correct me if I am mistaken.

To be complete, the same happens with FastExpressionCompiler as well. In that case, we had to change the namespace locally to be able to use it separately from DryIoc. Would be the solution for ImTools as well, but I don't wanna go this way because source inclusion...

• adds unnecessary extra time for compilation, and
• confuses code quality tools (like SonarQube) that will throw numerous warnings, and would impair the code duplication, reliability and other indexes, thus altering the real state of the project's source code ( this is what happened with FEC, not good )

from imtools.

You mean IMTOOLS_EMBEDDED will be set in DryIoc packages, right?

Then I think it is a good thing to go when thinking in the smaller steps. Thanks for the idea.
You can submit a PR if you want.
I'll probably will risk to go with the v4.2 - I've planned a more cleanup for the v5... lemmethink

from imtools.

You mean IMTOOLS_EMBEDDED will be set in DryIoc packages, right?

Yes!

You can submit a PR if you want.

I have some serious deadlines currently for the next few weeks, but after that, I would gladly help with this work throughout all the projects (FEC, IT, DryIoc).

How do you want to handle cross-project refences to coordinate this change? (FEC+IT first with minor version change and release THEN DryIoc with these new versions?)

from imtools.

Maybe a crazy thought, but why not make ImTools independent, and simply have Dryloc reference ImTools? That way, if projects reference both, there won't be any problem anymore, as there'll be only one shared library for the ImTools classes.

from imtools.

@abelbraaksma

Maybe a crazy thought, but why not make ImTools independent, and simply have Dryloc reference ImTools?

I've answered to this with the second choice in the discussion above.
Mainly I don't wont to get the same thing as with Json.NET dep.
Plus I still want a freedom in DryIoc to use a specially tuned versions of deps. It is just an impl detail - let's keep it that way.

@detoxhby
I'm thinking about simplier variant to your idea. Just change the namespaces in DryIoc to DryIoc.ImTools and DryIoc.FastExpressionCompiler respectevely.

from imtools.

I've answered to this with the second choice in the discussion above.

Apologies, I thought I read the whole discussion, must've missed it ;)

It is just an impl detail - let's keep it that way.

Fair enough 👍

from imtools.

I'm thinking about simplier variant to your idea. Just change the namespaces in DryIoc to DryIoc.ImTools and DryIoc.FastExpressionCompiler respectevely.

That's fine too, but a bit error more prone. If you copy over new code and forget about the namespace, you could easily make a VS quick action include mistake somewhere in DryIoc's source code unintentionally (there is a chance; but "fixing" the sources would discard this case entirely).

ℹ️ to reason about current single-file situation: I think it's fine to go with the easy way and just rewrite the namespace in the target project
(would not recommend though if IM or FEC would contains more than a few namespaces and/or files)

from imtools.