Best practices: throwing exceptions from properties
Asked Answered
P

9

129

When is it appropriate to throw an exception from within a property getter or setter? When is it not appropriate? Why? Links to external documents on the subject would be helpful... Google turned up surprisingly little.

Presswork answered 28/9, 2009 at 18:1 Comment(4)
See also: #634444Desquamate
Also related: #1488822Upheaval
I read both those questions, but neither answered this question completely IMO.Presswork
Whenever necessary. Both previous questions and answers show that it is allowed and appreciate to raise exceptions from getter or setter, so you can simple "be smart".Antiserum
P
151

Microsoft has its recommendations on how to design properties at http://msdn.microsoft.com/en-us/library/ms229006.aspx

Essentially, they recommend that property getters be lightweight accessors that are always safe to call. They recommend redesigning getters to be methods if exceptions are something you need to throw. For setters they indicate that exceptions are an appropriate and acceptable error handling strategy.

For indexers, Microsoft indicates that it is acceptable for both getters and setters to throw exceptions. And in fact, many indexers in the .NET library do this. The most common exception being ArgumentOutOfRangeException.

There are some pretty good reasons why you don't want to throw exceptions in property getters:

  • Because properties "appear" to be fields, it is not always apparent that they can throw a (by-design) exception; whereas with methods, programmers are trained to expect and investigate whether exceptions are an expected consequence of invoking the method.
  • Getters are used by a lot of .NET infrastructure, like serializers and databinding (in WinForms and WPF for example) - dealing with exceptions in such contexts can rapidly become problematic.
  • Property getters are automatically evaluated by debuggers when you watch or inspect an object. An exception here can be confusing and slow down your debugging efforts. It's also undesirable to perform other expensive operations in properties (like accessing a database) for the same reasons.
  • Properties are often used in a chaining convention: obj.PropA.AnotherProp.YetAnother - with this kind of syntax it becomes problematic to decide where to inject exception catch statements.

As a side note, one should be aware that just because a property is not designed to throw an exception, that doesn't mean it won't; it could easily be calling code that does. Even the simple act of allocating a new object (like a string) could result in exceptions. You should always write your code defensively and expect exceptions from anything you invoke.

Pharyngitis answered 28/9, 2009 at 18:1 Comment(16)
This isn't to say that exceptions won't ever be thrown, though. Consider that any Get property that allocates JIT has the potential to throw due to the constructor. Would code really be better if it caught that exception silently?Obmutescence
If you're running into a fatal exception like "out of memory", it hardly matters whether you get the exception in a property or somewhere else. If you didn't get it in the property, you'd just get it a couple nanoseconds later from the next thing that allocates memory. The question isn't "can a property throw an exception?" Almost all code can throw an exception due to a fatal condition. The question is whether a property should by design throw an exception as part of its specific contract.Wetmore
I'm not sure I understand the argument in this answer. For xample, regarding databinding - both WinForms and WPF are specifically written to properly handle exceptions thrown by properties, and to treat them as validation failures - which is a perfectly fine (some even believe it to be the best) way to provide domain model validation.Winters
@Pavel - while both WinForms and WPF can gracefully recover from exceptions in property accessors, it is not always easy to identify and recover from such errors. In certain cases, (such as when in WPF a Control Template setter throws an exception) the exception is silently swallowed. This can lead to painful debugging sessions if you've never run across such cases before.Pharyngitis
@Eric: If I understand what you're saying, then I agree. I'm not sure about the edge cases, though. For example, at one point I coded a class whose static constructor read some settings from a file, which would then be available as static Get properties. If it threw in that constructor, I caught it and set an invalid flag, so that every attempt to read a property would then throw an exception. Do you think that was a bad idea?Obmutescence
@Steven: Then how much use was that class to you in the exceptional case? If you've then had to write defensive code to handle all those exceptions because of one failure, and presumably supply suitable defaults, why not provide those defaults in your catch? Alternatively if the property exceptions are thrown to the user why not just throw the original "InvalidArgumentException" or similar so they could supply the missing settings file?Mitran
There's a reason why these are guidelines and not rules; no guideline covers all the crazy edge cases. I probably would have made these methods and not properties myself, but it's a judgement call.Wetmore
@Zhaph: If the static constructor failed, that class would have been utterly useless, which is why I made the properties throw on get. I could not throw the original exception because there's no handy way to clone it, but I did log that exception and mention the log in the message. @Eric: Fair enough; I can certainly understand why that would make sense.Obmutescence
I personally wouldn't and let an exception handler catch this. I had an issue at my work where another developer was very very annoyed that I caught an exception an re-threw it on certain conditions. For myself, I never saw someone throw exceptions in a get. I would have made it a method. Stack trace is much more readable. If you're in a web solution, you should have generic logging anyway and mask specific details from the user.Mccaslin
My issue was that the developer wasnt available at the time and due to a near release, I had to take a minimal risk approach disregarding performance, best practices because of this. Gotta remember, just cause something makes complete sense to you, doesn't mean it will to the next guy who goes "i need this value" but wont expect an exception from it.Mccaslin
I've seen exceptions thrown on static class getters at several different companies. These are usually lazy loaded application wide configuration/setting values that the application can't run without and are queried repetitiously. I've never questioned this, because making the values properties seemed descriptive. It indicated there was a real value in memory and that no work was being done. Where as a method would lead a developer to think it was loaded new each call leading to performance concerns or a misconception about the values life, perhaps leading to stale conditions.Mimetic
Is there a better way to handle this type of global value? Isn't DateTime.Now a violation of this suggestion, because there is obviously work going on behind the property.Mimetic
These are guidlines, and like most guidelines, it is sometimes better to break them than to enforce them. For example the .NET Nullable<T>.Value's getter throws when no value is assigned. It could have been done using a GetValue() method, but using a property, at least to me, seems nicer.Proof
It's funny how their own guidelines state that properties should be lightweight, safe calls when their own properties on windows forms classes are anything but.Mensch
@StevenSudit , I know I'm late here, but given you said your class is “utterly useless” if its constructor failed, I would have made the constructor private and exposed a static factory method that throws whatever exception you have to handle. This way, reading from a file should have been part of the factory method, and when instantiating that class using such a factory method, you would expect an exception to be thrown if the file didn't exist, rather than getting a stub instance that does nothing other than always throwing exceptions.Polished
@EricWu , like Eric Lippert has pointed out, these are guidelines (not rules) for a reason. WinForms couldn't have been different, since for instance the Control class is a wrapper over native calls, which uses P/Invoke to interoperate with the Windows kernel. It follows that if they had used fields to look up the values of the properties, they would have gotten wrong if someone (even another program) used P/Invoke (or it was even a C program, or anything compiled to machine code) to set those properties to other values, completely ignoring the managed wrappers the framework offers.Polished
W
40

There's nothing wrong with throwing exceptions from setters. After all, what better way to indicate that the value is not valid for a given property?

For getters, it is generally frowned upon, and that can be explained pretty easily: a property getter, in general, reports the current state of an object; thus, the only case where it is reasonable for a getter to throw is when the state is invalid. But it is also generally considered to be a good idea to design your classes such that it is simply not possible to get an invalid object initially, or to put it into invalid state via normal means (i.e., always ensure full initialization in constructors, and try make methods exception-safe with respect to state validity and class invariants). So long as you stick to that rule, your property getters should never get into a situation where they have to report invalid state, and thus never throw.

There is one exception I know of, and it's actually a rather major one: any object implementing IDisposable. Dispose is specifically intended as a way to bring object into an invalid state, and there's even a special exception class, ObjectDisposedException, to be used in that case. It is perfectly normal to throw ObjectDisposedException from any class member, including property getters (and excluding Dispose itself), after the object has been disposed.

Winters answered 28/9, 2009 at 18:1 Comment(6)
Thanks Pavel. This answer goes in to 'why' instead of simply stating again that it's not a good idea to throw an exception from the properties.Stratiform
I dislike the notion that absolutely all members of an IDisposable should be made useless after a Dispose. If invoking a member would require the use of a resource which Dispose has made unavailable (e.g. the member would read data from a stream which has been closed) the member should throw ObjectDisposedException rather than leaking e.g. ArgumentException, but if one has a form with properties that represent the values in certain fields, it would seem far more helpful to allow such properties to be read after disposal (yielding the last typed values) than to require...Periodate
...that Dispose be deferred until after all such properties have been read. In some cases where one thread might use blocking reads on an object while another closes it, and where data might arrive at any time prior to Dispose, it may be helpful to have Dispose cut off incoming data, but allow previously-received data to be read. One shouldn't force an artificial distinction between Close and Dispose in situations where none would otherwise need to exist.Periodate
Understanding the reason for the rule allows you to know when to break the rule (Raymond Chen). In this case, we can see that if there is an unrecoverable error of any type you shouldn't hide it in the getter, since in such cases the application needs to quit ASAP.Orren
The point I was trying to make is that your property getters generally shouldn't contain logic that would allow for unrecoverable errors. If it does, it may be that it's better off as a Get... method instead. An exception here is when you have to implement an existing interface that requires you to provide a property.Winters
One classic example of a getter throwing an exception is lazy loading of linked objects in database classes; if, for some reason, the loading fails, an exception should be thrown, simply because otherwise you can't make a difference between a failure or the linked object simply being empty.Horripilate
W
25

It is almost never appropriate on a getter, and sometimes appropriate on a setter.

The best resource for these sorts of questions is "Framework Design Guidelines" by Cwalina and Abrams; it's available as a bound book, and large portions of it are also available online.

From section 5.2: Property Design

AVOID throwing exceptions from property getters. Property getters should be simple operations and should not have preconditions. If a getter can throw an exception, it should probably be redesigned to be a method. Note that this rule does not apply to indexers, where we do expect exceptions as a result of validating the arguments.

Note that this guideline only applies to property getters. It is OK to throw an exception in a property setter.

Wetmore answered 28/9, 2009 at 18:1 Comment(4)
While (in general) I agree with such guidelines, it think it's useful to provide some additional insight as to why they should be followed - and what kind of consequences can arise when they are ignored.Pharyngitis
How does this relate to disposable objects and the guidance that you should consider throwing ObjectDisposedException once the object has had Dispose() called and something subsequently asks for a property value? It seems like the guidance should be "avoid throwing exceptions from property getters, unless the object has been disposed in which case you should consider throwing an ObjectDisposedExcpetion".Laplante
Design is the art and science of finding reasonable compromises in the face of conflicting requirements. Either way seems like a reasonable compromise; I would not be surprised to have a disposed object throw on a property; nor would I be surprised if it did not. Since using a disposed object is a terrible programming practice, it would be unwise to have any expectations.Wetmore
Another scenario where it's totally valid to throw exceptions from within getters is when an object is making use of class invariants to validate its internal state, which need to be checked whenever a public access is made, no matter it's a method or a propertyBini
A
1

I had this code where I was unsure of which exception to throw.

public Person
{
    public string Name { get; set; }
    public boolean HasPets { get; set; }
}

public void Foo(Person person)
{
    if (person.Name == null) {
        throw new Exception("Name of person is null.");
        // I was unsure of which exception to throw here.
    }

    Console.WriteLine("Name is: " + person.Name);
}

I prevented the model from having the property being null in the first place by forcing it as an argument in the constructor.

public Person
{
    public Person(string name)
    {
        if (name == null) {
            throw new ArgumentNullException(nameof(name));
        }
        Name = name;
    }

    public string Name { get; private set; }
    public boolean HasPets { get; set; }
}

public void Foo(Person person)
{
    Console.WriteLine("Name is: " + person.Name);
}
Ancy answered 28/9, 2009 at 18:1 Comment(0)
K
1

One nice approach to Exceptions is to use them to document code for yourself and other developers as follows:

Exceptions should be for exceptional program states. This means it's fine to write them wherever you want!

One reason you might want to put them in getters is to document the API of a class - if the software throws an exception as soon as a programmer tries to use it wrong then they wont use it wrong! For instance if you have validation during a data reading process it may not make sense to be able to continue and access the results of the process if there were fatal errors in the data. In this case you may want to make getting the output throw if there were errors to ensure that another programmer checks for this condition.

They are a way of documenting the assumptions and boundaries of a subsystem/method/whatever. In the general case they should not be caught! This is also because they are never thrown if the system is working together in the way expected: If an exception happens it shows that the assumptions of a piece of code are not met - eg it is not interacting with the world around it in the way it was originally intended to. If you catch an exception that was written for this purpose it probably means the system has entered an unpredictable/inconsistent state - this may ultimately lead to a crash or corruption of data or similar which is likely to be much harder to detect/debug.

Exception messages are a very coarse way of reporting errors - they cannot be collected en-masse and only really contain a string. This makes them unsuitable for reporting problems in input data. In normal running the system itself should not enter an error state. As a result of this the messages in them should be designed for programmers and not for users - things that are wrong in input data can be discovered and relayed to users in more suitable (custom) formats.

The Exception (haha!) to this rule is things like IO where exceptions are not under your control and cannot be checked for in advance.

Kelantan answered 28/9, 2009 at 18:1 Comment(1)
How did this valid and relevant answer get downvoted? There should be no politics in StackOverflow, and if this answer seemed to miss the bullseye, add a comment to that effect. Downvoting is for answers that are irrelevant or wrong.Hartsock
C
1

This is all documented in MSDN (as linked to in other answers) but here is a general rule of thumb...

In the setter, if your property should be validated above and beyond type. For example, a property called PhoneNumber should probably have regex validation and should throw an error if the format is not valid.

For getters, possibly when the value is null, but most likely that is something you will want to handle on the calling code (per the design guidelines).

Carolyn answered 28/9, 2009 at 18:1 Comment(0)
O
0

Fast Forward to 2023 and there are now new guidelines for throwing exceptions from a Property.

For example:

public string ConnectionString => _connectionString ?? throw new NullReferenceException("You forgot to inialize the connection");

I now use this as a part of Nullable reference types in an effort to avoid unexpected NullReferenceExceptions. In this case, I explicitly throw the exception, but I include a message as to what happened so it is now a clear error message.

Omor answered 28/9, 2009 at 18:1 Comment(0)
M
0

This is a very complex question and answer depends on how your object is used. As a rule of thumb, property getters and setters that are "late binding" should not throw exceptions, while properties with exclusively "early binding" should throw exceptions when the need arises. BTW, Microsoft's code analysis tool is defining the use of properties too narrowly in my opinion.

"late binding" means that properties are found through reflection. For example the Serializeable" attribute is used to serialize/deserialize an object via its properties. Throwing an exception during in this kind of situation breaks things in a catastrophic way and is not a good way of using exceptions to make more robust code.

"early binding" means that a property use is bound in the code by the compiler. For example when some code that you write references a property getter. In this case it is OK to throw exceptions when they make sense.

An object with internal attributes has a state determined by the values of those attributes. Properties expressing attributes that are aware and sensitive to the object's internal state should not be used for late binding. For example, lets say you have an object that must be opened, accessed, then closed. In this case accessing the properties without calling open first should result in an exception. Suppose, in this case, that we do not throw an exception and we allow the code access to a value without throwing an exception? The code will seem happy even though it got a value from a getter that is non-sense. Now we have put the code that called the getter in a bad situation since it must know how to check the value to see if it is non-sense. This means that the code must make assumptions about the value that it got from the property getter in order to validate it. This is how bad code gets written.

Motorboating answered 28/9, 2009 at 18:1 Comment(0)
M
0

MSDN: Catching and Throwing Standard Exception Types

http://msdn.microsoft.com/en-us/library/ms229007.aspx

Meteorology answered 28/9, 2009 at 18:1 Comment(0)

© 2022 - 2024 — McMap. All rights reserved.