Replace giant switch statement with what?

2019-01-23 13:26发布

问题:

I have a code that parses some template files and when it finds a placeholder, it replaces it with a value. Something like:

<html>
<head>
    <title>%title%</title>
</head>
<body bgcolor="%color%">
...etc.

In code, the parser finds those, calls this function:

string getContent(const string& name)
{
    if (name == "title")
        return page->getTitle();
    else if (name == "color")
        return getBodyColor();
    ...etc.
}

and then replaces the original placeholder with returned value.

In real case, it is not a dummy web page, and there are many (50+) different placeholders that can occur.

My code is C++, but I guess this problem exists with any language. It's more about algorithms and OO design I guess. Only important thing is that this must be compiled, even if I wanted I couldn't have any dynamic/eval'd code.

I though about implementing Chain of Responsibility pattern, but it doesn't seem it would improve the situation much.

UPDATE: and I'm also concerned about this comment in another thread. Should I care about it?

回答1:

Use a dictionary that maps tag names to a tag handler.



回答2:

You want replace conditional with polymorphism. Roughly:

string getContent(const string& name) {
    myType obj = factory.getObjForName(name);
    obj.doStuff();
}

where doStuff is overloaded.



回答3:

Have you considered XSLT? It's very well suited to this kind of thing. I developed a content management system that did the exact same thing and found XSLT to be very effective. The parser does a lot of the work for you.

UPDATE: Steven's comment raises an important point- you'll want your templates to be valid XHTML if you decide to go the XSLT route. Also- I would use a different delimiter for your replacement tokens. Something less likely to occur naturally. I used #!PLACEHOLDER#! in my CMS.



回答4:

i'll combine 3 ideas:

  1. (from Steven Hugig): use a factory method that gets you a different class for each selector.
    • (from Neil Butterworth): inside the factory, use a dictionary so you get rid of the big switch(){}.
    • (mine): add a setup() method to each handler class, that adds itself (or a new class instance) to the dictionary.

explaining a bit:

  • make an abstract class that has a static dict, and methods to register an instance with a selector string.
  • on each subclass the setup() method registers itself with the superclass' dict
  • the factory method is little more than a dictionary read


回答5:

Rather than parsing, have tried just reading the template into a string and then just performing replaces.

fileContents = fileContents.Replace("%title%", page->getTitle());
fileContents = fileContents.Replace("%color%", getBodyColor());


回答6:

As "Uncle" Bob Martin mentioned in a previous podacast with Joel and Jeff, pretty much anything you come up with is going to essentially be reproducing the big switch statement.

If you feel better implementing one of the solutions selected above, that's fine. It may make your code prettier, but under the covers, it's essentially equivalent.

The important thing is to ensure that there is only one instance of your big switch statement. Your switch statement or dictionary should determine which class handles this tag, and then subsequent determinations should be handled using polymorphism.