CodeSOD: A Valid Applicant
In the late 90s into the early 2000s, there was an entire industry spun up to get businesses and governments off their mainframe systems from the 60s and onto something modern. "Modern", in that era, usually meant Java. I attended vendor presentations, for example, that promised that you could take your mainframe, slap a SOAP webservice on it, and then gradually migrate modules off the mainframe and into Java Enterprise Edition. In the intervening years, I have seen exactly 0 successful migrations like this- usually they just end up trying that for a few years and then biting the bullet and doing a ground-up rewrite.
That's is the situation ML was in: a state government wanted to replace their COBOL mainframe monster with a "maintainable" J2EE/WebSphere based application. Gone would be the 3270 dumb terminals, and here would be desktop PCs running web browsers.
ML's team did the initial design work, which the state was very happy with. But the actual development work gave the state sticker shock, so they opted to take the design from ML's company and ship it out to a lowest-bidder offshore vendor to actually do the development work.
This, by the way, was another popular mindset in the early 00s: you could design your software as a set of UML diagrams and then hand them off to the cheapest coder you could hire, and voila, you'd have working software (and someday soon, you'd be able to just generate the code and wouldn't need the programmer in the first place! ANY DAY NOW).
Now, this code is old, and predates generics in Java, so the use of ArrayLists isn't the WTF. But the programmer's approach to polymorphism is.
public class Applicant extends Person {// ... [snip] ...}...public class ApplicantValidator {public void validateApplicantList(List listOfApplicants) throws ValidationException {// ... [snip] ...// Create a List of Person to validateList listOfPersons = new ArrayList();Iterator i = listOfApplicants.iterator(); while (i.hasNext()) {Applicant a = (Applicant) i.next();Person p = (Person) a;listOfPersons.add(p);}PersonValidator.getInstance().validatePersonList(listOfPersons);// ... [snip] ...}// ... [snip] ...}
Here you see an Applicant is a subclass of Person. We also see an ApplicantValidator class, which needs to verify that the applicant objects are valid- and to do that, they need to be treated as valid Person objects.
To do this, we iterate across our list of applications (which, it's worth noting, are being treated as Objects since we don't have generics), cast them to Applicants, then cast the Applicant variable to Person, then create a list of Persons- which again, absent generics, is just a list of Objects. Then we pass that list of persons into validatePersonList.
All of this is unnecessary, and demonstrates a lack of understanding about the language in use. This block could be written more clearly as: PersonValidator.getInstance().validatePersonList(listOfApplicants);
This gives us the same result with significantly less effort.
While much of the code coming from the offshore team was actually solid, it contained so much nonsense like this, so many misundertandings of the design, and so many bugs, that the state kept coming back to ML's company to address the issues. Between paying the offshore team to do the work, and then ML's team to fix the work, the entire project cost much more than if they had hired ML's team in the first place.
But the developers still billed at a lower rate than ML's team, which meant the managers responsible still got to brag about cost savings, even as they overran the project budget. "Imagine how much it would have cost if we hadn't gone with the cheaper labor?"
[Advertisement] Utilize BuildMaster to release your software with confidence, at the pace your business demands. Download today!