>Refactoring XSL to Clean Code

>Well, at least less ugly and easier to extend code.

I’ve been ankle and waist deep at times in the XSL Tools content assistance. I’ve learned a lot about the way the WTP XML Content assist processor class is setup. I originally thought I’d take the easy way out and just Extend the XML Content Assist Processor to provide XSL content assistance where it was need. This actually became a mess of Override statements and copying of private methods so I could access them. The original code can be seen here. While the XML Content Assist Processor itself handles the vast majority of XML content assistance, it also is trying to do too much in one class. It makes it more brittle as the various pieces and parts are all interwoven, and if you change one portion, you could end up breaking it.

So, for XSL Tools 1.0M2, I ended up refactoring (notice I didn’t say rewriting), the way I was doing content assistance. The result can now be seen here. The code itself still needs some more refactoring to get it to a cleaner state, but it’s much easier to add content assistance now.

The key was in a utility class that WTP had, called the ContentAssistRequest class. XSL Tools has it’s own version of this which I have made abstract, and refactored a bit more. The purpose of the XSL Tools class and the WTP XML class is to provide the content assit proposals. How it is done is different after the refactoring. For XSL Tools I’ve promoted the ContentAssistRequest away from a utility class to an actual Abstract class that other proposals can extend. The type of content assist that should be provided is done using a Factory, XSLContentAssistRequestFactory.

What this now allows is for the XSL Content Assist Processor to just worry about getting the necessary information for the other classes to use. XSL Content AssistProcessor will delegate determining which proposals to be implemented and returned to the Factory.

So what this means is that instead of putting all of the Select/Test content assistance, Match content assistance, and any other type of XSL content assistance into the XSLContentAssistProcessor class, each of these separate process is it’s own specifc class that extends and implements the AbstractXSLContentAssistRequest class. The XSL Content Assistance just has the factory return the correct class, and then calls the standard IContentAssistRequest interface method getCompletionProposals() to return the actual proposals:

To invoke retrieving of the proposals, the XSL Content Assist Processor class executes this code:


XSLContentAssistRequestFactory requestFactory = new XSLContentAssistRequestFactory();

ICompletionProposal[] xslProposals = null;
IContentAssistProposalRequest contentAssistRequest = requestFactory
.getContentAssistRequest(textViewer, documentPosition, xmlNode,
sdRegion, completionRegion, matchString);

xslProposals = contentAssistRequest.getCompletionProposals();

The above is all that is needed to get the appropriate content assistance. I probably need to refactor this a bit more so that all of the parameters are passed into the constructor instead of the method.

The getContentAssistRequest() method is a standard factory instance method, containing the logic to select the appropriate instance of the IContentAssistProposalRequest to be returned (which is implemented mostly in the AbstractXSLContentAssistRequest class).

To actually implement a particular content assist request, such as Excluding Result Prefix proposals, we extend the AbstractXSLContentAssistRequest class, and implement the getCompletionProposals() abstract method:


@Override
public ICompletionProposal[] getCompletionProposals() {
proposals.clear();

IDOMAttr attrNode = (IDOMAttr)((IDOMElement)getNode()).getAttributeNode(EXCLUDE_RESULT_PREFIXES);
String excludeResultPrefixes = attrNode.getValue();
int offset = getCursorPosition();

if (excludeResultPrefixes == null || excludeResultPrefixes.equals(DEFAULT)) {
return getAllCompletionProposals();
}

tokens = excludeResultPrefixes.split("\\s"); //$NON-NLS-1$
if (tokens[0].equals("")) { //$NON-NLS-1$
CustomCompletionProposal proposal =
new CustomCompletionProposal(DEFAULT, offset, 0, DEFAULT.length(),
XSLPluginImageHelper.getInstance().getImage(XSLPluginImages.IMG_PREFIX),
DEFAULT, null, null, 0);
addProposal(proposal);
}

Collection namespaces = this.getNamespaces((IDOMElement)node);
for (NamespaceInfo namespace : namespaces) {
if (includePrefix(namespace)) {
CustomCompletionProposal proposal =
new CustomCompletionProposal(namespace.prefix, offset, 0, namespace.prefix.length(),
XSLPluginImageHelper.getInstance().getImage(XSLPluginImages.IMG_PREFIX),
namespace.prefix, null, namespace.uri, 0);
addProposal(proposal);
}
}
return getAllCompletionProposals();
}

This same pattern is used for all of the XSL specific content assistance. It allows for easier testing of the proposals:

Sample Unit Test


public void testAllDefaultValueNoProposals() throws Exception {
fileName = "TestResultPrefixes.xsl";
String xslFilePath = projectName + File.separator + fileName;
loadFileForTesting(xslFilePath);
IStructuredDocument document = (IStructuredDocument) sourceViewer.getDocument();
int column = 29;
int line = 2;

int offset = document.getLineOffset(line) + column;
System.out.println(document.get(document.getLineOffset(line), column));
ICompletionProposal[] proposals = getProposals(offset);
assertEquals("Found proposals when #all already in result value.", 0, proposals.length);
sourceViewer = null;
}

Also, now when new content assistance proposals are needed. The changes needed are isoloated and not intermingled with the other proposals. For the XSL Proposals that were implemented in XSLT 1.0M2, they are all isolated in the ElementContentAssistRequest class. That class extracted many of the methods that were buried in the XML Content Assist Processor class and moved them to an Element Content Assist request class.

Much of this still needs more refactoring to clean up some long methods and confusing names, but this portion of the refactoring has helped make the code a bit more maintainable. This refactoring could not have been done as easily as it was with out the unit tests being created. It made sure that the functionality was still working as designed, and if I broke anything, I fixed it first.

Anyways, I’d be interested in hearing of ways to refactor this further, and will gladly take any patches anybody wants to provide. Just open a bug in the XSL Tools incubator in the WTP Incubator project.

More information on the code syntax highlighter I used can be found here. This works with Blogger, I haven’t tried it with wordpress.

Advertisements
This entry was posted in eclipse, xml, xslt. Bookmark the permalink.

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s