BHoM_Adapter: refactoring- should all IBHoMObjects implement a default "base" comparer that when checking for equality first of all looks at the GUID?

[alelom]

Because I am not sure that the following is something we want (Push with RobotAdapter, which uses comparers):

here node duplicates are correctly filtered out -- but that just because they have a comparer implemented.

here bars duplicates are not filtered out -- even though they are have the same GUID (i.e. BHoM knows they are the same bar)

Implementing a default base comparer for IBHoMObjects that uses GUID for equality would solve this without having to manually implement a comparer for every type.
Not sure about potential side-effects though. Replace() and consequently Push, Pull etc should all work as usual, I think.

@IsakNaslundBh @al-fisher @epignatelli @adecler

[IsakNaslundBh]

First, I am not sure if I think the above behaviour is a bug. As you stated, the bars do not have a comparer implemented, hence the whole comparison is skipped. might be cases (not for structural packages maybe, but for other things) where we want to be able to push the same item twice for whatever reason, so no-comparer -> no culling I think is expected behaviour to some extent.

If we want to add a default comparer for the adapters I think a Guid comparer could work. Only thing to make sure is that the Guids are dealt with as we want in everything done before, leading up to the adapters. As we have not relied that heavily on the Guids up until this point it is probably worth checking the impact it has, but again, could work.

[alelom]

Thanks for the reply @IsakNaslundBh!

First, I am not sure if I think the above behaviour is a bug. As you stated, the bars do not have a comparer implemented, hence the whole comparison is skipped. might be cases (not for structural packages maybe, but for other things) where we want to be able to push the same item twice for whatever reason, so no-comparer -> no culling I think is expected behaviour to some extent.

Sure, I never called that a bug because I know it's a design behaviour. I'm just not sure it's the one we really want!
I just don't think the default behaviour should allow you to export the exact same item (bar or anything else) twice, which I think in 99% of cases is simply a mistake/overlook -- I can't even think of any case where anybody might want that intentionally. However, I'm not saying to remove this completely.
Where that is an intended action, I think the Adapter should rather 1. Prompt that it has found duplicates based on GUID 2. give the possibility to actually do it (perhaps in the config).
I think that something like that would be of help to beginners.

If we want to add a default comparer for the adapters I think a Guid comparer could work. Only thing to make sure is that the Guids are dealt with as we want in everything done before, leading up to the adapters. As we have not relied that heavily on the Guids up until this point it is probably worth checking the impact it has, but again, could work.

Agreed. As I said when we chatted, we have the GUIDs, why not trying to use them -- subject to proper checks of course :)