CODEWITHSUNDEEP
c#asp.netgetter-settergetter
Tom Gullen
Tom Gullen

Reputation: 61755

Stackoverflow error C# with getter and setter

This is the working class:

namespace Lite
{
    public class Spec
    {
        public int ID { get; set; }

        public string Name { get; set; }
        public string FriendlyName { get; set; }
        public int CategoryID { get; set; }
        public int Width { get; set; }
        public int Height { get; set; }
        public string UOM { get; set; }
        public int Pagination { get; set; }
        public int ColoursFront { get; set; }
        public int ColoursBack { get; set; }
        public string Material { get; set; }
        public int GSM { get; set; }
        public string GSMUOM { get; set; }
        public bool Seal { get; set; }

        public Spec(int ID)
        {
            using (CrystalCommon.MainContext db = new CrystalCommon.MainContext())
            {
                var q = (from c in db.tblSpecifications where c.id == ID select c).SingleOrDefault();
                if (q != null)
                    loadByRec(q);
            }
        }

        public Spec(CrystalCommon.tblSpecification Rec)
        {
            loadByRec(Rec);
        }

        public void loadByRec(CrystalCommon.tblSpecification Rec)
        {
            this.ID = Rec.id;
            this.Name = Rec.Title;
            this.Width = Convert.ToInt32(Rec.FinishedSizeW.Value);
            this.Height = Convert.ToInt32(Rec.FinishedSizeL.Value);
            this.UOM = Rec.FlatSizeUOM;
            this.Pagination = Rec.TxtPagination.Value;
            this.ColoursFront = Convert.ToInt32(Rec.TxtColsF.Value);
            this.ColoursBack = Convert.ToInt32(Rec.TxtColsB.Value);
            this.Material = Rec.TxtMaterial;
            this.GSM = Rec.TxtGSM.Value;
            this.GSMUOM = Rec.txtGsmUnit;
            this.Seal = Rec.TxtSeal.Value == 1;
        }

        public string displayDimensions()
        {
            return Width + " x " + Height + " " + UOM;
        }
    }
}

Then I try and modify the Name getter and setter:

namespace Lite
{
    public class Spec
    {
        public int ID { get; set; }

        // User friendly name if available otherwise fall back on spec name
        public string Name { get {
            if (null != FriendlyName)
                return FriendlyName;
            else
                return Name;
            }
            set
            {
                Name = value;
            }
        }
        public string FriendlyName { get; set; }
        public int CategoryID { get; set; }
        public int Width { get; set; }
        public int Height { get; set; }
        public string UOM { get; set; }
        public int Pagination { get; set; }
        public int ColoursFront { get; set; }
        public int ColoursBack { get; set; }
        public string Material { get; set; }
        public int GSM { get; set; }
        public string GSMUOM { get; set; }
        public bool Seal { get; set; }

        public Spec(int ID)
        {
            using (CrystalCommon.MainContext db = new CrystalCommon.MainContext())
            {
                var q = (from c in db.tblSpecifications where c.id == ID select c).SingleOrDefault();
                if (q != null)
                    loadByRec(q);
            }
        }

        public Spec(CrystalCommon.tblSpecification Rec)
        {
            loadByRec(Rec);
        }

        public void loadByRec(CrystalCommon.tblSpecification Rec)
        {
            this.ID = Rec.id;
            this.Name = Rec.Title;
            this.Width = Convert.ToInt32(Rec.FinishedSizeW.Value);
            this.Height = Convert.ToInt32(Rec.FinishedSizeL.Value);
            this.UOM = Rec.FlatSizeUOM;
            this.Pagination = Rec.TxtPagination.Value;
            this.ColoursFront = Convert.ToInt32(Rec.TxtColsF.Value);
            this.ColoursBack = Convert.ToInt32(Rec.TxtColsB.Value);
            this.Material = Rec.TxtMaterial;
            this.GSM = Rec.TxtGSM.Value;
            this.GSMUOM = Rec.txtGsmUnit;
            this.Seal = Rec.TxtSeal.Value == 1;
        }

        public string displayDimensions()
        {
            return Width + " x " + Height + " " + UOM;
        }
    }
}

On my computer this compiles fine, but the server seems to crash when it runs. (First version works fine). My colleague compiled it on his machine and it threw a "Stack overflow error" apparently, but he's not around for me to get specifics on that right now.

Am I applying the getter correctly here?

Upvotes: 1

Views: 3010

Answers (8)

Adam Houldsworth
Adam Houldsworth

Reputation: 64517

Your set is referencing the property itself, and your get is referencing the property itself, both of these will cause a potentially endless loop leading to a StackOverflowException (no more stack space to push the current call into). You need to use a backing field:

private string _name;

public string Name 
{ 
    get 
    {
        if (null != FriendlyName)
            return FriendlyName;
        else
            return _name;
    }
    set
    {
        _name = value;
    }
}

It looks as though you tried to turn an auto-property into a manual one. Auto-properties (public string Name { get; set; }) work because the compiler will create the backing field itself.

As a learning exercise, if you step through with the debugger and step into return Name or Name = value you will see first hand the code going back into the property you are already in.

Upvotes: 5

codeandcloud
codeandcloud

Reputation: 55248

This is much better.

string _name = "";
public string Name
{
    get { return FriendlyName ?? _name; }
    set { _name = value; }
}

Upvotes: 2

Nathanial Woolls
Nathanial Woolls

Reputation: 5291

You have a getter for Name, that calls the property Name, which will call the getter for Name, etc. You need a private field to back the property, and you need to access that backing field in your getter instead.

Upvotes: 1

thekip
thekip

Reputation: 3768

No you should use a backing field. The error is in the else

        public string Name { get {
        if (null != FriendlyName)
            return FriendlyName;
        else
            return Name;//error, you're calling the property getter again.
        }

Upvotes: 0

detaylor
detaylor

Reputation: 7280

If FriendlyName is null then the Name getter attempts to get the value from the Name getter - i.e. it loops. This is what causes the stack overflow.

Upvotes: 0

Daniel A. White
Daniel A. White

Reputation: 190976

public string Name { get {
            if (null != FriendlyName)
                return FriendlyName;
            else
                return Name;
            }
            set
            {
                Name = value;
            }
        }

Name in the get/set refers to the property. You will need to define a backing field and use that.

Upvotes: 0

Tim Lloyd
Tim Lloyd

Reputation: 38464

One of your properties gets and sets itself, see:

   public string Name
   {
       get {
        if (null != FriendlyName)
            return FriendlyName;
        else
            return Name; //<-- StackOverflow
        }
        set
        {
            Name = value; //<-- StackOverflow
        }
    }

Upvotes: 1

&#216;yvind Br&#229;then
&#216;yvind Br&#229;then

Reputation: 60714

This is an endless loop:

public string Name { get {
  ...
  set
  {
    Name = value;
  }
}

The setter will call itself repeatedly until you get the Stack overflow exception.

usually you have a backing variable, so it ends up like this

private string name;
public string Name { 
  get {
    if (null != FriendlyName)
      return FriendlyName;
    else
      return name;
    }
  set {
    name = value;
  }
}

Upvotes: 11

Related Questions