1

I have some dto object which I want to sent over the wire using wcf. In this simple case it has FirstName, LastName and Name. Name returns firstname and lastname in conjuction.

Normally I would'nt use settter for Name property but I have to since otherwise it cannot be serialized, so I tried with internal but I'm getting error on Name setter

An unhandled exception of type 'System.StackOverflowException' occurred in LibSys.WebServices.dll

[DataMember]
public string FirstName { get; set; }

[DataMember]
public string LastName { get; set; }

[DataMember]
public string Name  {
   get { return string.Format("{0} {1}", FirstName, LastName); }
         internal set { Name = value; }
}
Improve this question
1
  • In fact, Name should not be serialized at all. You already serialize LastName and FirstName. When you deserialize your object and call Name property it will calculate its value from those 2 properties. Commented Dec 4, 2013 at 7:04

3 Answers 3

Reset to default
6

Your setter calls itself recursively. If the value of Name is calculated from the other properties, you should probably omit the setter entirely. Since it's a calculated entirely from other properties, you probably don't need it to be serialized either.

If having a setter is an absolute must, you can just create an empty setter like this:

public string Name  {
   get { return string.Format("{0} {1}", FirstName, LastName); }
   internal set { }
}
Improve this answer
Sign up to request clarification or add additional context in comments.

5 Comments

letting the setter empty is not really good as just make the Name get-only.
@KingKing In original question property is DataMember.
@KingKing This is supposed to be a readonly property anyway, right? And what would it matter if it's not callable from user code (without reflection)? I agree it's bad practice, though. Just offering an alternative.
Yes, It is better to throw InvalidOpertaionException or NotSupportedException than empty setter. User will feel value set successfully, but really not.
@SriramSakthivel I don't see that as an issue. The setter is internal. A user would not be able to set the value without using reflection. If they are circumventing the public interface and fiddling around with non-public methods, why should it matter what they expect to happen?
5

The bug is the setter of Name. It unconditionally calls the setter of Name and hence will result in a stack overflow.

If Name is a combination of FirstName and LastName then you should split the value in the setter and assign the two values to FirstName and LastName respectively

public string Name  {
   get { return string.Format("{0} {1}", FirstName, LastName); }
   internal set 
   {
      var values = value.Split(new char[] { ' ' });
      FirstName = values[0];
      LastName = values[1];
   }
}

That's a pretty rough example, before checking in I'd add some error checking to ensure the string actually had a space in it.

Really though I question why this property needs a setter at all. It's really a calculated property. As such it should be responsible for displaying values only. Setting the values should be done directly on the properties from which the value is calculated.

Improve this answer

5 Comments

Too much work in setter. Also I would not expect IndexOutOfRangeException to be thrown in setter. Omitting setter is just fine.
@Leri i agree that it's a calculated property and probably shouldn't be serialized at all. But if it must have a setter then this is the best way to implement it
Well, being marked with DataMemberAttribute means property must have setter. However, it may be remained empty.
@Leri that seems much worse. Having a setter which does nothing would be extremely confusing to a consumer of the type. The setter should either do work or not be present
I agree. Since properties that construct Name property are serialized there's no need of extra traffic and on the receiving end it's pretty trivial to recalculate property value.
0

The problem is here:

public string Name  {
   get { return string.Format("{0} {1}", FirstName, LastName); }
         // you're setting Name to value, that's recursive, you should add another variable.
         internal set { Name = value; }
}
Improve this answer

Comments

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.