The JADE 2022 release meets tomorrow’s business demands.
The online help for tryAddIfNotNull: "attempts to add the value specified by the value parameter to the collection if the value is not null and it is not already contained in the collection. This method returns true if the value was successfully added; otherwise it returns false."
It leaves out the bit where it throws an exception if the key is already used in this dictionary. Currently the wording implies this function will return false gracefully, not error.
The text for tryAdd's online help regarding the exception should be copied to this and similar methods.
The tryAdd methods aren't about preventing duplicates, they're about preventing attempts to re-add the same values/objects that are already in the collection. The 1310 exception is not thrown because the same object is already in a dictionary, it's thrown when there's another object already in the dictionary using the same key values. If you want to allow duplicates in this scenario, you need to change the dictionary definition to allow duplicate keys. This exception can also occur when you've got an inverse relationship setup, and the key values are changed for an object after they've been added to a collection in such a way that they duplicate the key values for another object already in the collection.
I'd advise against the idea of a tryAddIfNotDuplicate method to suppress 1310 exceptions in a generic way as that could end up masking problems. While this may be okay for specific scenarios, more often than not, these exceptions highlight wider data model/design issues that need to be addressed, whether that be the need for more keys, allowing duplicates, or validation to prevent the creation of duplicates records with the same unique key values.
If you implement your own method on Collection, you’ll lose the benefit of the collection not being exclusively locked unless it needs to perform the maintenance. Not an issue with transient collections, but it could impact contention between processes on persistent collections. It may be better for those places where 1310s could be raised, which hopefully isn’t too many places, to arm an exception handler prior to calling the current Collection::tryAddIfNotNull method.
I'd upvote a new Collection::tryAddIfNotNullIgnoreDuplicateKeys and deferred equivalent in RootSchema, to handle ignoring duplicate keys as well as avoiding exclusive locks unless maintenance is required. This method clearly implies that behaviour, whereas having Collection;:tryAddIfNotNull suppress 1310s could potentially hide application bugs from the developer.
To continue conversing with the, erm, advocate, good chance I will create a .tryAddIfNotDuplicate to capture the 1310. As that's how I interpreted the original documentation.
Just playing Devil's advocate here.
The tryXXX methods were to replace all the boiler plate logic doing "if not includes then add" and "if includes then remove" logic, which in a number of systems opened up opportunity for deadlocks on persistent collections, as well as avoiding the lock altogether if no collection maintenance was required.
These methods weren't designed to prevent all possible exceptions that could be raised by a call to the non-try version of the method. The "otherwise false" doesn't mean that it would never raise any exceptions, including but not limited to 1310 duplicate key, 1027 object locked, 1081 deadlock, 1026 not in transaction state, etc.
The Collection::tryAddIfNotNull will of course prevent the 1309 object already in collection and 1311 null object reference cannot be passed to collection method exceptions that it is specifically targeted to avoid.