Reputation: 2964
I have a class called 'documentsection' which defines part of a document that later gets assembled in HTML. There are properties within it, and which values are required (or valid) depend on one another. For example:
a) There is an enumeration called 'section type' which can be 'view' (in this case, another property called 'content' contains the path/filename of a file to read in). Or it can be 'text' where the 'content' property text is itself placed in the document.
b) There is another enumeration called 'Action' which can be of type 'append', 'prepend' or 'replacebytag'. In the last case, another property becomes relevant, called 'tagtoreplace'. If we are appending/prepending, this tagtoreplace property can be empty.
What is the best practice way to represent such interdependencies? There are several ways I can think of, none of which reek with beauty:
When the method to 'generate the document' is called, go through the properties to ensure they conform to this logic.
Put checks in the get/set methods. One issue with this is that when I set my section type to 'view' I may not set my 'content' property until a line or so afterwards - so you can't reject the request to set it at that point.
Use separate properties somehow to partition the uses - eg 'content' in my first example above shouldn't be used for a filepath in one case and a bunch of HTML content in another. This doesn't smell right to me, but having separate properties for each seems excessive.
Inherit subclasses, each with different sets of the additional needed properties. Since there can be various combinations of section type and action type, I can't think of an elegant way to bake all this logic into such a structure. But I'm no OOP guru!
Any thoughts on the best approach?
Thanks!
Upvotes: 0
Views: 274
Reputation: 151588
I would go with the following approach:
interface ISection
{
void Render(); // or String Render() if you want to return a string
}
class ViewSection : ISection
{
public String Filename { get; set; }
public void Render()
{
// do stuff with Filename and/or return the content of the file
}
}
class TextSection : ISection
{
public String Text { get; set; }
public void Render()
{
// do stuff with Text and/or return it
}
}
class DocumentSection
{
ISection _section;
public void Render()
{
_section.Render();
}
}
You can then easily create new classes implementing ISection
that supply additional Sections. Same goes for the "actions", define an IAction
interface with a kind of Render
or PerformAction
method which you call.
Upvotes: 2
Reputation: 101130
I would separate HTML generation from the document generation itself. Have classes used to define a document and then have other classes used to generate each section of a document. By doing so you can easily add new formats later on (or more easily either test document building or html generation)
a) There is an enumeration called 'section type' which can be 'view' (in this case, another property called 'content' contains the path/filename of a file to read in). Or it can be 'text' where the 'content' property text is itself placed in the document.
Type properties are usually a design smell.
Have a Section
class which you derive for each type of section. Then just add the correct derived class.
b) There is another enumeration called 'Action' which can be of type 'append', 'prepend' or 'replacebytag'. In the last case, another property becomes relevant, called 'tagtoreplace'. If we are appending/prepending, this tagtoreplace property can be empty.
I'm not clear with how you use the action property.
Upvotes: 1
Reputation: 3830
If a property is meant for a filepath, then that's what it is meant for. If it is meant for content, that is what it is meant for. When one property can represent two different things, confusion and errors are bound to occur.
I think you need subclasses. Take a look at the abstract factory pattern as a way to manage the creation of the classes.
Upvotes: 0
Reputation: 107
Setting a property shouldn't have side effects on other properties.
If you are to do something like that you should use a method to set everything at once.
Upvotes: 1