mgravell / fast-member

Automatically exported from code.google.com/p/fast-member
Apache License 2.0
1.03k stars 139 forks source link

New TrySet and TryGet methods? #66

Open AlexSikilinda opened 5 years ago

AlexSikilinda commented 5 years ago

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.

mgravell commented 5 years ago

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?

AlexSikilinda commented 5 years ago

@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
}

2) 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
}

3) 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