views:

184

answers:

5

Hi,

I've run into a really interesting runtime bug which generates a rogue stack overflow.

I've defined a structure as follows:

public enum EnumDataType { Raspberry, Orange, Pear, Apple };

public class DataRequest
{
    public long DataSize 
    { 
        get { return 0; } 
        set { DataSize = value; } 
    }

    public EnumDataType DataType  
    { 
        get { return EnumDataType.Apple; } 
        set { DataType = value; } 
    }
}

The following lines work perfectly:

DataRequest request = new DataRequest();
request.DataSize = 60;

However, when I step over the following line in code, it generates a stack overflow:

request.DataType = EnumDataType.Raspberry;

Of course, I can fix it by removing the defaults, or using auto get/set, but I need it to be both readable and writable, and return a default - any ideas?

+19  A: 
public long DataSize { get { return 0; } set { DataSize = value; } } 

You are constantly setting the value of DataSize. You need to create a local variable and use that instead. e.g.

private long dataSize;

public long DataSize
{
    get { return this.dataSize; }
    set { this.dataSize = value; }
}

EDIT I've written DataSize but the same applies to DataType

Ardman
+8 but no explanation of why the stack overflow occurs?
John Nicholas
-1: This will still *always* return 0 instead of just making 0 the *default* value. Admittedly the question isn't clear, but I don't think it's what was meant.
Jon Skeet
@John Nicholas: That is because the program is trying to set DataSize with DataSize, that in turn wants to set it's DataSize with DataSize, which will set that one's DataSize with DataSize. And then it will set it's DataSize with DataSize, after that it will set the DataSize with DataSize… Whew, mindblowing; and I'm not even half-way there!
Spoike
I know why ;) the origional asker didn't though ;)
John Nicholas
Thanks - great answer. I understand exactly whats going on now. We're not dealing with properties, we're dealing with methods, as its an object.
Gravitas
you are dealing with properties ... but your properties were self referencing so they called themselves over and over (hence the stack overflow) ... properties are methods in a way (I suppose in msil they are the same) the difference is that because you canassign to them they are easier to use in expressions. They also carry the implication/expectation of not being expensive unlike a method which could use all kinds of resources and take any amount of time to complete.
John Nicholas
+1  A: 

you have to implemet it with a backing store:

private EnumDataType dataType;
public EnumDataType DataType  { get { return EnumDataType.Apple; } set { dataType = value; } }

}

You should do so anytime you do some acion in the getter and setters. By the way, why can you even set the variables? you can't read them out, you always get EnumDataType.Apple. If you want a start value, you can do like this:

private EnumDataType dataType = EnumDataType.Apple;
public EnumDataType
{
   get
   {
       return dataType;
   }
   set
   {
       dataType = value;
   }
 }
Tokk
That won't compile - EnumDataType is a non-nullable value type.
Jon Skeet
+8  A: 

As others have said, the stack overflow occurs because your property setter is just calling itself. It may be simpler to understand if you think of it as a method:

// This obviously recurses until it blows up
public void SetDataType(long value)
{
    SetDataType(value);
}

As I understand it, you're trying to create normal properties but with a default value, right?

In that case, you need backing variables which are set by the setters - and the getters should return those variables, too. It's the variables which should get default values:

private long dataSize = 0;
public long DataSize {
  get { return dataSize; }
  set { dataSize = value; }
}

private EnumDataType dataType = EnumDataType.Apple;
public EnumDataType DataType { 
  get { return dataType; }
  set { dataType = value; }
}

Alternatively, use automatic properties but set the defaults in your constructor:

public long DataSize { get; set; }
public EnumDataType DataType { get; set; }

public DataRequest()
{
    DataSize = 0; // Not really required; default anyway
    DataType = EnumDataType.Apple;
}
Jon Skeet
Ardman beat you Skeet... Times-a-changin'
Neurofluxation
Thanks - great answer. I understand exactly whats going on now. We're not dealing with properties, we're dealing with methods, as its an object. The reason behind my original question was how to set the defaults that are returned, and your answer covers this. p.s. Enjoyed your talk in London at the start of this year - your presentation was genuinely interesting, and not just because of the sock puppets :)
Gravitas
A: 

I don't understand how the first line: request.DataSize = 60;

Doesn't cause a stack overflow - my advice would be to use backing properties:

public class DataRequest
{
    protected int dataSize = 0;
    protected EnumDataType enumDataType;
    public long DataSize { get { return 0; } set { dataSize = value; } }
    public EnumDataType DataType  { get { return EnumDataType.Apple; } set { enumDataType = value;} 
}
Dave
+1  A: 

stack overflow happens because in the setter you are setting the property to a value (Ie you are trying to get something to set itself to something ... which causes an infinite loop) ... which means that it tries to set itself to a value which means its tries to set itself to a value untill boom

your properties will never get the values you are setting because they always return the same value (not the stored value)

public enum EnumDataType { Raspberry, Orange, Pear, Apple }; 

public class DataRequest 
{ 
private long _dataSize = 0;
private EnumDataType _dataType = EnumDataType.Apple;

public long DataSize { get { return _dataSize ; } set { _dataSixe= value; } } 
public EnumDataType DataType  { get { return _dataType; } set { _dataType= value; } } 
} 

is what you really wanted

John Nicholas