views:

177

answers:

6

I have a class with a string property that's actually several strings joined with a separator.

I'm wondering if it is good form to have a proxy property like this:

public string ActualProperty
{
    get { return actualProperty; }
    set { actualProperty = value; }
}

public string[] IndividualStrings
{
    get { return ActualProperty.Split(.....); }
    set 
    { 
            // join strings from array in propval .... ;
            ActualProperty = propval;
    }
}

Is there any risks I have overlooked?

A: 

Well I'd say your "set" is high risk, what if somebody didn't know they had to pass an already joined sequence of values or your example above was maybe missing that. What if the string already contained the separator - you'd break.

I'm sure performance isn't great depending on how often this property is used.

typemismatch
+1  A: 

Define "good". It shouldn't break (unless you failed to properly guarantee that the delimiter(s) passed to Split() are never allowed in the individual strings themselves), but if IndividualStrings is accessed more frequently than ActualProperty you'll end up parsing actualProperty far more often than you should. Of course, if the reverse is true, then you're doing well... and if both are called so often that any unnecessary parsing or concatenation is unacceptable, then just store both and re-parse when the value changes.

Shog9
A: 

I'm not sure what the benefit of this design would be. I think the split would be better served in an extension method.

At a minimum, I'd remove the setter on the IndividualStrings property, or move it into two methods: string[] SplitActualProperty() and void MergeToActualProperty(string[] parts).

Will
+2  A: 

Linking two settable properties together is bad juju in my opinion. Switch to using explicit get / set methods instead of a property if this is really what you want. Code which has non-obvious side-effects will almost always bite you later on. Keep things simple and straightforward as much as possible.

Also, if you have a property which is a formatted string containing sub-strings, it looks like what you really want is a separate struct / class for that property rather than misusing a primitive type.

Wedge
+1  A: 

Seems that the array is the real data, and the single-string stuff is a convenience. That's fine, but I'd say look out for things like serialization and memberwise cloning, which will get and set both writeable properties.

I think I would;

  • keep the array as a property
  • provide a GetJoinedString(string seperator) method.
  • provide a SetStrings(string joined, string seperator) or Parse(string joined, string seperator) method.

Realistically, the seperator in the strings isn't really part of the class, but an ephemeral detail. Make references to it explicit, so that, say, a CSV application can pass a comma, where a tab-delimited app could pass a tab. It'll make your app easier to maintain. Also, it removes that nasty problem of having two getters and setters for the same actual data.

Steve Cooper
A: 

Properties are intended to be very simple members of a class; getting or setting the value of a property should be considered a trivial operation without significant side-effects.

If setting a property causes public values of the class other than the assigned property to change, this is more significant than a basic assignment and is probably no longer a good fit for the property.

A "complex" property is dangerous, because it breaks from the expectations of callers. Properties are interpreted as fields (with side-effects), but as fields you expect to be able to assign a value and then later retrieve that value. In this way, a caller should expect to be able to assign to multiple properties and retrieve their values again later.

In your example, I can't assign a value to both properties and retrieve them; one value will affect the other. This breaks a fundamental expectation of the property. If you create a method to assign values to both properties at the same time and make both properties read-only, it becomes much easier to understand where the values are set.

Additionally, as an aside:

It's generally considered bad practise to return a temporary array from a property. Arrays may be immutable, but their contents are not. This implies you can change a value within the array which will persist with the object.

For example:

YourClass i = new YourClass();
i.IndividualStrings[0] = "Hello temporary array!";

This code looks like it's changing a value in the IndividualStrings property, but actually the array is created by the property and is not assigned anywhere, so the array and the change will fall out of scope immediately.

public string ActualProperty { get; set; }

public string[] GetIndividualStrings()
{
    return ActualProperty.Split(.....);
}

public void SetFromIndividualStrings(string[] values)
{
    // join strings from array .... ;
}
Programming Hero