Soraya’s company recently decided to expand the payment options that they support. This meant integrating “Inipay”, Initech’s API for handling payment processing globally. This particular API is open sourced, which means that Soraya was able to investigate exactly how the sausage was made.
Many of the classes were tagged as being @author auto create
. In fact, there were about 2,000 such classes, all nearly identical aside from a few minor differences. What got Soraya’s attention though was that each of them referred to InipayObject
and InipayHashMap
. Re-implementing standard classes is always a concern.
public class InipayHashMap extends HashMap<String, String>
That right there is a bit menacing. I’m actually a big fan of aliasing complex generic types into simpler classes, but a HashMap<String, String>
tells me that we’re looking at stringly typed code.
public String put(String key, Object value) {
String strValue;
if (value == null) {
strValue = null;
} else if (value instanceof String) {
strValue = (String) value;
} else if (value instanceof Integer) {
strValue = ((Integer) value).toString();
} else if (value instanceof Long) {
strValue = ((Long) value).toString();
} else if (value instanceof Float) {
strValue = ((Float) value).toString();
} else if (value instanceof Double) {
strValue = ((Double) value).toString();
} else if (value instanceof Boolean) {
strValue = ((Boolean) value).toString();
} else if (value instanceof Date) {
DateFormat format = new SimpleDateFormat(InipayConstants.DATE_TIME_FORMAT);
format.setTimeZone(TimeZone.getTimeZone(InipayConstants.DATE_TIMEZONE));
strValue = format.format((Date) value);
} else {
strValue = value.toString();
}
return this.put(key, strValue);
}
So, this is how they overload the put
method. Fun fact: if you invoke toString
on a String
object in Java, it returns itself. Which means nearly all of the instanceof
checks are 100% unnecessary. I say nearly, because while you could call Object.toString
on pretty much everything, Date
s do need some special care and handling. Still, I’m skeptical that this code belongs in your HashMap
implementation.
Note, the last line calls the base class’s put
, but… no, not really. We overrode that, too.
public String put(String key, String value) {
if (StringUtils.areNotEmpty(key, value)) {
return super.put(key, value);
} else {
return null;
}
}
The real magic here is that if key
and value
don’t meet whatever criteria areNotEmpty
requires (presumably not null, and not empty strings), we silently ignore them. We don’t fail. We don’t raise an exception. We just silently ignore it. Worse, the put
method in Java’s Map
s should return the old value, if there was one, and null otherwise, so this doesn’t even fit the contract of the interface it’s implementing, and thus doesn’t behave like a HashMap
should.
In practice, these InipayHashMap
objects are used as part of every one of the 2,000 request objects, meant to store “user defined values”. In other words, it’s an uncapped pile of stringly typed data offered to end users to do whatever they please with it, making it maximally Enterprisey.