c# set/get problems

Started by
9 comments, last by Ravyne 14 years, 3 months ago
Hi, a quick question, suppose we have a code like that

    public class Vector2
    {
        private float _x;
        private float _y;
        public float X
        {
            set
            {
                //ToDo
                _x = value;
            }
            get
            {
                return _x;
            }
        }
        public float Y
        {
            set
            {
                //ToDo
                _y = value;
            }
            get
            {
                return _y;
            }
        }
    };

    public class Node
    {
        private Vector2 _position = new Vector2;
        private Vector2 _scale = new Vector2;
        public Vector2 Position
        {
            set
            {
                OnChangePosition();
                _position = value;
            }
            get
            {
                return _position;
            }
        }
        public Vector2 Scale
        {
            set
            {
                OnChangeScale();
                _scale = value;
            }
            get
            {
                return _scale;
            }
        }
        public void OnChangePosition()
        {
            //....
        }
        public void OnChangeScale()
        {
            //....
        }
    }
    public class Test
    {
        Node node = new Node();
        public Test()
        {
            Vector2 v = new Vector2();
            v.X = 0.0f;
            v.Y = 1.0f;

            node.Position.X = 2.0f;      //OnChangePosition isn't called (WRONG)
            node.Position = v;           //OnChangePosition is called (OK)

            node.Scale.X = 2.0f;      //OnChangeScale isn't called (WRONG)
            node.Scale = v;           //OnChangeScale is called (OK)
        }
    }


The problem is that when I'm changing Position or Scale through X accessor, Position.set or Scale.set isn't called due to obvious reasons and thus OnChangePosition and OnChangeScale also aren't called, my question would be what would be the best way to invoke Position/Scale set accessor from Vector2 accessors? Delegates, Events? or maybe there is a better way, I am still green in C#, so maybe I am missing something here. Thanks
Advertisement
Quote:Original post by Tct
Hi, a quick question, suppose we have a code like that

*** Source Snippet Removed ***

The problem is that when I'm changing Position or Scale through X accessor, Position.set or Scale.set isn't called due to obvious reasons and thus OnChangePosition and OnChangeScale also aren't called, my question would be what would be the best way to invoke Position/Scale set accessor from Vector2 accessors?

Delegates, Events? or maybe there is a better way, I am still green in C#, so maybe I am missing something here.

Thanks


maybe:
    public class Vector2    {        private float _x;        private float _y;        public float X        {            set            {                //ToDo               OnChangePosition();               OnChangeScale();                _x = value;            }            get            {                return _x;            }        }        public float Y        {            set            {                //ToDo               OnChangePosition();               OnChangeScale();                 _y = value;            }            get            {                return _y;            }        }    };


I dunno...but the only thing I see that is apparent about why OnPositionChange/OnScaleChange isn't firing is because you aren't calling it when an X or Y value changes? I'd probably use a function with a return value instead. Also, there may be some logic that you can implement that will determine if one or both of those events need to be triggered.

Hope that helps some...
In addition:
Think about when you want the actual assignment (before or after the event raisers)?

[Edited by - ernow on December 30, 2009 2:59:19 AM]
If you implement your vector as a value type instead (which makes more sense when you think about it), you won't be able to modify the return value of the property and will avoid the problem entirely.
Mike Popoloski | Journal | SlimDX
ddboarm> Thx for answer, but I think you've misunterstood what was the problem, if I had more than those two functions, it would be a bit unwise to write them all in Vector2.

ernow> Sorry for mistake, ye I wanted that callback to be called after setting the value

Mike.Popoloski> I've picked Vector2 as reference type because I actually wanted to change X, Y separably,I am passing Vector2 to PropertyGrid which allows such modifications.

Anyways, I've solved the problem by passing delegate to Vector2, so whenever X,Y is changed the appropriate function is called.. I guess that's the only way to do it.

Thanks for the answers
    public class Vector2    {        private float _x;        private float _y;        public float X        {            set            {                //ToDo                _x = value;            }            get            {                return _x;            }        }        public float Y        {            set            {                //ToDo                _y = value;            }            get            {                return _y;            }        }    };    public class Node    {        private Vector2 _position = new Vector2;        private Vector2 _scale = new Vector2;        public Vector2 Position        {            set            {                OnChangePosition();                _position = value;            }            get            {                return _position;            }        }        public Vector2 Scale        {            set            {                OnChangeScale();                _scale = value;            }            get            {                return _scale;            }        }        public void OnChangePosition()        {            //....        }        public void OnChangeScale()        {            //....        }    }    public class Test    {        Node node = new Node();        public Test()        {            Vector2 v = new Vector2();            v.X = 0.0f;            v.Y = 1.0f;            node.Position.X = 2.0f;      //OnChangePosition isn't called (WRONG)            node.Position = v;           //OnChangePosition is called (OK)            node.Scale.X = 2.0f;      //OnChangeScale isn't called (WRONG)            node.Scale = v;           //OnChangeScale is called (OK)        }    }



The real problem is that you are exposing implementation details of your Node class through its public API. Because the Node API allows access to the Vector2 directly, Node loses its measure of control over mutations of its components. You could refactor Node such that this isn't a problem, like so;

    public class Vector2    {        private float _x;        private float _y;        public float X        {            set            {                //ToDo                _x = value;            }            get            {                return _x;            }        }        public float Y        {            set            {                //ToDo                _y = value;            }            get            {                return _y;            }        }    };    public class Node    {        private Vector2 _position = new Vector2;        private Vector2 _scale = new Vector2;        public Vector2 Position        {            //set            //{            //    OnChangePosition();            //    _position = value;            //}            get            {                return _position;            }        }        public void SetPosition (Vector2 position)        {            SetPosition (position.X, position.Y);        }        public void SetPosition (float x, float y)        {            OnChangeScale();            _position.X = x;            _position.Y = y;        }        public Vector2 Scale        {            //set            //{            //    OnChangeScale();            //    _scale = value;            //}            get            {                return _scale;            }        }        public void SetScale (Vector2 scale)        {            SetScale (scale.X, scale.Y);        }        public void SetScale (float x, float y)        {            OnChangeScale();            _scale.X = x;            _scale.Y = y;        }        public void OnChangePosition()        {            //....        }        public void OnChangeScale()        {            //....        }    }    public class Test    {        Node node = new Node();        public Test()        {            Vector2 v = new Vector2();            v.X = 0.0f;            v.Y = 1.0f;            node.SetPosition(2.0f, 0f);  //OnChangePosition is called            node.SetPosition(v);           //OnChangePosition is called             node.SetScale( 2.0f, 1f);   //OnChangeScale is called            node.SetScale (v);           //OnChangeScale is called         }    }



Now everything works as expected with no need for an extra layer of binding with callbacks. Always let the API talk on its own terms, do not let it be determined by the its composite parts.
There's no difference between a 'set' property and your SetPosition method; both achieve the same thing but yours in a less intuitive manner. Besides, since he still exposes the 'get' and the Vector2 is a reference he can easily just do 'node.Position.X = 5' and it will work, completely sidestepping any callbacks.

My suggestion would be to make Vector2 a struct instead of a class such that you can't use the object given by the 'get' accessor to modify the underlying data. You should be just fine there as the PropertyGrid does work with modifying value type properties, but you could choose to add an X and Y property to your Node class for directly modifying single components of the position.
Quote:Original post by NickGravelyn
There's no difference between a 'set' property and your SetPosition method; both achieve the same thing but yours in a less intuitive manner.


I'm sorry my friend, but this is not necessarily true (although the difference is one of semantics and API discoverability rather than in implementation terms). The set property directly exposes his implementation, allowing for the exact problem he has now (that he effectively has public access to his types internals, and can manipulate their public API).

The void SetFoo methods do not expose the internals; he may be setting elements in a vector, as in my example, or setting elements in a 4 element float array where 0,1 are position, 2,3 are scale, or something else entirely. The point is, by making a logical seperation of interface and implementation his class has fine grained control over the mutation of its members, allowing him to trigger events, etc when he so chooses.

He can certainly apply your changes, and it would work, but I disagree that it is a general solution to the problem of exposing implementation. Consider a case similar case where we want to encapsulate all the enemies in a game level in a EnemyContainer class, and we want to inform observers of when a new enemy is added or removed from the level;

    class EnemyContainer    {        List<Enemy> m_enemies;        event Action<Enemy> EnemyAdded;        event Action<Enemy> EnemyRemoved;        public void AddEnemy(Enemy enemy)        {            m_enemies.Add(enemy);            InvokeEnemyAdded(enemy);        }        public void RemoveEnemy(Enemy enemy)        {            if (m_enemies.Remove(enemy))            {                InvokeEnemyRemoved(enemy);            }        }        public IEnumerable<Enemy> Enemies         {            get { return new List<Enemy>(m_enemies); }        }                private void InvokeEnemyAdded(Enemy enemy)        {            var enemyAdded = EnemyAdded;            if (enemyAdded != null)            {                enemyAdded(enemy);            }                }        private void InvokeEnemyRemoved(Enemy enemy)        {            var enemyRemoved = EnemyRemoved;            if (enemyRemoved != null)            {                enemyRemoved(enemy);            }        }    }


FWIW I am aware of the observable collection class, but assume for a moment we could not use this. In the above circumstance, it would be crazy to expose the list through a public accessor, circumventing all of the containers control over the mutation of its data and the triggering of the events on mutation. That is what you are advocating, and as a solution to the problems that this creates you are advocating creating a value type implementation of the List<> class. Do you agree that this seems like a backward solution? What if we need to switch to a Dictionary<Guid, Enemy>? Do we just add a Guid ID argument to the class methods, and replace the internal implementation with a dictionary, or do we do a wholesale reimplementation of Dictionary<K,V> as a struct?

Quote:Original post by NickGravelyn
Besides, since he still exposes the 'get' and the Vector2 is a reference he can easily just do 'node.Position.X = 5' and it will work, completely sidestepping any callbacks.


This is true, my apologies. To circumvent this you could return a new Vector2 which is a copy of your internal one, as in my example.
Quote:Original post by return0
Quote:Original post by NickGravelynThere's no difference between a 'set' property and your SetPosition method; both achieve the same thing but yours in a less intuitive manner.


I'm sorry my friend, but this is not necessarily true (although the difference is one of semantics and API discoverability rather than in implementation terms).
I respect that not all examples will be true, but in your example it is the same and your example was the one I was directly addressing.

However a 'set' property accessor also doesn't have to directly expose the implementation. A 'set' accessor is just a method that looks (syntactically) like you're setting a value directly. You could easily have a class like this:

class Foo{   private Matrix m;   public Vector3 Position   {      get { return m.Translation; }      set { m.Translation = value; }   }}


Or something even more weird:

class Foo{   private string innerValue;   public int Value   {      get { return int.Parse(innerValue); }      set { innerValue = value.ToString(); }   }}


Suffice to say a property is just as much of hiding implementation details as your method is. The only difference between the two is what syntax a user of your API will see when trying to modify the object.

Quote:Original post by return0
He can certainly apply your changes, and it would work, but I disagree that it is a general solution to the problem of exposing implementation.
I disagree that any solution is the be-all-end-all solution of exposing implementation. For something like a simple Vector2, my changes are all that's needed; adding more methods doesn't gain anything. In your list example, those methods clearly are the superior solution and I wouldn't argue against it. There isn't a best fit solution for every situation; sometimes you use one solution and others require a different solution.
Quote:Original post by NickGravelyn
Quote:Original post by return0
Quote:Original post by NickGravelynThere's no difference between a 'set' property and your SetPosition method; both achieve the same thing but yours in a less intuitive manner.


I'm sorry my friend, but this is not necessarily true (although the difference is one of semantics and API discoverability rather than in implementation terms).
I respect that not all examples will be true, but in your example it is the same and your example was the one I was directly addressing.

However a 'set' property accessor also doesn't have to directly expose the implementation. A 'set' accessor is just a method that looks (syntactically) like you're setting a value directly. You could easily have a class like this:

*** Source Snippet Removed ***

Or something even more weird:

*** Source Snippet Removed ***

Suffice to say a property is just as much of hiding implementation details as your method is. The only difference between the two is what syntax a user of your API will see when trying to modify the object.



I absolutely agree, and am aware that properties simply compile to void setFoo(T value) and T getFoo () methods. The relevant question is why then do they exist? It would be glib to say that they are absolutely identical to their equivalent methods.

Perhaps it is a matter of personal taste, but I prefer to stick to a couple of rules for properties in languages that support them; no work, other than copying, caching or event invocation, and as little as possible that would be tricky to support if the implementation of the class changed significantly (i.e., the interface of the class is speaking in terms of the level of abstraction of the class, not of its composite data structures). In this case a Vector is fine I guess, which is why I left it in in my first (incorrect) example; I would correct it by returning a new Vector, a copy of the class's data.

This topic is closed to new replies.

Advertisement