locked
Use properties inside a class RRS feed

  • Question

  • Hello,

    The use of properties to access private fields from a class from outside this class is pretty straight forward and the preferred way of working.

    What I regularly have is that I have a private field that I would like to set in the constructor of the class but to which all other access (read and write) should be blocked. 

    public class A
    {
        private readonly int b;
        public B
        {
            get{ //return some form of b}
            set{ //use value to set b}
        }
    
        public A(int b)
        {
           this.B = b;
        }
    
        public int CalculateSomething(int c)
        {
            return b*c; //This is wrong, should be B*c
        }
    }

    The easy answer would be just use B everywhere. When writing the class that is no problem, but if you need to make changes to that piece of code a year later (or if it is someone else....) a mistake is easily made (in this case it can be a simple as a typo error).

    The only solution I can come up with is to put b in a separate class and create a readonly instance of that class in class A. But that's a lot of overhead (at best you'd need a class for every primitive type (int, bool,...) and one for object type which can be used with some casts).

    Isn't there a better solution?

    Friday, May 25, 2012 12:22 PM

Answers

  • Here's an example class that utilizes the class I defined above:

    public class Sample
    {
        public Sample()
        {
            SomeProperty = new MySpecialProperty<int>(0, GetSomeProperty, SetSomeProperty);
        }
    
        public MySpecialProperty<int> SomeProperty { get; private set; }
    
        /// <summary>
        /// This is whatever you would have had in the custom getter for your property
        /// </summary>
        public int GetSomeProperty(int n)
        {
            if (n > 0)
            {
                return n;
            }
            else
            {
                return -n;
            }
        }
    
        /// <summary>
        /// This is whatever you would have had in the custom setter for your property
        /// </summary>
        public int SetSomeProperty(int n)
        {
            FirePropertyChangedEvent();
            return n;
        }
    
        public void FirePropertyChangedEvent()
        {
            //TODO fire event
        }
    
        private void SomeMethod()
        {
            SomeProperty.Value = 15; //set the value
            Console.WriteLine(SomeProperty.Value); //get the value
        }
    }
    I'll let you play around with that for a bit.  Let me know if you want me to discuss any aspect of it in more detail.


    • Marked as answer by Mini2 Thursday, May 31, 2012 7:22 AM
    Tuesday, May 29, 2012 1:55 PM

All replies

  • I would use inheritance.

     
    private class BaseA
        {
            private int b;
            
            public int B
            {
                //custom logic will be here
                get { return b;}
                set { b = value; }
            }
        }
    
        public class A: BaseA
        {
            public A(int b)
            {
                this.B = b;
            }
    
            public int CalculateSomething(int c)
            {
                return B * c; //b is not accessible here
            }
        }


    Friday, May 25, 2012 12:41 PM
  • You can use Generics so that you don't need to write the class once for each type:

    public class MySpecialProperty<T>
    {
        private T _value;
        private Func<T, T> getter;
        private Func<T, T> setter;
        public MySpecialProperty(T initialValue, Func<T, T> getter, Func<T, T> setter)
        {
            _value = initialValue;
            this.getter = getter;
            this.setter = setter;
        }
    
        public T Value
        {
            get
            {
                return getter(_value);
            }
            set
            {
                _value = setter(value);
            }
        }
    }

    • Proposed as answer by servy42 Thursday, May 31, 2012 1:52 PM
    Friday, May 25, 2012 1:50 PM
  • Dmitry, if you did that you're limited by the fact that there is no multiple inheritance.  You couldn't have more than one property per class with that feature, and you couldn't inherit from any other class if you wanted to have a property like that.
    Friday, May 25, 2012 1:56 PM
  • I like this generic approach. It provides more flexibility.

    Three is one thing that was not mentioned in thread. We should not use any of proposed options for every class field. It will be complete misusing of Property pattern. Normally Property should not hide private field. Property should manage field visibility and perform basic value validations. Class member’s access to such normal fields should not be forbidden because it is the most efficient way to work with it. Set/Get/Add/Transform or any other necessary functions will make code much easier to support if complex logic is required. It that case we do not need to implement property or its part.

    Hiding private field will make sense for something like Stream.Position because you cannot just change that value without consequences bit for entity fields like FirstName, LastName that will add more confusion than value.

    What do you think?

    Friday, May 25, 2012 2:51 PM
  • I like this generic approach. It provides more flexibility.

    Three is one thing that was not mentioned in thread. We should not use any of proposed options for every class field. It will be complete misusing of Property pattern. Normally Property should not hide private field. Property should manage field visibility and perform basic value validations. Class member’s access to such normal fields should not be forbidden because it is the most efficient way to work with it. Set/Get/Add/Transform or any other necessary functions will make code much easier to support if complex logic is required. It that case we do not need to implement property or its part.

    Hiding private field will make sense for something like Stream.Position because you cannot just change that value without consequences bit for entity fields like FirstName, LastName that will add more confusion than value.

    What do you think?

    I think that there is very rarely ever a reason to bypass a property and access it's underlying value (outside of the property getters/setters).

    In most cases the getters/setters will be very simple, and generally be inlined, so the performance implications are often either none or very, very tiny.  There may be occasional apps where that very tiny bit matters, yes, but they will likely be quite rare.  There may also be times where you specifically don't want the additional code in the getter/setter to run (maybe you want to change a value internally during initialization without firing a bunch of property changed events) and so you are forced to access the underlying value.  For me though, I always say that the value should be accessed from the property (even inside of the class) unless there is some compelling reason not to.  That's why I use auto-implemented properties whenever I can; because I have no need for the underlying field.


    • Edited by servy42 Friday, May 25, 2012 3:02 PM
    Friday, May 25, 2012 3:01 PM
  • Do it this way:

    public class A
    {
      public B { get; private set; }
    }

    There is no private member which a user can erroneously use in place of the property.  Technically, there is - the compiler creates it for you and assigns it a special name that the parser would reject if you were to try to use it in your code.  This prevents the very common problem in which a property exists and so does a backing field and the authors do not consistently use the property instead of the field internally.

    The readonly keyword is generally unnecessary for private members.  You are trying to use a language construct to enforce your own rules inside your own class.  The readonly keyword should only be used to prevent others from violating your rules; almost always, the readonly keyword is used on public static fields to allow others to access your static field but not change it.

    One aspect here troubles me and that is that you're implying that after setting the value of property B, your accessor may return some other value - as you put it, "some form of b".

    This should always hold:

    myClass.MyProperty = someValue;
    var propertyValue = myClass.MyProperty;
    bool b = object.Equals(someValue, propertyValue);
    // b should be true

    If not, then you are misusing properties.

    Evan

    Friday, May 25, 2012 3:26 PM
  • Evan, the comments in the OPs code clearly imply that he's doing more than just getting/setting a variable, so auto-implemented properties wouldn't work.  Additionally it's important for the property to have a public setter.  The goal here is to have a property with custom get/set methods for which only the property itself can access the underlying field.  The reason for that last requirement is to prevent programmers from the class from accidentally using the field when they really should use the property (because they simply got the casing wrong).
    Friday, May 25, 2012 3:32 PM
  • A public setter is patently useless because a readonly field can only be initialized in a constructor.  If you attempted to set the value of a readonly field in the body of a set property, you would get a compiler error.

    The only way to prevent programmers from accidentally using a property's backing field is to eliminate the backing field.  The only way to do so is with auto-implemented properties.

    Evan

    Friday, May 25, 2012 5:44 PM
  • Here's an alternative:

        public static class Immutable
        {
            public static Immutable<T> Create<T>(T value)
            {
                return new Immutable<T>(value);
            }
        }
    
        public class Immutable<T>
        {
            public Immutable(T value)
            {
                _value = value;
            }
    
            private readonly T _value;
    
            public static implicit operator T(Immutable<T> immutable)
            {
                return immutable._value;
            }
    
        }
    
            private class MSDN
            {
                public Immutable<string> ReadonlyProperty { get; private set; }
    
                public MSDN(string value)
                {
                    ReadonlyProperty = Immutable.Create(value);
                }
            }
    
            [TestMethod]
            public void blah()
            {
                var msdn = new MSDN();
                string hello = msdn.ReadonlyProperty;
            }

    I should note that I strongly opposed to conversion operators on reference types; if I were implementing this, I would replace that operator with a get-only property called Value.  I showed a conversion operator here because if you're trying to retrofit a lot of existing code it will save you a lot of refactoring time because the change becomes non-breaking.

    If you need to publicly return a variant of the value in ReadonlyProperty, then make ReadonlyProperty private and create a public get-only property which executes whatever other code you need to do before returning the value.

    Evan


    Friday, May 25, 2012 5:56 PM
  • A public setter is patently useless because a readonly field can only be initialized in a constructor.  If you attempted to set the value of a readonly field in the body of a set property, you would get a compiler error.

    Which is why the code provided by the OP doesn't actually solve his problem. It's his own failed attempt at solving the problem, hence why he asked for help.

    The only way to prevent programmers from accidentally using a property's backing field is to eliminate the backing field.  The only way to do so is with auto-implemented properties.

    Well, your first sentence is contradicted by the second, as that's obviously one way of preventing programmers from accidentally using a backing field.  That said, it's not the only way.  See the answers provided by myself and Dmitry for two others.  These answers introduce additional problems, so they aren't bulletproof, but then again an auto-implemented property isn't meeting the requirement of having get/set methods more complex than directly getting/setting the backing field.
    Friday, May 25, 2012 5:57 PM
  • Thank you all for your insights on this, I totally agree with Dmitry that this is only for very special fields. Mostly this are fields whose value can only be used if other conditions are met e.g if b where a path name instead of an int, the function B could check if the path exist and if not create it or throw an exception or....).

    The generic solution seems a good idea, but I'm not really a generics specialist so could you explain about the constructor, as variables you pass the initialValue (which is clear) but also the Func<T, T> getters and setters which are delegates to some function (i guess) but where would these function be implemented (and how).


    Tuesday, May 29, 2012 9:11 AM
  • Here's an example class that utilizes the class I defined above:

    public class Sample
    {
        public Sample()
        {
            SomeProperty = new MySpecialProperty<int>(0, GetSomeProperty, SetSomeProperty);
        }
    
        public MySpecialProperty<int> SomeProperty { get; private set; }
    
        /// <summary>
        /// This is whatever you would have had in the custom getter for your property
        /// </summary>
        public int GetSomeProperty(int n)
        {
            if (n > 0)
            {
                return n;
            }
            else
            {
                return -n;
            }
        }
    
        /// <summary>
        /// This is whatever you would have had in the custom setter for your property
        /// </summary>
        public int SetSomeProperty(int n)
        {
            FirePropertyChangedEvent();
            return n;
        }
    
        public void FirePropertyChangedEvent()
        {
            //TODO fire event
        }
    
        private void SomeMethod()
        {
            SomeProperty.Value = 15; //set the value
            Console.WriteLine(SomeProperty.Value); //get the value
        }
    }
    I'll let you play around with that for a bit.  Let me know if you want me to discuss any aspect of it in more detail.


    • Marked as answer by Mini2 Thursday, May 31, 2012 7:22 AM
    Tuesday, May 29, 2012 1:55 PM