tags:

views:

110

answers:

5

Hi.

This is just a matter of taste but I'd like to hear some of your opinions (that's also why this question is marked as subjective).

If I have a property, say

private string _Text;
public string Text;
get
{
   object tmp = ViewState["Text"];
   if (tmp != null)
      _Text = Convert.ToString(tmp);
   return _Text;
}
set
{
   ViewState.Add("Text", value);
}

Now this is the property which may be specified by the programmer, by setting some custom text. This is then mapped - say - to some control on the UI. In the default case however, the Text of the control comes from a predefined resource file. So internally to handle that internally in a better way, I'd have some central point where I check whether the user has specified the "Text" property (above) and if so, use that data, otherwise rely on the default one from the resource file.

So what approach would you take? I have two options in mind:

private string ResolvedText
{
   get
   {
      if(!string.IsNullOrEmpty(Text))
         return Text;
      else
         //return the one from the resource file
   }
}

Or put everything in a method

public string GetResolvedText()
{
   if(!string.IsNullOrEmpty(Text))
      return Text;
   else
      //return the one from the resource file   
}

The question may sound stupid to you since it's really a minor difference. But I'd like to know whether there are some conventions about this.

Thx

+6  A: 

Personally, I'd take the body of your GetResolvedText method, and use it in the property, thus:

private string _Text;
public string Text
{
   get
   {
     if(string.IsNullOrEmpty(_Text))
        //return the one from the resource file  
     else
        return _Text;
   }

   set
   {
      _Text = value;
   }
}

This puts all the responsibility for managing the string into the one place. The class itself can access _Text internally, if it needs the raw value.

Steve Gilham
This is the whole point of properties, exactly how they are intended to be used.
Martin Doms
Exactly!! :-) +1
Cerebrus
That would also be an option. But the logic inside my Text property is a bit more complicated (retrieving it from the Viewstate etc..). That's why I kept them separated. I'll update my code so that it is more clear.
Juri
The `//return the one from the resource file` line is at the very least hiding a method call to the resources APIs; in your more complex case, that clause should probably be to a private method that wraps whatever logic goes with your resource call.
Steve Gilham
A: 

To me, if the getter can't throw an exception, or null/invalid value, it should be a property. It is what properties are made for.

BUT, if you do some complicated stuff, if has to be a a function getter. Obviously here you have only 1 if, so I would use a property.

Clement Herreman
+2  A: 

I would keep this as a property, since it represents a single value that does not require a lot of computing to retrieve.

Fredrik Mörk
A: 

Reworking together your example and Steve's answer, plus adding in some caching, as obviously the resource value should be read only once since it never changes and by contract we must return the value from the property as fast as possible:

private static string ResourceText;

static [constructor]
{
    ResourceText = //get resource;
}

private string text;
public string Text;
get
{
   string tmp = (string)ViewState["Text"];
   if (!String.IsNullOrEmpty(tmp))
      text = tmp;
   else
      text = ResourceText;
   return text;
}
set
{
   ViewState.Add("Text", value);
   // Note: passing null or empty strings will not work.
}
Sklivvz
+1  A: 

I find that the best general rule here is: if calling the action twice results in multiple resource calls or different behaviour - use a method.

So, in your example use of a property is fine if it caches:

public string ResolvedText
{
   get { return Text ?? (Text = GetResolvedText()); }
}

However the method doesn't need to - users expect it to be a more intensive operation:

public string GetResolvedText()
{
   //return the one from the resource file   
}

The design question is how do you want this class to be used?

A property will get called as if it is a 'cheap' operation:

if( myInstance.ResolvedText != null && 
    myInstance.ResolvedText.Length > 5 )
    Response.Write( myInstance.ResolvedText );

A method hints to the developer that they should call it as few times as possible:

string resolvedText = myInstance.GetResolvedText();

if( resolvedText != null && 
    resolvedText.Length > 5 )
    Response.Write( resolvedText );

Personally I prefer to keep interim classes simple, so in the vast majority of cases I would use the method model.

As this is a fairly standard convention you should avoid properties that don't cache and methods that do.

Keith
Actually it isn't such a matter because it will just be used internally, therefore also the "private" property. However I was interested in the kind of thinking you're writing here. (+1)
Juri