fast-member icon indicating copy to clipboard operation
fast-member copied to clipboard

New TrySet and TryGet methods?

Open AlexSikilinda opened this issue 6 years ago • 2 comments

Hi,

Currently if you try to set a value for a nonexistent property, an ArgumentOutOfRange exception is thrown. In some scenarios it would be beneficial to have TrySet (and TryGet correspondingly) method, which will return true/false instead of throwing.

Based on indexer code, it's quite straightforward (well, possible) to implement:

public override object this[object target, string name]
{
    get
    {
        int index;
        if (map.TryGetValue(name, out index)) return getter(index, target);
        else throw new ArgumentOutOfRangeException("name");
    }
    set
    {
        int index;
        if (map.TryGetValue(name, out index)) setter(index, target, value);
        else throw new ArgumentOutOfRangeException("name");
    }
}

I can submit a PR, just want to make sure I am not missing anything and adding these methods is okay.

AlexSikilinda avatar Dec 24 '18 03:12 AlexSikilinda

the implementation shown only works for one of the concrete implementations - it fails for the most common cases; in reality, it is quite a bit more complex than this. It may be worth doing, but... I wonder whether some kind of IsDefined(name) might make more sense?

mgravell avatar May 24 '19 13:05 mgravell

@mgravell ,IsDefined(name) is an option, but I still like TryGet(string, out object)\ TrySet(string, out object) more.

Let me give you some context:

  1. The way we do it now to avoid ArgumentOutOfRange:
// we store all the members of BusinessObject in private HashSet, so that we can check it exists
static readonly HashSet<string> objectMembers =
    new HashSet<string>(typeof(BusinessObject).GetMembers().Select(m => m.Name));

// in the method
if (objectMembers.Contains(propertyName))
{
    ObjectAccessor accessor = ObjectAccessor.Create(mergedObject);
    object value = accessor[propertyName];
    // working with value here
}
  1. Proposed IsDefined(names) improves it since we don't need the HashSet anymore:
ObjectAccessor accessor = ObjectAccessor.Create(mergedObject);
if (accessor.IsDefined(propertyName))
{
    object value = accessor[propertyName];
    // working with value here
}
  1. But this is where TryGet is useful, since we avoid extra IsDefined call:
ObjectAccessor accessor = ObjectAccessor.Create(mergedObject);
if (accessor.TryGet(propertyName, out object value))
{
    // working with value here
}

Again, this is probably very specific to my project, but it could be useful in similar scenarios. Kind of the same way we have throwing\nonthrowing methods equivalents in .NET framework, like int.Parse\ int.TryParse

AlexSikilinda avatar May 24 '19 14:05 AlexSikilinda