Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I am working on a small custom Markup script in Java that converts a Markdown/Wiki style markup into HTML.

The below works, but as I add more Markup I can see it becoming unwieldy and hard to maintain. Is there is a better, more elegant, way to do something similar?

private String processString(String t) {
    t = setBoldItal(t);
    t = setBold(t);
    t = setItal(t);
    t = setUnderline(t);
    t = setHeadings(t);
    t = setImages(t);
    t = setOutLinks(t);
    t = setLocalLink(t);

    return t;
}

And on top of it, passing in the string itself and setting it back to the same string just doesn't feel right. But, I just don't know of any other way to go about this.

share|improve this question
    
Could you provide implementation, at least some examples, of the different set methods you're calling? –  Simon André Forsberg Feb 3 at 19:29
    
it's not the same String, as Strings are immutable, so you don't have to worry about this part. You could name your return value something else, if that helps. –  njzk2 Feb 3 at 23:46

4 Answers 4

up vote 17 down vote accepted

You could create a StringProcessor interface:

public interface StringProcessor {

    String process(String input);
}


public class BoldProcessor implements StringProcessor {

    public String process(final String input) {
        ...
    }
}

and create a List from the available implementations:

final List<StringProcessor> processors = new ArrayList<StringProcessor>();
processors.add(new ItalicProcessor());
processors.add(new BoldProcessor());
...

and use it:

String result = input;  
for (final StringProcessor processor: processors) {
    result = processor.process(result);
}
return result;
share|improve this answer
4  
Great solution. I recommend encapsulating all this into a new object that contains a list of processors so that you can instantiate these objects from config files. Once you've done that you will easily be able to change your text attributes without recompiling. –  Bill K Feb 3 at 20:27
4  
Just a note, as this is the decorator pattern. You don't need BoldAndItalicProcessor, you just need BoldProcessor and ItalicProcessor. –  Silviu Burcea Feb 4 at 7:58
1  
@SilviuBurcea: Thank you, good point, I've updated the post. –  palacsint Feb 4 at 8:33
    
@SilviuBurcea Actually, that's a different tag in what I'm doing. It adds in <span class="boldclass italclass">stuff</span> when it finds ***stuff*** instead of just one, then the other. Which, I should really detect if something is text first then add in the respective css classes to the tag (and I will in the future). But for now, this works and it's an easier implementation. –  mburn7 Feb 4 at 20:19

If you want to process a language, even a simple one like a Wiki Markup, you should eventually write a proper parser, not do step-by-step replacement, nor chain a number of individual processors, no matter how fancy their implementation.

You can go with the fully generic approach, generate an AST from the markup (this would look similar to @rolfl's StyledString), and then use an AST serializer to create the end result (but for efficiency's sake, please append to a StringBuilder instead of repeatedly creating new strings). This allows you to use multiple serializers; e.g. if at one point you want to create PDF instead of HTML, this gives you a huge advantage. Your AST nodes should implement the visitor pattern for this purpose. (The serializer would be the visitor.)

But that would probably be overkill here. A simple parser that outputs the HTML as it parses would be simpler and probably sufficient.

You can use parser generators like ANTLR to generate the parser, or you can hand-write a parser.

share|improve this answer
    
This is something I will definitely keep in mind. Hopefully I will get a chance to do something like this. But, it's very much overkill at the moment because what I'm working on is still in the initial prototyping phase and I don't want to get too attached to it. Thank you for leading me in the proper direction. –  mburn7 Feb 3 at 19:19

I like @palacsint's approach but I just have one thing to add, you can probably do most of the processing with the same class.

public class TagProcessor implements StringProcessor {
    private final String wrapWith;
    public TagProcessor(String wrapWith) {
        this.wrapWith = wrapWith;
    }
    @Override
    public String process(String input) {
        return "<" + wrapWith + ">" + input + "</" + wrapWith + ">";
    }
}

processors.add(new TagProcessor("i"));
processors.add(new TagProcessor("b"));

I also believe that you can add generalize a lot of the functionality for other processors into a proper class and use it's constructor to send proper parameters. (Wrapping in <div class="someclass">...</div> for example).

share|improve this answer

This sounds like a case where you should encapsulate the data with a 'Decorator Pattern'.

You should declare a simple interface such as:

public interface StyledString {
    public String toFormatted();
    public StyledString getSource();
}

Then create a concrete class for each style you have:

public class BoldStyle implements StyledString {
    private final StyledString source;

    public BoldStyle(StyledString source) {
        this.source = source;
    }

    public String toFormatted() {
       return "<b>" + source.toFormatted() + "</b>";
    }

    public StyledString getSource() {
        return source;
    }

}

You should also have a 'NoStyle' class that takes a raw String input, and returns a null getSource();

using this system you can easily add Styles, and you can have styles that join phrases, etc.....

Also, you can add the styles together in a way that makes decomposing the value easier at a later point, and you only need to add/wrap the styles that you want.

share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.