Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New TrySet and TryGet methods? #66

Open
AlexSikilinda opened this issue Dec 24, 2018 · 2 comments
Open

New TrySet and TryGet methods? #66

AlexSikilinda opened this issue Dec 24, 2018 · 2 comments

Comments

@AlexSikilinda
Copy link

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
Copy link
Owner

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
Copy link
Author

AlexSikilinda commented May 24, 2019

@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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants