set is throwing StackOverflowException C#

Question

I am trying to get and send a list of this object to a text file. The text file is in the following format.

name,IDnumber,department,value

there are quite a few lines of this so i used a for to read them in. This is the code for the read and write to the file.

    public List<Employee> ReadFile(string fileName) {
        StreamReader fileIn = new StreamReader(fileName);
        fileIn = File.OpenText(fileName);
        List<Employee> list = new List<Employee>();
        string[] test;
        string name;
        string ID;
        string dep;
        string post;

        while (!fileIn.EndOfStream || !File.Exists(fileName)) {

            string inString = fileIn.ReadLine();
            test = inString.Split('#');
            name = test[0];
            ID = test[1];
            dep = test[2];
            post = test[3];
            Employee newEmp = new Employee(name, ID, dep, post);
            list.Add(newEmp);

        }
        fileIn.Close();
        return list;
    }

    public void WriteFile(List<Employee> outList, string file) {

        StreamWriter writeOut = new StreamWriter(file);

        for (int i = 0; i < outList.Count; i++) {

            writeOut.WriteLine(outList[i].name + '#' + outList[i].IDnum + '#' + outList[i].department + '#' + outList[i].position);

        }
        writeOut.close();
    }

This is the code for my class. The error is being thrown at the set.

public class Employee {

    public string name { get { return name; } set { name = value; } }
    public string IDnum { get { return IDnum; } set { IDnum = value; } }
    public string department { get { return department; } set { department = value; } }
    public string position { get { return position; } set { position = value; } }

    public Employee() {

        name = string.Empty;
        IDnum = string.Empty;
        department = string.Empty;
        position = string.Empty;

    }

    public Employee(string newName, string newID) {

        name = newName;
        IDnum = newID;
        department = string.Empty;
        position = string.Empty;

    }

    public Employee(string newName, string newID, string newDep, string
    newPost) {

        name = newName;
        IDnum = newID;
        department = newPost;
        position = newDep;

    }

}

I am not sure if there is some kind of formatting that I am missing for the set function to function as needed. The This is the function i am calling for the in and out of the file. I believe that it is never making it to the out so it is likely how i am importing the data.

score 5 · Accepted Answer · edited May 23 '17 at 10:26

It's a really common gotcha... a C# rite of passage!

Let's take a look at a single property (this applies to all of your properties though)...

public string name { get { return name; } set { name = value; } }

so what happens when you try myObj.name = "foo";?

In the set method, you refer back to the very same property name. So it tries to access name, which goes around again (and again, and again, recursively until you StackOverflow).

A backing field with proper naming conventions is the norm here:

private string name;
public string Name{get { return name; } set{ name = value; }}

or even better, if there's no logic involved, an auto-property.

public string Name{ get; set; }

score 2 · Answer 2 · answered Jan 27 '15 at 01:33

2

You keep calling IDnum and other properties over and over recursively, until the stack overflows

public string IDnum { get { return IDnum; } set { IDnum = value; } }

When you do something like

IDnum = someValue;

that calls the setter for IDnum, which runs the code in the setter

IDnum = value

Which in turn calls the setter of IDnum, until you run out of stack.

The Fix

In your case, it looks like you can use automatic properties

public string IDnum { get; set; }

answered Jan 27 '15 at 01:33

Eric J.

147,927
63
340
553

So, how should the syntax be? should it just be `get {return IDnum;` and `set{ IDnum = value;` – Mason Toy Jan 27 '15 at 01:35
++, and the other properties have the same problem. – Mike Dunlavey Jan 27 '15 at 01:35
Thank you very much! I was having trouble with the automatic before but looks like i corrected my error causing that. – Mason Toy Jan 27 '15 at 01:45
Glad you worked it out. If you did not want to use automatic properties, you would have to setup a variable to store the property value (called the "backing store", using a different name e.g. `private string idnum;`. – Eric J. Jan 27 '15 at 01:50

Viktors Garkavijs · Answer 3 · 2015-01-27T02:06:11.030

You should change

public string name { get { return name; } set { name = value; } }
public string IDnum { get { return IDnum; } set { IDnum = value; } }
public string department { get { return department; } set { department = value; } }
public string position { get { return position; } set { position = value; } }

to

public string name { get; set; }
public string IDnum { get; set; }
public string department { get; set; }
public string position { get; set; }

or introduce backing fields:

private string _name;
public string name { get { return _name; } set { _name = value; } }

See https://msdn.microsoft.com/en-us/library/bb384054.aspx for more info on Auto-Implemented Properties in C#.

Please note, that the commonly used naming of public properties is PascalCasing. Your properties in PascalCasing would look like this:

public string Name { get; set; }
public string IdNum { get; set; }
public string Department { get; set; }
public string Position { get; set; }

set is throwing StackOverflowException C#

3 Answers3