Matt needed to add a new field to a form. This simple task was made complicated by the method used to save changes back to the database. Let’s see if you can spot what the challenge was:
public int saveQualif(String docClass, String transcomId, String cptyCod, String tradeId, String originalDealId, String codeEvent, String multiDeal,
String foNumber, String codeInstrfamily, String terminationDate, String premiumAmount, String premiumCurrency, String notionalAmount,
String codeCurrency, String notionalAmount2, String codeCurrency2, String fixedRate, String payout, String maType, String maDate,
String isdaZoneCode, String tradeDate, String externalReference, String entityCode, String investigationFileReference,
String investigationFileStartDate, String productType, String effectiveDate, String expiryDate, String paymentDate, String settInstrucTyp,
String opDirection, String pdfPassword, String extlSysCod, String extlDeaId, String agrDt) throws TechnicalException, DfException
That’s 36 parameters right there. This function, internally, creates a data access object which takes just as many parameters in its constructor, and then does a check: if a field is non-null, it updates that field in the database, otherwise it doesn’t.
Of course, every single one of those parameters is stringly typed, which makes it super fun. Tracking premiumAmount and terminationDate as strings is certainly never going to lead to problems. I especially like the pdfPassword being stored, which is clearly just the low-security password meant to be used for encrypting a transaction statement or similar: “the last 4 digits of your SSN” or whatever. So I guess it’s okay that it’s being stored in the clear in the database, but also I still hate it. Do better!
In any case, this function was called twice. Once from the form that Matt was editing, where every parameter was filled in. The second time, it was called like this:
int nbUpdates = incoming.saveQualif(docClass, null, null, null, null, null, multiDeal, null,
null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null,
null, null, null, null, null, null, null, null, null, null, null, null);
As tempted as Matt was to fix this method and break it up into multiple calls or change the parameters to a set of classes or anything better, he was too concerned about breaking something and spending a lot of time on something which was meant to be a small, fast task. So like everyone who’d come before him, he just slapped in another parameter, tested it, and called it a day.
Refactoring is a problem for tomorrow’s developer.
[Advertisement]BuildMaster allows you to create a self-service release management platform that allows different teams to manage their applications. Explore how!
Source: Read MoreÂ
