05a80d2c7b26a787f4f1742644f76054

I have a map of strings where the values are strings that may contain one or more CSS class names separated by spaces. I want to make sure that each of the class names is valid. If a class is found to be invalid it will be replaced with nothing.

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
// Check to make sure value is non-null and matches CSS grammar
// A property may contain more than one class name so we need to
// check each potential class name in the string.
for (Map.Entry<String, String> entry : chromeCssProps.entrySet()) {
	String property = entry.getValue();

	if (property == null) {
		property = "";
		if (LOGGER.isDebugEnabled()) {
			LOGGER.debug("Property was null: " + entry.getKey());
		}
	} else {
		String[] classNames = property.split("\\s+");
		for (String s : classNames) {
			if (!s.matches("-?[_a-zA-Z]+[_a-zA-Z0-9-]*")) {
				property = property.replace(s, "");
				if (LOGGER.isInfoEnabled()) {
					LOGGER.info("Invalid CSS class name in "
							+ entry.getKey() + ": "
							+ s);
				}
			}
		}
	}

	entry.setValue(property);
}

Refactorings

No refactoring yet !

60929e8e0445f0070e501f5ded8ad348

MetroidFan2002

July 31, 2010, July 31, 2010 18:45, permalink

1 rating. Login to rate!

The code seems incorrect - as Strings are immutable in Java, the following line should replace line 14: property = property.replace(s, "");

Also, if you prevented nulls from being valid values in the map, you could shorten the method slightly - I'm assuming your regex is working, and everything else is ok.

1
2
3
4
5
6
7
8
9
10
11
12
for (Map.Entry<String, String> entry : chromeCssProps.entrySet()) {
	String property = entry.getValue();

	String[] classNames = property.split("\\s+");
	for (String s : classNames) {
		if (!s.matches("-?[_a-zA-Z]+[_a-zA-Z0-9-]*")) {
			property = property.replace(s, "");
		}
	}

	entry.setValue(property);
}
Avatar

Stefan Vartolomeev

August 29, 2010, August 29, 2010 16:54, permalink

1 rating. Login to rate!

Your code for matching (Line 15: !s.matches("-?[_a-zA-Z]+[_a-zA-Z0-9-]*") ) is lot's more readable, but you can gain some performance if you compile your regular expression only once. It is similar to using StringBuilder.append instead of String concatenations.

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
Pattern classPattern = Pattern.compile("-?[_a-zA-Z]+[_a-zA-Z0-9-]*");	

for (Map.Entry<String, String> entry : chromeCssProps.entrySet()) {
	String property = entry.getValue();

	if (property == null) {
		property = "";
		debug("Property was null: " + entry.getKey());
	} else {
		String[] classNames = property.split("\\s+");
		for (String s : classNames) {
			if (!classPattern.matcher(s).matches()) {
				property = property.replace(s, "");
				info("Invalid CSS class name in " + entry.getKey() + ": " + s);
			}
		}
	}
	entry.setValue(property);
}

Your refactoring





Format Copy from initial code

or Cancel