HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpModerate

Checking the validation of a field through the Property mutator method `set`

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
fieldthemethodsetcheckingvalidationpropertythroughmutator

Problem

I'm newish to C#, being mainly Java oriented. I have a field _config which needs to be a value from 1 to 8, and I have this code which checks to make sure I can only set _config to 1-8. If the value given for set is "invalid", _config becomes -1.

public int Config {
    get
    {
        return _config;
    }

    set
    {
        if (value > 8 || value < 1)
        {
            _config = -1;
        }
        else
        {
            _config = value;
        }
    }
}
private int _config; // the actual field that the property 'Config' refers to.


This feels really weird to me, but it does what I need it to do. I'm also thinking about having the setter throw an exception instead of setting _config to -1 when given an invalid value. Am I using Properties correctly, or have I completely misunderstood the point of them?

Solution

No one has yet addressed the fact that you have chosen the wrong tool for the job.

What does Config represent? It looks like you should have chosen an enum where you chose an int.

public enum ConfigurationType
{
    None = 0,
    Type1 = 1,
    Type2 = 2,
    // etc.
}


You can get what ConfigurationType an int corresponds to quite easily:

ConfigurationType configType = (ConfigurationType)someInteger;


Then, you can validate it more fluidly:

public ConfigurationType Config
{
    get
    {
        return _config;
    }
    set
    {
        if (ConfigurationType.IsDefined(typeof(ConfigurationType), value))
        {
            _config = value;
        }
        else
        {
            throw new InvalidEnumArgumentException($"The specified value must be a valid value of the {nameof(ConfigurationType)} enumeration.");
        }
    }
}
private ConfigurationType _config; // the actual field that the property 'Config' refers to.


If you don't have C#6.0, your exception would be more along the lines of:

throw new InvalidEnumArgumentException(string.Format("The specified value must be a valid value of the {0} enumeration.", ConfigurationType.ToString()));


You should also consider a refactoring of the name Config, it's non-descriptive. There is likely a better name for such a property/field combination. (Had we more context we would likely be able to suggest better names.)

Code Snippets

public enum ConfigurationType
{
    None = 0,
    Type1 = 1,
    Type2 = 2,
    // etc.
}
ConfigurationType configType = (ConfigurationType)someInteger;
public ConfigurationType Config
{
    get
    {
        return _config;
    }
    set
    {
        if (ConfigurationType.IsDefined(typeof(ConfigurationType), value))
        {
            _config = value;
        }
        else
        {
            throw new InvalidEnumArgumentException($"The specified value must be a valid value of the {nameof(ConfigurationType)} enumeration.");
        }
    }
}
private ConfigurationType _config; // the actual field that the property 'Config' refers to.
throw new InvalidEnumArgumentException(string.Format("The specified value must be a valid value of the {0} enumeration.", ConfigurationType.ToString()));

Context

StackExchange Code Review Q#104638, answer score: 10

Revisions (0)

No revisions yet.