Best way of protect a backing field from mistaken use in C#

Question

I have a class (Foo) which lazy loads a property named (Bar). What is your preferred way to protect against mistaken use (due to intellisense or inexperienced staff) of the uninitialized backing field?

I can think of 3 options:

  class Foo {
    // option 1 - Easy to use this.bar by mistake. 
    string bar;
    string Bar {
        get {
            // logic to lazy load bar  
            return bar; 
        }
    }

    // option 2 - Harder to use this._bar by mistake. It is more obscure.
    string _bar2;
    string Bar2 {
        get {
            // logic to lazy load bar2  
            return _bar2;
        }
    }

    //option 3 - Very hard to use the backing field by mistake. 
    class BackingFields {
        public string bar; 
    }

    BackingFields fields = new BackingFields();

    string Bar3 {
        get {
            // logic to lazy load bar  
            return fields.bar;
        }
    }

}

Keep in mind, the only place I want people mucking around with the backing field bar is in setter and getter of the property. Everywhere else in the class they should always use this.Bar

Update

I am currently using the following Lazy implementation (not for all properties with backing fields, but for select ones that require lazy loading, synchronization and notification). It could be extended to support futures as well (force evaluation in a separate thread in a later time)

Note My implementation locks on read, cause it supports an external set.

Also, I would like to mention that I think this is a language limitation which can be overcome in Ruby for example.

You can implement lazy in this way.

x = lazy do
    puts "<<< Evaluating lazy value >>>"
    "lazy value"
end

puts x
# <<< Evaluating lazy value >>>
# lazy value

Answer 1

I usually just go for option 1. Because it is a private field I don't think it really an issue, and using something like the wrapper class as in your option 3 only makes code difficult to read and understand.

Answer 2

Option 1, coupled with some education.

Rationale: software is meant to be read more often than written, so optimize for the common case and keep it readable.

Answer 3

// option 4
class Foo
{
    public int PublicProperty { get; set; }
    public int PrivateSetter { get; private set; }
}

C# 3.0 feature, the compiler will generate anonymous private backing fields which can't be accessed by mistake, well unless you use reflection...

EDIT: Lazy instantiation

You can have laziness like this:

// I changed this with respect to ShuggyCoUk's answer (Kudos!)
class LazyEval<T>
{
  T value;
  Func<T> eval;
  public LazyEval(Func<T> eval) { this.eval = eval; }
  public T Eval()
  {
      if (eval == null)
          return value;
      value = eval();
      eval = null;
      return value;
  }
  public static implicit operator T(LazyEval<T> lazy) // maybe explicit
  {
    return lazy.Eval();
  }
  public static implicit operator LazyEval<T>(Func<T> eval) 
  {
    return new LazyEval(eval);
  } 
}

Those implicit conversion make the syntax tidy...

// option 5
class Foo
{
    public LazyEval<MyClass> LazyProperty { get; private set; }
    public Foo()
    {
        LazyProperty = () => new MyClass();
    }
}

And closures can be used to carry scope:

// option 5
class Foo
{
    public int PublicProperty { get; private set; }
    public LazyEval<int> LazyProperty { get; private set; }
    public Foo()
    {
        LazyProperty = () => this.PublicProperty;
    }
    public void DoStuff()
    {
        var lazy = LazyProperty; // type is inferred as LazyEval`1, no eval
        PublicProperty = 7;
        int i = lazy; // same as lazy.Eval()
        Console.WriteLine(i); // WriteLine(7)
    }
}

Answer 4

I usually go for option 2, as it is easier to spot mistakes later on, although option 1 would pass a code review. Option 3 seems convoluted and whilst it may work, it's not going to be nice code to revisit 6 months down the line whilst trying to refactor/fix a bug/etc.

Answer 5

Option 4 (a new solution):

See if the question is really about how to prevent people from using an uninitialized variable then init it with an KNOWN INVALID value.

I would say something like:

string str = "SOMETHING_WRONG_HERE";

Who ever is using 'str' will have some sort of warning.

Otherwise Option 3 if preventing users from using 'str' is more important than readability etc.

Answer 6

Automatic properties:

public int PropertyName { get; set; }

will prevent access to the backing field. But if you want to put code in there (e.g. for lazy loading on first access) this clearly won't help.

The simplest route is likely to be a helper type which does the lazy loading, and have an private field of that type, with the public property calling to the correct property/method of the helper type.

E.g.

public class Foo {
  private class LazyLoader {
    private someType theValue;
    public someType Value {
      get {
        // Do lazy load
        return theValue;
      }
    }
  }

  private LazyLoader theValue;
  public someType {
    get { return theValue.Value; }
  }
}

This has the advantage that the backing field is harder to use than the property.

(Another case of an extra level of indirection to solve problems.)

Answer 7

Option 5

Lazy<T>

works quite nicely in several situations, though option 1 should really be just fine for most projects so long as the developers aren't idiots.

Adding [EditorBrowsable(EditorBrowsableState.Never)] to the field won't help if it is private since this logic only kicks in for intellisense generated from metadata rather than the current code (current project and anything done via project references rather than dlls).

Note: Lazy<T> is not thread safe (this is good, there's no point locking if you don't need to) if you require thread safety either use one of the thread safe ones from Joe Duffy or the Parallel Exetensions CTP

Answer 8

Code reviews will catch misuse so just go with the most readable. I dislike attempts to work around bad programmers in code, because 1) they don't work, 2) they make it harder for smart programmers to get their work done, and 3) it addresses the symptom rather than the cause of the problem.

Answer 9

How about use of ObsoleteAttribute and #pragma - hard to miss it then!

    void Test1()
    {
        _prop = ""; // warning given
    }
    public string Prop
    {
#pragma warning disable 0618
        get { return _prop; }
        set { _prop = value; }
#pragma warning restore 0618
    }
    [Obsolete("This is the backing field for lazy data; do not use!!")]
    private string _prop;
    void Test2()
    {
        _prop = ""; // warning given
    }

Answer 10

I would just put a large comment block on the top of the class that would look like that:

/************************************************************
* Note: When updating this class, please take care of using *
*       only the accessors to access member data because of *
*       ... (state the reasons / your name, so they can ask *
*       questions)                                          *
*************************************************************/

Usually, just a note like that should be enough, but if this is the same for all the classes in the project, you might prefer to put it in a simple document that you give to programmers working on the project, and everytime you see code that isn't conform, you point them to the document.

Answer 11

Currently, I'm in a similar situation. I have a field which should only be used by its property accessor.
I can't use automatic properties, since I need to perform additional logic when the property is set. (The property is not lazy loaded as well).

Wouldn't it be great if a next version of C# would allow something like this:

public class MyClass
{
    public int MyProperty
    {
        int _backingField;

        get
        {
            return _backingField;
        }
        set
        {
            if( _backingField != value )
            {
                _backingField = value;
                // Additional logic
                ...
            }
        }
    }
}

With such construct, the _backingField variable's scope is limited to the property. I would like to see a similar language construction in a next version of C# :)

But, I'm afraid this feature will never be implemented: http://connect.microsoft.com/VisualStudio/feedback/ViewFeedback.aspx?FeedbackID=381625

Answer 12

This might be overly simple, but why not abstract all the lazy to a base class

 public class LazyFoo{
      private string bar;
      public string Bar{
         get{
             // lazy load and return
         }
         set {
             // set
         }
      }
    }

    public class Foo : LazyFoo{
      // only access the public properties here
    }

I could see the argument that it is unnecessary abstraction, but it is the simplest way I can see to eliminate all access to backing fields.

Answer 13

This seems like trying to design-out mistakes that might not happen in the first place, and basically it's worrying about the wrong thing.

I would go with option 1 + comments:

///<summary>use Bar property instead</summary> 
string bar;

///<summary>Lazy gets the value of Bar and stores it in bar</summary>
string Bar {
    get {
        // logic to lazy load bar  
        return bar; 
    }
}

If you do get a developer who keeps using the backing variable then I'd worry about their technical competence.

By all means design to make your code easier to maintain, but try to keep it simple - any rule that you make for yourself here is going to be more hassle than it's worth.

And if you're still really worried about it create an FxCop (or whatever you're using) rule to check for this sort of thing.