CODEWITHSUNDEEP
c#properties
ios85
ios85

Reputation: 2134

Using a private variable in a public property getter

If I have a public property that only has a getter, is it correct to use a private variable to assign the value to then return that value or should I just use return rather then setting the value of userID? In the future we plan to add more functionality that will use the userID field, and Methods will be added to this class that use the userID field. Is there any benefit to one way or the other? Is there some other way that this should be done?

    private string userID;
    public string ID
    {
        get
        {
            if (System.Web.HttpContext.Current.Request.Headers.AllKeys.Contains("UID"))
            {
                userID = System.Web.HttpContext.Current.Request.Headers["UID"].ToString();
            }
            else
            {
                userID = "0000";
            }
            return userID;
        }
    }

Upvotes: 1

Views: 246

Answers (5)

Piotr Stapp
Piotr Stapp

Reputation: 19828

Because you are using public property, you can change implementation anytime. No userID variable is useless, if you will need it in the future you can always add it.

Upvotes: 0

Tigran
Tigran

Reputation: 62265

You can do this, but I personally would go, in this case, for a function.

The reason is simple: One when calls a function, expects that some computation may occur inside of it.

Instead when one calls a property, it's not something that he would expect to happen. When you call a property, you think you call some internal field wrapper.

Repeat, what you do is possible to do, but it's not so good from architectual point of view.

So, my suggession, for this case: use a function.

Upvotes: 2

Sergey Kalinichenko
Sergey Kalinichenko

Reputation: 727047

The way the getter is coded right now does not need an assignment, because subsequent calls ignore the value set by previous methods. However, you could cache the result, like this:

private string userID;
public string ID {
    get {
        if (userID == null) {
            if (System.Web.HttpContext.Current.Request.Headers.AllKeys.Contains("UID")) {
                userID = System.Web.HttpContext.Current.Request.Headers["UID"].ToString();
            } else {
                userID = "0000";
            }
        }
        return userID;
    }
}

This implementation avoids reading the "UID" repeatedly by caching the result of the initial retrieval in a private instance variable.

Upvotes: 2

Matthew
Matthew

Reputation: 25793

In this instance, you are not persisting any data, therefore having a backing field of userID is pointless.

Upvotes: 0

Sam Leach
Sam Leach

Reputation: 12966

Use return

You are setting userID in the get

So

public string ID
    {
        get
        {
            if (System.Web.HttpContext.Current.Request.Headers.AllKeys.Contains("UID"))
            {
                 return System.Web.HttpContext.Current.Request.Headers["UID"].ToString();
            }
            else
            {
                return "0000";
            }

        }
    }

Upvotes: 0

Related Questions