This is dangerous
-
Poking around in one of the UI components in our tool library, implementing a feature that our content developers wanted. Before checking in, I like to make sure I do a search for any TODO items I may have added.
When I searched this component I found 21 instances of, "TODO: This is dangerous, but we need something that will allow the properties to not be readonly."
Huh.
-
Isn't setting the property's access to public or protected sufficient?
-
A major thing that requires properties to be read-write is .Net's XML serialization, which works by first instantiating the object with its default constructor and then setting its public fields and properties. I despise this.
It's made all worse by the fact that they actually did the right thing elsewhere: For the "real" serialization, they provided initialization via a "deserializing constructor" that receives a data dictionary in argument.
Edit: Thinking back of it, it does have an advantage: It encourages separating the "business logic" classes from the "storage" classes, making the serialized format less likely to be "broken" by a change in the business logic. So it may actually be a good thing.
-
@Medinoc said:
Thinking back of it, it does have an advantage: It encourages separating the "business logic" classes from the "storage" classes, making the serialized format less likely to be "broken" by a change in the business logic. So it may actually be a good thing.
It also makes it easier to bypass the business layer, because the storage classes are fully public, so it's still a net disadvantage.
-
@pjt33 said:
@Medinoc said:
Thinking back of it, it does have an advantage: It encourages separating the "business logic" classes from the "storage" classes, making the serialized format less likely to be "broken" by a change in the business logic. So it may actually be a good thing.
It also makes it easier to bypass the business layer, because the storage classes are fully public, so it's still anet disadvantagetrade off that you have to evaluate.FTFY
-
What's odd about it is that the majority of these comments exist in functions like the following:
virtual bool CanConvertTo(ITypeDescriptorContext ^context, Type ^destinationType) override
{
// TODO: This is dangerous, but we need something that will allow the properties to not be readonly.
return true;
}In fact, there's each data type has a class named <typename>Convertor (StringConvertor, IntConvertor, Vector2Convertor, etc) that contains two functions named, "CanConvertFrom" and "CanConvertTo" that do the exact same thing: they return true. The other two functions simply convert whatever value between it's native and string form using "ToString" and "Parse".
What this has to do with property read-only settings, I haven't a clue. Maybe I just need to look into it further.
-
@pjt33 said:
@Medinoc said:
I couldn't resist being punny this morningThinking back of it, it does have an advantage: It encourages separating the "business logic" classes from the "storage" classes, making the serialized format less likely to be "broken" by a change in the business logic. So it may actually be a good thing.
It also makes it easier to bypass the business layer, because the storage classes are fully public, so it's still a .Net disadvantage.
