Things you don’t want to do with properties in C#

I was working with some code a couple of weeks ago and I stumbled over this “interesting” “pattern”…

1: public class SomeApiWrapper
2: {
3: private string _method = “”;
4: List<string> _arguments = new List<string>();
5: private string _instance = “”;
6: private string _result = “”;
8: public string Method
9: {
10: set
11: {
12: _method = value;
13: ExecuteIfReady();
14: }
15: }
17: public string Instance
18: {
19: set
20: {
21: _instance = value;
22: ExecuteIfReady();
23: }
24: }
26: public string Argument
27: {
28: set
29: {
30: _arguments.Add(value);
31: }
32: }
34: public string Result
35: {
36: get
37: {
38: if (_result.Length > 0)
39: return _result;
40: throw new InternalErrorException(“Method not executed.”);
41: }
42: }
44: private void ExecuteIfReady()
45: {
46: if (_instance.Length > 0 && _method.Length > 0)
47: _result = SomeApi.Execute(_instance, _method, _arguments);
48: }
49: }

So what is happening here? Well, first of all you need to set the arguments, the method and the instance properties and then you just get the result. Setting the properties magically executes the method and stores the result as soon as you’ve provided enough information for the method call to be completed. An interesting twist however is that you cannot set the arguments last since the method is executed when you have set method and instance properties. the ordering between these two are however irrelevant…

Please don’t do this. Ever. Properties should not have side effects in general and especially not side effects that are dependent of the order in which they are called. That is only confusing at best. If you read what I wrote the other day you might be confused because a DSL created using properties will have side effects. Well, no rule without an exception! Also consider properties that are lazy loaded (i.e. they do a costly retrieval of values only when asked but then remembers that value). That is also going to be OK. But the pattern you see above is not.

Preemptive comment: No this pattern was not seen in any code created by a Microsoft employee.

Comments (2)

  1. Doug says:

    Side effects in property setters can be useful in a ViewModels bound to Views in WPF or Silverlight.

    For example when the selected item in a combo box changes you want to have the side effect of filling an ObservableCollection so that you can view the child data of the selection.

    Do you have a suggestion for an alternate way or working in MVVM that would not involve side effects in setters?

  2. cellfish says:

    @Doug: Short answer is "I don’t know" since I’ve not worked a lot with WPF nor silverlight. From a generic perspective however (i.e. the long answer) all frameworks typically have some pain point and/or necesary "evil" where you may have to divert from your preferred design patterns in order to acomplish something. When this happens I don’t think it means you revert to a bader design. You keep your good design and then just plug it into the not so good design needed by your framework.

    An even longer answer is that the situation you describe is also different from the code in this post which I just find very confusing. I don’t see a big problem with a property affecting other properties/fields  when that make sense. So I honestly don’t think you need to seek an alternative solution for MVVM applications. Just don’t get too creative with your side effects…

    Even lobger answer: Having a property trigger an event when updated and having the observablecollection have an event handler would make most sense to me.