CODEWITHSUNDEEP
c#
arjun kr
arjun kr

Reputation: 993

What I am doing wrong with below "Set" property?

I was trying to create simple event and below is the block of code but its not working. When I try to debug, as soon as we create Point object it is throwing "StackOverFlowException" at "Set" property(even before we assign the value p.x=10). What I am doing wrong? 

using System;

namespace Workshop
{

    public class Point
    {
        public int x
        {
            get { return x; }
            set
            {
                x = value;
                onPointeChanged();
            }
        }

        public int y
        {
            get { return y; }
            set
            {
                y = value;
                onPointeChanged();
            }
        }

        public event EventHandler pointchanged;

        private void onPointeChanged()
        {
            if (pointchanged != null)
                pointchanged(this, EventArgs.Empty);
        }

    }

    public class Program
    {

        public static void Main(String[] args)
        {
            Point p = new Point();
            p.pointchanged += HandleEvent;
            p.x = 10;
        }

        public static void HandleEvent(object obj, EventArgs sender)
        {
            Console.WriteLine("Event Raised");
        }

    }
}

Thanks

Upvotes: 1

Views: 209

Answers (3)

rory.ap
rory.ap

Reputation: 35318

The problem is you are assigning the property a value in its own setter: 

public int x
{
    get { return x; }
    set
    {
        x = value; // <-- see you are assigning `value` to your `x` property.
        onPointeChanged();
    }
}

This will loop over and over again ad infinitum. You need to create a backing field:

private int _myField;

public int BetterNameThanX
{
    get { return _myField; }
    set
    {
        _myField = value;
        onPointeChanged();
    }
}

Upvotes: 3

V0ldek
V0ldek

Reputation: 10623

You are calling the set method indefinitelly until you run out of stack memory. What you're doing with

x = value;

is you're calling the x property's setter, which in turn does x = value, so it calls itself, and so on, and so on for all eternity.

To fix this, introduce a field:

private int x;

public int X
{
    get => x;
    set
    {
         x = value;
         OnPointChanged();
    }
}

This is the proper way of creating properties with custom logic behind get and/or set. If you didn't have your OnPointChanged() logic you could just do

public int X { get; set; }

which would generate the following code for you under the hood:

private int x;
public int X { get => x; set => x = value; }

Upvotes: 5

Felipe Oriani
Felipe Oriani

Reputation: 38638

You have defined properties and you return every own property which causes recursivity on get and set scopes. Try to define private attributes and expose it by properties:

public class Point
{
    private int _x;
    public int x
    {
        get { return _x; }
        set
        {
            _x = value;
            onPointeChanged();
        }
    }

    private int _y;
    public int y
    {
        get { return _y; }
        set
        {
            _y = value;
            onPointeChanged();
        }
    }

    public event EventHandler pointchanged;

    private void onPointeChanged()
    {
        if (pointchanged != null)
            pointchanged(this, EventArgs.Empty);
    }

}

Upvotes: 1

Related Questions