Future of Castle DictionaryAdapter

[jonorossi]

We would like to hear from anyone using the DictionaryAdapter component that is part of the Castle.Core package (was merged into Castle.Core.dll many years ago).

This component has seen close to no activity over recent years and adds a lot of baggage to Castle Core. I want to spin it off out of Castle Core, whether that is back into another project or deprecating it depends on the community.

If you've used it in the past and are not longer it would be great to hear from you too, I'm CCing those that have contributed to its source code over the last 6 years.

/cc @Lakritzator @justin-edwards @sharpjs @cneuwirt

[TimLovellSmith]

Looks pretty non-useful to me given modern C# language feature 'dynamic' and ExpandoObject. How about either deprecating it or pulling it out into a separate library, Castle.DictionaryAdapter for next version?

[yallie]

Looks pretty non-useful to me given modern C# language feature 'dynamic' and ExpandoObject.

ExpandoObject and dynamic variables lack compile-time checks and intellisense.
I think moving DictionaryAdapter out to a separate project and nuget package is a good idea.

[Lakritzator]

I think you should move it do a different project, so people can still use it if needed. It doesn't cost to just have a repository lying around... If it's baggage for Castle, make sure you can concentrate on the main functionality. Just don't get rid of it!

I can imagine that the DictionaryAdapter is not well known, in other words it lacks popularity, I was search for something with similar functionality and only found it after a while. Maybe if people know more about it, they start using it.

Personally I rather have a solution that creates code at compile time as generating code at runtime still adds another layer of complexity. I noticed the performance impact too! So I was actually thinking of using Fody instead, for my current solution.

[jonorossi]

I think you should move it do a different project, so people can still use it if needed. It doesn't cost to just have a repository lying around

@Lakritzator sorry I wasn't clearer for those not following along so closely, but that is exactly what I want to do, the only difference is if someone "maintains" it and it stays in the castleproject GitHub organisation (with CI builds, releases, etc), or just gets dumped in our castleproject-deprecated organisation where we are housing old stuff people are free to clone to their hearts content but is clearer there isn't even an expectation they'll build.

I want to spin it off out of Castle Core, whether that is back into another project or deprecating it depends on the community.

This issue is my polite way of informing users that we have no maintainers for DA (and little interest), and that those users are going to need to get involved if they want it to stick around.

[TimLovellSmith]

BTW was just exploring dependencies today and noticed DictionaryAdapter seems to be the only reason that Castle.Core depends on System.ComponentModel. So once this is pulled out there'll be one less dependency. 👍

[sharpjs]

Sorry for the delay. This is the bad side of ignoring GitHub notifications.

My contributions to DictionaryAdapter and the related XML stuff came at a time when @cneuwirt and I were using it on a project that ultimately was cancelled. At the time, I intended to spin off a generalization of the ideas into a separate project, but I lost interest when the complexity became too much for the potential benefit.

I am not aware of any users of DictionaryAdapter, nor would I recommend that anyone use it today. Unless something core has changed, it is both overcomplicated and grossly underperformant for what it does, as measured by a profiler. The more obvious approach — convert X to objects, work with objects, convert objects to X — is simpler and more performant in almost all cases. My opinion, of course, and @cneuwirt might disagree.

It is surprising that DictionaryAdapter has lurked here for so long without removal. I have no objection to removing it.

[ghost]

@sharpjs Personally I would like to thank you for your honest and kind input into things like this. Let this be an example to anyone “who can say” but is not. Steer like this is invaluable. It helps the next gen castle contribs make informed decisions about deprecating cruft. Thank you you. Very refreshing 👍

[ghost]

I vote for destroying this code with extreme and unjust prejudice.

[jonorossi]

It is surprising that DictionaryAdapter has lurked here for so long without removal. I have no objection to removing it.

Thanks for the reply @sharpjs. The reason basically boils down to Castle being in caretaker mode for the last few years because I was the last remaining active committer, until the last 6-12 months where we've been making some good progress cleaning things up.

[stakx]

@jonorossi, I created a clone of Castle.Core today where I removed everything except DictionaryAdapter, then renamed the project to Castle.DictionaryAdapter (as well as changed some of the documentation). I could set up a new DictionaryAdapter repository and upload what I've currently got; then we could remove DictionaryAdapter from Core. Seeing that you've previously self-assigned this issue, do you have any objections (or suggestions) to that?

[jonorossi]

I created a clone of Castle.Core today where I removed everything except DictionaryAdapter, then renamed the project to Castle.DictionaryAdapter (as well as changed some of the documentation). I could set up a new DictionaryAdapter repository and upload what I've currently got; then we could remove DictionaryAdapter from Core. Seeing that you've previously self-assigned this issue, do you have any objections (or suggestions) to that?

I was planning to do the same so don't mind who does it, however the reason I delayed doing it is to wait until the next major version because I didn't want the work to go stale. I think to discuss doing this we need to know when we want to do the next major, and at the moment we are working on non-breaking features.

[stakx]

@jonorossi, agreed, I think we should get the current non-breaking work released first. Hopefully that won't take much longer, anyway.

[stakx]

A small update. I previously said that...

[...] I could set up a new DictionaryAdapter repository and upload what I've currently got; then we could remove DictionaryAdapter from Core. [...]

I haven't forgotten this, but thought it would be sensible to do #441 first—that way, the new repo (which is a derivative of this one) could start with a clean CI setup right from day one, meaning we wouldn't have to do the same CI setup work twice.

[jonorossi]

Sounds like a good idea to get our CI cleaned up before the split.

I too haven't forgotten about this issue, we've got heaps of breaking changes to make 😄.

[jonorossi]

I've put this one back on my list to do very soon, been very slack getting this done.

[stakx]

@jonorossi, if splitting up the repository turns out to be too much work, here's an alternative suggestion (which I've already tried locally):

Instead of extracting DictionaryAdapter and putting it in a different repository, we could turn Core into a monorepo, and turn the Castle.Core NuGet package into a (mostly-) meta package. It's probably easiest to explain by showing the final dependency graph:

                     +---- Castle.DictionaryAdapter <---+
                    /                                    \
Castle.Logging <---+                                      +--- Castle.Core
                    \                                    /
                     +------ Castle.DynamicProxy <------+
                      \
                       \
                        +--- Castle.Core-log4net
                        |
                        |
                        +---- Castle.Core-NLog
                        |
                        |
                        +--- Castle.Core-Serilog

That is, we would:

1. Extract the logging-related stuff into a separate project (but keep it in the same solution).
2. Extract DictionaryAdapter and DynamicProxy into separate projects and let them reference the logging project.
3. Keep everything else (like the configuration and SMTP stuff) in Castle.Core, and adding references to all the other projects.

Users would then be able to pick just what they need (e.g. the new Castle.DynamicProxy package if they don't need the other stuff), or continue to reference Castle.Core to get the same functionality they'd get today (even though it will come from different assemblies).

We can only pull this off without a major version increase in Castle.Core' by adding a ton of type-forwarder attributes for everything that moves to a different package. Perhaps not worth it and better to do a major version increase.

We would also have to increase the major version number of the logging integration packages since they'd no longer reference Castle.Core and thus implicitly bring in everything else like they do today.

(Other than that, I haven't thought too deeply about versioning TBH, but I suspect we might have to be careful about propagating all version number increases down the dependency chain. Say there's a minor version number increase in Castle.Logging. We'd also have to up the minor version in all depending packages, all the way down to Castle.Core. If this approach is correct, I suppose we would give up the Explicit.NuGet.Versions tool.)

I could have this ready in a very short time.

[jonorossi]

@stakx I understand the proposal, let me do some thinking. Need to consider assembly identities, Windsor and the future of some of this stuff. Will get back to you.