I am getting into infinite loop in property setter

Question

public int Position
{
    get
    {
        if (Session["Position"] != null)
        {
            Position = Convert.ToInt32(Session["Position"]);
        }
        else
        {
            Position = 5;
        }
        return Position;
    }
    set
    {
        Position = value;
    }
}

my program calls the get and goes into if loop and then runs infitely into set code

@Knaģis has the corrected code, but to further explain why: Think of these getters and setters as functions because that's what they are behind the scenes. If a function calls itself within, without a means to exit, you'll be in an infinite loop. — bland, Apr 22 '13 at 19:31

score 16 · Accepted Answer · answered Apr 22 '13 at 19:13

16

The error is because in your set {} you are invoking the same setter recursively.

Correct code would be

private int _position;
public int Position
{
    get
    {
        if (Session["Position"] != null)
        {
            this._position = Convert.ToInt32(Session["Position"]);
        }
        else
        {
            this._position = 5;
        }
        return this._position;
    }
    set
    {
        this._position = value;
    }
}

answered Apr 22 '13 at 19:13

Knaģis

20,827
7
66
80

3

You don't really need the underscore and `this` at the same time. Most folks will discern that the underscore means that you are referring to a local member, and there's nothing that needs to be disambiguated. – Robert Harvey Apr 22 '13 at 19:16
The underscore is really just a matter of habit for me (and many others), but in my opinion (and also StyleCop http://stackoverflow.com/questions/1562540/why-does-stylecop-recommend-prefixing-method-or-property-calls-with-this) `this` should be mandatory always. – Knaģis Apr 22 '13 at 19:18
2

Yeah, StyleCop's wrong. :) And that Stack Overflow question has [dissenting opinions](http://stackoverflow.com/a/1562571/102937). – Robert Harvey Apr 22 '13 at 19:19

score 5 · Answer 2 · answered Apr 22 '13 at 19:13

Use a member variable or perhaps store it in the session.

private int _position;
public int Position
{
    get
    {
        if (Session["Position"] != null)
        {
            _position= Convert.ToInt32(Session["Position"]);
        }
        else
        {
            _position= 5;
        }
        return _position;
    }
    set
    {
        _position = value;
    }
}

Nicholas Carey · Answer 3 · 2013-04-22T20:09:09.413

2

There's nothing particularly string-like about session state items.

Why wouldn't you just follow the KISS principle and do something like

public int Position
{
  get { return (int) ( Session["Position"] ?? 5 ) ; }
  set { Session["Position"] = value ;               }
}

or (depending on your actual requirements/specs:

public int Position
{
  get { return Session["Pointer"] as int? ?? position ?? 5 ; }
  set { position = value ; }
}
private int? position ; // backing store

edited Apr 22 '13 at 20:09

answered Apr 22 '13 at 20:01

Nicholas Carey

71,308
16
93
135

as `int`? `int` is value type, how can it be `as`? – Ken Kin Apr 22 '13 at 21:36
@KenKin: `int?` is syntactic sugar for `Nullable`, a reference type (and special, to boot). A straight `int` will get boxed in the session store; an `int?` is for all intents and purposes, already boxed. See the spec, ISO 23270 § 14.9.11: *The as operator is used to explicitly convert a value to a given reference type or nullable type...When converting to a nullable type, the as operator uses a wrapping conversion, an unboxing conversion, or a null type conversion (§13.7.1)...if the indicated conversion is not possible, the resulting value is null.* – Nicholas Carey Apr 22 '13 at 21:46

Ken Kin · Answer 4 · 2013-04-22T22:12:02.100

An auto-implemented property property consists of a getter, a setter and a backing field. If you write the code yourself, a field might not be necessary.

Your getter invokes setter, and the setter invokes setter; that would be infinite recursion. You might need a field for storing Position.

However, if we change it with storing to a field, and the setter in fact doesn't effect. So, the code could be changed to:

public int Position {
    set {
    }

    get {
        int x;
        return (x=Convert.ToInt32(Session["Position"]))>0?x:5;
    }
}

You don't need to check for null, Convert.ToInt32(null) is zero.

I am getting into infinite loop in property setter

4 Answers4

Linked

Related