>Refactoring getMatchString, refactored

>Robert has correctly pointed out, that there was now a chance for a Null Pointer Exception to occur with aRegion. Also, I noticed that there was a possibility that a Null parent could cause a Null Pointer Exception to occur as well. None of the unit tests I had created had caught this scenario, so it’s always good to have a code review done. With a little more refactoring the getMatchString section now looks like this:


private String getMatchString(IStructuredDocumentRegion parent,
ITextRegion aRegion, int offset) {
String matchString = "";

if (isNotMatchStringRegion(parent, aRegion, offset)) {
return matchString;
}

if (hasMatchString(parent, aRegion, offset)) {
matchString = extractMatchString(parent, aRegion, offset);
}
return matchString;
}

private boolean isNotMatchStringRegion(IStructuredDocumentRegion parent, ITextRegion aRegion, int offset) {
if (aRegion == null || parent == null)
return true;

String regionType = aRegion.getType();
int totalRegionOffset = parent.getStartOffset(aRegion) + aRegion.getTextLength();
return (isCloseRegion(aRegion) || hasNoMatchString(offset, regionType, totalRegionOffset));
}

private boolean isCloseRegion(ITextRegion region) {
String type = region.getType();
return ((type == DOMRegionContext.XML_PI_CLOSE)
|| (type == DOMRegionContext.XML_TAG_CLOSE)
|| (type == DOMRegionContext.XML_EMPTY_TAG_CLOSE)
|| (type == DOMRegionContext.XML_CDATA_CLOSE)
|| (type == DOMRegionContext.XML_COMMENT_CLOSE)
|| (type == DOMRegionContext.XML_ATTLIST_DECL_CLOSE)
|| (type == DOMRegionContext.XML_ELEMENT_DECL_CLOSE)
|| (type == DOMRegionContext.XML_DOCTYPE_DECLARATION_CLOSE)
|| (type == DOMRegionContext.XML_DECLARATION_CLOSE));
}

private boolean hasMatchString(IStructuredDocumentRegion parent, ITextRegion aRegion, int offset) {
return (parent.getText(aRegion).length() > 0) && (parent.getStartOffset(aRegion) < offset);
}

private boolean hasNoMatchString(int offset, String regionType, int totalRegionOffset) {
return regionType == DOMRegionContext.XML_CONTENT
|| regionType == DOMRegionContext.XML_TAG_ATTRIBUTE_EQUALS
|| regionType == DOMRegionContext.XML_TAG_OPEN
|| offset > totalRegionOffset;
}

private String extractMatchString(IStructuredDocumentRegion parent, ITextRegion aRegion, int offset) {
String matchString = parent.getText(aRegion).substring(0, offset - parent.getStartOffset(aRegion));
if (matchString.startsWith("\"")) {
matchString = matchString.substring(1);
}
return matchString;
}

Again, thanks to Robert for pointing the error out.

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

2 Responses to >Refactoring getMatchString, refactored

  1. >Ooh, I like how this looks now. It’s much easier to understand.Eclipse code can often be really tough to read, I’m glad to see you making this effort.

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