Android XML parser to parse themes

Question

I used XML to control the theme of an InputMethod for Android. A theme in XML contains the color and the shape of keys.

XML file

<theme> <color name="FIRST_LINE_LETTER_COLOR" value="#263238"></color> <color name="SECOND_LINE_LETTER_COLOR" value="#263238"></color> <color name="THIRD_LINE_LETTER_COLOR" value="#263238"></color> <color name="PIN_KEY_COLOR" value="#263238"></color> <color name="PIN_KEY_PRESSED_COLOR" value="#80CBC4"></color> <shape name="PIN_KEY_SHAPE" value="circle"></shape> </theme> 

ThemeXmlParser.java

import android.content.Context; import android.graphics.Color; import org.xml.sax.Attributes; import org.xml.sax.InputSource; import org.xml.sax.SAXException; import org.xml.sax.XMLReader; import org.xml.sax.helpers.DefaultHandler; import javax.xml.parsers.ParserConfigurationException; import javax.xml.parsers.SAXParser; import javax.xml.parsers.SAXParserFactory; import java.io.IOException; import java.util.HashMap; import java.util.Map; public class ThemeXmlParser { private Context context; private static Map<String, Integer> colorMap; private static Map<String, String> configMap; private ThemeXmlParser(Context ctx) { context = ctx; colorMap = new HashMap<String, Integer>(); configMap = new HashMap<String, String>(); } public static ThemeXmlParser newInstance(Context ctx) { return new ThemeXmlParser(ctx); } private String selectAssetsPath(String scheme) { String path = ""; if (scheme.equals(XMLStr.Theme.google_dark)) { path = "themes/google_dark_theme.xml"; } else { //todo add other skins } return path; } public void parse(String scheme) { String assetsPath = selectAssetsPath(scheme); InputSource is = null; try { is = new InputSource(context.getAssets().open(assetsPath)); } catch (IOException e) { e.printStackTrace(); } if (is != null) { doParse(is); } } public static Map<String, Integer> colorMap() { return colorMap; } public static Map<String, String> configMap() { return configMap; } private void doParse(InputSource is) { XMLReader xmlReader = createXMLReader(); ColorHandler colorHandler = new ColorHandler(); xmlReader.setContentHandler(colorHandler); try { xmlReader.parse(is); } catch (SAXException e) { e.printStackTrace(); } catch (IOException e) { e.printStackTrace(); } } private XMLReader createXMLReader() { XMLReader reader = null; SAXParser saxParser = createSAXParser(); try { reader = saxParser.getXMLReader(); } catch (SAXException e) { e.printStackTrace(); } return reader; } private SAXParser createSAXParser() { SAXParserFactory factory = SAXParserFactory.newInstance(); SAXParser saxParser = null; try { saxParser = factory.newSAXParser(); } catch (ParserConfigurationException e) { e.printStackTrace(); } catch (SAXException e) { e.printStackTrace(); } return saxParser; } class ColorHandler extends DefaultHandler { @Override public void startElement(String uri, String localName, String qName, Attributes attrs) throws SAXException { String name = attrs.getValue(0); String value = attrs.getValue(1); try { if (value.startsWith("#")) { //color int color = toColor(value); colorMap.put(name, color); } else { //config configMap.put(name, value); } } catch (Exception e) { e.printStackTrace(); } } /** * @param colorStr: #FFFFFF * @return:Color.rgb(256, 256, 256) */ private int toColor(String colorStr) { return Color.parseColor(colorStr); } } } 

I'd like to focus on:

1. The best library for parsing XML
2. The best code style that obeys software engineering
3. The best way to handle these exceptions

Answer 1

In short:

• If you split your parser in a few parts, you'd follow S in SOLID -- the Single Responsibility Principle that suggests to keep heterogeneous components apart.
• Let the parser throw exceptions itself so it could notify an exception to the caller. And let the call-site handle exceptions if it can. If you let the exceptions "bubble", you'd mostly find your code cleaner (at least, no more try/catch with null checks).
• Don't use e.printStackTrace() in most cases. There are sometimes reasons when this is necessary, but usually it marks wrong exception handling.

First, in Java 7, I'd abstract the parser component and let it just parse and do nothing else. Don't use the Context class to the parser as this is not a necessary thing here. You can easily use just an InputStream you could obtain from anywhere, not necessarily an input stream from assets (this simplifies unit testing very much).

public final class ThemeXmlParser { private final IThemeXmlHandler handler; private ThemeXmlParser(final IThemeXmlHandler handler) { this.handler = handler; } public static ThemeXmlParser getThemeXmlParser(final IThemeXmlHandler handler) { return new ThemeXmlParser(handler); } public void parse(final InputStream inputStream) throws ParserConfigurationException, SAXException, IOException { final XMLReader reader = SAXParserFactory.newInstance().newSAXParser().getXMLReader(); final ContentHandler contentHandler = new ThemeContentHandler(handler); reader.setContentHandler(contentHandler); reader.parse(new InputSource(inputStream)); } private static final class ThemeContentHandler extends DefaultHandler { private final IThemeXmlHandler handler; private ThemeContentHandler(final IThemeXmlHandler handler) { this.handler = handler; } @Override public void startElement(final String uri, final String localName, final String qName, final Attributes attributes) throws SAXException { switch ( qName ) { case "theme": break; case "color": handler.onColor(attributes.getValue("name"), attributes.getValue("value")); break; case "shape": handler.onShape(attributes.getValue("name"), attributes.getValue("value")); break; default: throw new SAXException(new IllegalArgumentException("Can't handle: " + qName)); } } } } 

Note that I prefer static factory methods and hide constructors as much as possible. Also I think that static inner classes are better because: 1) you can extract/move such classes easier than non-static ones; 2) you control what it gets, hence you don't capture that outer class reference and all of its fields. Also note that I use switch intentionally here for a few reasons (performance in general, compiler can trace code execution branches [let's say, you can switch to switch over enum rather than String and configure your IDE to show a warning that not all enum values are covered]).

Second, this is how the handler is defined:

public interface IThemeXmlHandler { void onColor(String name, String value); void onShape(String name, String value); } 

The method names are self-descriptive, and you can implement the methods in any way you want (the simplest case: just log the colors and shapes). Note that I intentionally pass raw String values as this doesn't let the interface bloat, and you can implement/delegate a value parser in a handler implementation.

Third, then, as one of many other implementations, we can define a simple aggregating handler.

public final class AggregatingThemeXmlHandler implements IThemeXmlHandler { private final Map<String, Integer> colors = new LinkedHashMap<>(); private final Map<String, String> aliasedColors = new LinkedHashMap<>(); private final Map<String, String> shapes = new LinkedHashMap<>(); private AggregatingThemeXmlHandler() { } public static AggregatingThemeXmlHandler getAggregatingThemeXmlHandler() { return new AggregatingThemeXmlHandler(); } @Override public void onColor(final String name, final String value) { if ( value.startsWith("#") ) { colors.put(name, parseColor(value)); } else { aliasedColors.put(name, value); } } @Override public void onShape(final String name, final String value) { shapes.put(name, value); } public Map<String, Integer> getColors() { return unmodifiableMap(colors); } public Map<String, String> getAliasedColors() { return unmodifiableMap(aliasedColors); } public Map<String, String> getShapes() { return unmodifiableMap(shapes); } } 

Note that unmodifiableMap decorator (basically a result can be immutable, as it's just a snapshot of a given XML) "protects" the aggregation result directly from elsewhere, however in this case it can be modified using the handler methods, but you can make rework it to build a result object with defence-copied maps so no handler interaction could change the result maps.

I'd recommend the Context class and the selectAssetsPath strategy method to be fully extracted elsewhere. Note that the implementation above can be tested, in principle, using unit tests during builds (except of parseColor that would throw a stub exception, but this could be worked around using the Strategy pattern).