What are the cons of some code like this:
public class Class1
{
public object Property1
{
set
{
// Check some invariant , and null
// throw exceptions if not satisfied
// do some more complex logic
//return value
}
}
}
Properties are expected be cheap to execute. Code looks tidier if the value of the property doesn't need to be held in a variable to avoid expensive property code execution.
The same goes for setting them, its not expected that assigning a value will invoke some expensive operation.
The con would be that if you are doing excessive logic that takes a great period of time, and/or has side effects which don't logically flow from the setting of the property, you are going to have a pretty convoluted class design.
Methods are traditionally used to indicate that an action with consequence is being performed, not properties. While you definitely can have logic in them, having too much might give the wrong impression of what you are trying to do, which is assign a value, as opposed to perform an operation.
In the end, it all depends on what "do some more complex logic" means.
For a code to be understandable, it's better if you can get exactly what you have just set.
Except for "some complex logic", which we cannot see for now, this code doesn't look like a big mess.
It's hard to answer the question since "complex logic" is very vague.
I see no issues with basic sanity checking on setting of a property and it's very common to need to do some sort of processing on an object when one of its properties changes.
Now, if by "complex logic" you mean do things like some crazy database access or creating a network connection to go do something then yes, that would be bad.
If you see a property, you assume that it behaves just like a field, i.e., that you can retrieve its value over and over again:
if(obj.Prop.Equals(otherObj.Prop))
{
Console.WriteLine(obj.Prop);
Log.CreateEntry(obj.Prop);
}
Were Prop implemented as a method. The users suspects expensive computation and will probably copy the result into a local variable and work on it instead.
A property must never change the object. A property should always return the same object if it's value hasn't changed. Especially if the creation of the object is expensive (like returning a new StreamReader on every call)
Properties should strive to be side effect free. IF the side effect is invisible to consumers of the class (a lazy creation of a string which then doesn't change) then this is much less of an issue. By default debuggers tend to display (thus evaluate) properties. If they have side effects this can cause considerable grief.
Throwing from a setter if the supplied value is illegal is generally okay. Throwing from a getter is generally considered poor.
These are, as ever, rules of thumb and circumstances can dictate that they are not followed. A good example is the properties on the types generated via Linq to SQL. If you have lazy lookup of child properties then evaluating it does indeed trigger a database read. This is offset but the ease of use, readability and consistency of the resulting objects - it's one of those balancing competing rules things.
Be wary of rules that say MUST or NEVER. Sometimes rules of this nature exist, and are reasonable, but they are actually uncommon.
For clarity of code business logic should be separate from the property setters. Uncomplicated sanity checks can and should often take place in the property. But anything that is application specific, or not immediately implicit in the type should be abstracted away. For best OO practices You would want any logic that applies only to that object to be part of that object, and any complicated logic that relates to how you DEAL with that object should be in the part of the code that deals with handling the object.