Contradictory validation in TrackableCollectionConfiguration.ReplaceImplementation

[ImAMightyPirate]

It looks like the checks for IsGenericTypeDefinition and IsAssignableFrom contradict each other. The former ensures that an open generic type is provided for the interface, while the latter will only work with closed generic types.

Trying to replace an existing collection (that should be allowed) implementation demonstrates this:-

config.Collections.ReplaceImplementation(
    typeof(ICollection<>), 
    typeof(List<>), 
    typeof(TrackerDog.CollectionHandling.DefaultCollectionChangeInterceptor<>));

[mfidemraizer]

@ImAMightyPirate Hey! First of all thank you for looking at TrackerDog's code. As we're not machines, we make mistakes and it seems like you're right.

BTW, I'll fix this during this weekend.

Anyway, did you find this bug because you were going to replace a collection change interceptor? Or it was reviewing the code? I'm just curious ;)

[ImAMightyPirate]

I was trying to replace an interceptor with one of our own (to support ObservableCollection). I had tried a number of permutations of calling ReplaceImplementation to do this without success.

I ended up in the source code to see how things worked for your default interceptors (I thought I might have been doing something wrong), which led me down the path of trying to replace an existing interceptor with an identical one.

As a side note, I have been very impressed by the ease of use of TrackerDog so far. It suits our need to track changes only on certain properties of our domain objects perfectly!

[mfidemraizer]

@ImAMightyPirate I see I see!

I'm glad that a project made by an unknown like me could be useful for you and others! This is the magic of open source!

If you find issues don't hetistate to ask for more assistance. The greater is the challenge, the more we'll learn!

Be sure I'll be on this in the next couple of days (next weekend). In the other hand, if you find more problems, create new issues.

[mfidemraizer]

@ImAMightyPirate I forgot to say that I created TrackerDog with DDD in mind, as my main goal with this project has been always being able to implement completely agnostic domain transactions and, unless you're backed by an OR/M, real-world project timings aren't enough to implement a good domain unit of work!

EDIT Anyway, current NuGet package version is already .NET Standard and .NET Full Framework (i.e. it's already a version built from the v2-netstandard branch).

Probably I'll both fix this issue and merge the .NET Standard branch. Note that v2 as is branch isn't the latest code base, but our current issue is still there in any branch, even v1 (which I won't update, as it wasn't DI-friendly and as TrackerDog isn't a popular library by no means, I find that v1 branch should be there for the posterity).

[f135ta]

Hi,
I'm waiting on this fix pretty urgently too, any ideas when you will have time to implement it?

[mfidemraizer]

Sorry for the inconvenience, @ImAMightyPirate and @f135ta

It's a long story but I've had to make a code test for some company and I took more than I ever expected, hence I couldn't invest time on solving the whole problem.

As it seems to be a simple fix, I expect to fix it today.

I apologize for the inconveniences since I'm very glad you both are relying on TrackerDog!

[f135ta]

Brilliant news! Thank you! 👍

[mfidemraizer]

@f135ta It would be nice to know what's your goal when using TrackerDog! It might be awesome that I could add a wiki page with summaries about use cases :)

[mfidemraizer]

@f135ta @ImAMightyPirate

I've already committed the solution. BTW, before releasing the NuGet package, I've built a DLL for you so you can check if there're no further issues.

TrackerDog.zip

Since TrackerDog is still used by few people, I've decided to completely drop AddImplementation/ReplaceImplementation and I've implemented a new method called AddOrUpdateImplementation. It differs from the old ones in which the new method uses generic parameters instead of regular parameters.

For example, you might use it as follows:

config.AddOrUpdateImplementation<IList<string>, List<string>, DefaultCollectionChangeInterceptor<string>>();

Since you can't provide open generic types/generic type definitions to generic parameters, I've opted-in to give a dummy collection T type. AddOrUpdateImplementation will still configure generic type definitions. It's just for the sake of using generic constraints as much as possible: TImplementation must implement/inherit TInterface.

Let me know if you've found more issues or it's already ok and I'll release the NuGet package v2.2.1.

Thank you!

[mfidemraizer]

@f135ta @ImAMightyPirate

Well, I'll await 2-3 days more for your responses. Otherwise, I'll publish a new TrackerDog version and if you're using it and you both find more issues related or unrelated to current one, we'll treat them separately.

[ImAMightyPirate]

Apologies for the delay in responding (I have been on holiday for the last week). Your fix resolved the error I was seeing. Thank you for making this change.

I am having some difficulty with working collections still, but will raise that as a new topic when I have the opportunity to do so and I have some code samples to provide as I do not believe it is related to this.

[mfidemraizer]

@ImAMightyPirate No problem! ;)

Feel free to post them, don't scratch your head! If you're experiencing issues, even if those aren't errors within TrackerDog and are related to the library usage, I'll be glad to assist you on them too.

I couldn't cover all use cases in TrackerDog, yet they can be easily implemented.

Therefore, I'll look forward for your other issues!

[mfidemraizer]

I'm going to close this issue. I've already released a new NuGet package versioned as 2.2.1 which already includes the bugfix to this issue!

Thank you again for your feedback and I'll be glad to assist you with those other issues that you've already found and you still need to add write down them!