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 a net 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:
    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.
    I couldn't resist being punny this morning


Log in to reply
 

Looks like your connection to What the Daily WTF? was lost, please wait while we try to reconnect.