Question

I've always been of the mindset that properties (ie, their set/get operations) should be fast/immediate and failure-free. You should never have to try/catch around getting or setting a property.

But I'm looking at some ways to apply role-based security on the properties of some objects. For instance an Employee.Salary property. Some of the solutions I've run across that others have tried (one in particular is the AOP example here) involve throwing an exception if the accessor doesn't have the right permissions - but this goes against a personal rule that I've had for a long time now.

So I ask: am I wrong? Have things changed? Has it been accepted that properties should be able to throw exceptions?

Was it helpful?

Solution

when you set the value of a property, throwing an exception on an invalid value is fine

getting the value of a property should (almost) never throw an exception

for role-based access, use different/dumber interfaces or facades; don't let people see things they can't have!

OTHER TIPS

I consider it to mostly be bad form but even Microsoft recommends using exceptions sometimes. A good example is the abstract property Stream.Length. As guidelines go I would be more concerned with avoiding side effects on getters and limiting side effects on setters.

I would definitely argue that there is a flaw in the design if you feel the need to throw exceptions from a property setter or getter.

A property is an abstraction that represents something that is just a value. And you should be able to set a value without fearing that doing so could throw an exception.*

If setting the property results in a side effect, that that should really be implemented as a method instead. And if it doesn't produce any side effects, then no exception should be thrown.

One example already mentioned in a different answer is the Stream.Position property. This does produce side effects, and may throw exceptions. But this property setter is basically just a wrapper around Stream.Seek that you could call instead.

Personally, I believe that the position should not have been a writeable property.

Another example where you could be tempted to throw an exception from a property setter is in the validation of data:

public class User {
    public string Email {
        get { return _email; }
        set { 
            if (!IsValidEmail(value)) throw InvalidEmailException(value);
            _email = value;
        }
    }

But there is a better solution to this problem. Introduce a type representing a valid email address:

public class Email {
    public Email(string value) {
        if (!IsValidEmail(value)) throw new InvalidEmailException(value);
        ...
    }
    ...
}

public class User {
    public Email Email { get; set; }
}

The Email class ensures that it cannot hold a value that is not a valid email address, and classes that need to store emails are relieved of the duty of validating them.

This also leads to higher cohesion (an indicator of good software design) - the knowledge about what an email address is, and how it is validated, exists only in the Email class, which has only that concern.

* ObjectDisposedException is the only valid exception (no pun intended) I can think of at the moment.

I know your question is specific to .NET, but since C# shares some history with Java, I thought you might be interested. I am not in any way implying that because something is done in Java, it should be done in C#. I know the two are very different, especially in how C# has much-improved language-level support for properties. I am just giving some context and perspective.

From the JavaBeans specification:

Constrained properties Sometimes when a property change occurs some other bean may wish to validate the change and reject it if it is inappropriate. We refer to properties that undergo this kind of checking as constrained properties. In Java Beans, constrained property setter methods are required to support the PropertyVetoException. This documents to the users of the constrained property that attempted updates may be vetoed. So a simple constrained property might look like:

PropertyType getFoo();
void setFoo(PropertyType value) throws PropertyVetoException;

Take all this with a grain of salt, please. The JavaBeans specification is old, and C# Properties are (IMO) a huge improvement over the "naming convention" based properties that Java has. I'm just trying to give a little context, is all!

The point of a property is the Uniform Access Principle, i.e. that a value should be accessible through the same interface whether implemented by storage or computation. If your property is throwing an exception that represents an error condition beyond the programmer's control, the kind that is supposed to be caught and handled, you are forcing your client to know that the value is obtained via computation.

On the other hand, I don't see any problem with using asserts or assertion-like exceptions that are meant to signal incorrect use of the API to the programmer rather than being caught and handled. In these cases the correct answer from the API user's perspective isn't to handle the exception (thus implicitly caring about whether the value is obtained via computation or storage). It's to fix his/her code so the object doesn't end up in an invalid state or make you fix your code so the assertion doesn't fire.

AFAIK, this guidance primarily comes from the thought process that properties may end up used at design time. (E.g. the Text property on a TextBox) If the property tosses an exception when a designer's trying to access it, VS isn't going to have a good day. You will get a bunch of errors just trying to use the designer and for the UI designers, they won't render. It also applies to debug time as well, although what you'll see in an exception scenario is just "xxx Exception" and it won't clobber VS IIRC.

For POCOs, it won't really do any harm, but I still shy away from doing it myself. I figure people will want to access properties more frequently, so they should normally be low cost. Properties shouldn't do the work of methods, they should just get/set some info and be done with it as a general rule.

Although a lot will depend on the context I'd generally avoid putting any sort of breakable logic (including exceptions) into properties. Just as one example, there's a lot of things which you (or some library that you're using) could do which use reflection to iterate over properties resulting in errors you didn't anticipate at design-time.

I say generally though as there may be some times when you absolutely, definitely, want to block something without worrying too much about the consequences. Security I guess would be the classic case there.

Licensed under: CC-BY-SA with attribution
scroll top