C# Stack overflow exception when trying to Set in Get

Question

So I'm trying to be smart about this but it looks like this idea is quite dumb on account of I'm getting a stack overflow exception right off the bat.

Ideally, I would like for CheckInternetStatus to run whenever I use the IsConnected flag.

How can I accomplish this correctly? Or is this just not a good idea at all?

public static class Internet
{
    public static bool IsConnected
    {
        get
        {
            CheckInternetStatus();
            return IsConnected;
        }
        set
        {
            IsConnected = value;
        }
    }
}

Additional Information: CheckInternetStatus updates IsConnected

Mitch Wheat · Accepted Answer · 2014-05-14 08:26:31Z

5

There are two things wrong with your code:

1) No backing variable, you are recursively calling property IsConnected:

private bool _isConnected;

public static bool IsConnected
{
    get
    {
        _isConnected = CheckInternetStatus();
        return _isConnected;
    }
    set
    {
        _isConnected = value;
    }
}

2) You should not do 'lots of work' in a property. Use a method instead. This better conveys intent to someone reading your code

Actually 3 things:

3) If you stick with using a property (I wouldn't), the setter should probably be private. You don't want connected to be set if you are not actually connected.

edited May 14, 2014 at 8:26

answered Feb 16, 2013 at 1:39

Mitch Wheat

302k44 gold badges482 silver badges552 bronze badges

Sign up to request clarification or add additional context in comments.

1 Comment

Kashif Over a year ago

thank you for your answer. I voted for yours as it is the most concise and in-depth.

Keith Nicholas · Accepted Answer · 2013-02-16 01:38:28Z

4

think you want

private static bool _isConnected;
public static bool IsConnected
    {
        get
        {
            CheckInternetStatus();
            return _isConnected;
        }
        set
        {
            _isConnected= value;
        }
    }

because you are recursively calling into your property

answered Feb 16, 2013 at 1:38

Keith Nicholas

44.4k15 gold badges101 silver badges168 bronze badges

Comments

Tim M. · Accepted Answer · 2013-02-16 01:56:41Z

1

This is a case where a backing field can be extremely useful.

private static bool _isConnected;

public static bool IsConnected {
    get {
        CheckInternetStatus();
        return _isConnected;
    }
    set {
        _isConnected= value;
    }
}

Alternatives

Additional Information: CheckInternetStatus updates IsConnected

CheckInternetStatus should manipulate the private field. Futhermore, the set should be private (or possibly not exist). But IsConnected seems to have a somewhat unexpected side-effect (the act of connecting). Instead, what about:

public static bool IsConnected {
    get;
    private set;
}

public static void Connect(){
    if( IsConnected ){
        // exit, or fail if this is considered an exceptional scenario
    }

    // do work
    IsConnected = true;
}

// call
if( !Foo.IsConnected ){
    Foo.Connect();
}

This seems cleaner and more obvious. Properties (as a general rule) shouldn't perform large amounts of work.

edited Feb 16, 2013 at 1:56

answered Feb 16, 2013 at 1:38

Tim M.

54.5k14 gold badges126 silver badges167 bronze badges

1 Comment

Andrew Barber Over a year ago

Just remember that CheckInternetStatus() should be setting _isConnected rather than IsConnected, too.

Jason · Accepted Answer · 2013-02-16 01:39:01Z

0

You're infinitely recursing.

return IsConnected;

calls your getter, which calls your getter again and again, infinitely, until you crash.

answered Feb 16, 2013 at 1:39

Jason

89.5k15 gold badges137 silver badges151 bronze badges

Comments

Osa E · Accepted Answer · 2013-02-16 01:39:53Z

0

You are doing this wrong. You need some instance variable. You are calling yourself. Correction looks like:

public static class Internet
{
private static bool _isConnected;

public static bool IsConnected
{
    get
    {
        CheckInternetStatus();
        return _isConnected;
    }
    set
    {
        _isConnected = value;
    }
}

}

answered Feb 16, 2013 at 1:39

Osa E

1,7911 gold badge15 silver badges26 bronze badges

Comments

Pyromancer · Accepted Answer · 2013-02-16 01:49:17Z

0

I think you can also use like this,

       private static bool _isConnected;

        public static bool isConnected
        {
            get { CheckInternetStatus(); return _isConnected; }
            set { isConnected = value; }
        }

answered Feb 16, 2013 at 1:49

Pyromancer

2,5395 gold badges22 silver badges28 bronze badges

3 Comments

Tim M. Over a year ago

How is this any different than the half-dozen answers already stating the same thing?

Pyromancer Over a year ago

oops im sorry, i forgot to refresh the page, so I didn't notice that there are already answers, sorry ,

Tim M. Over a year ago

No problem. You should see a bar across the page saying "new answers to this question", which you can click to see new answers and save answering something that has already been answered.

Collectives™ on Stack Overflow

C# Stack overflow exception when trying to Set in Get

6 Answers 6

1 Comment

Comments

Alternatives

1 Comment

Comments

Comments

3 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

6 Answers 6

1 Comment

Comments

Alternatives

1 Comment

Comments

Comments

3 Comments

Your Answer

Sign up or log in

Post as a guest

Related