CodeSOD: Read the Comments

Many many years ago, when I took my first programming course in high school, our instructor had... opinions. One of those opinions was that you should manually syntax check your code and not rely on compiler errors, because back in her day, you had to punch your program into cards, drop it off in the computer lab, wait for the technician to run your batch, and then pick up the results on the printer. It needed to work the first try or you might be waiting a whole day before you could try again.

One of her other opinions was that your code should contain as many comments as it contained lines of code. Everything needed comments. Everything. Which brings us to this code from Doug.

Private Sub BuildActuarial04_14Row(exportString As StringBuilder, record As ap_Actuarial04_14_Result) 'Card Type exportString.Append(record.CardType) 'SSN exportString.Append(record.SSN) 'Name exportString.Append(helper.FormatData(record.Name, 25, Helpers.DataType.StringFormat)) 'Sex exportString.Append(record.Sex) 'Birth Date exportString.Append(record.BirthDate) 'Hire Date exportString.Append(helper.FormatData(record.HireDate, 8, Helpers.DataType.StringFormat)) 'Plan Number exportString.Append(record.PlanNumber) 'Parish Or Other Codes 1 exportString.Append(record.ParishOrOtherCodes1) 'Parish Or Other Codes 2 exportString.Append(record.ParishOrOtherCodes2) 'Service Code exportString.Append(record.ServiceCode) 'Years Of Eligible Service exportString.Append(record.YearsOfEligibleService) 'Years Of Credited Service exportString.Append(record.YearsOfCreditedService) 'Final Average Compensation Monthly exportString.Append(record.FinalAverageCompensationMonthly) 'Annual Member Contributions For Fye exportString.Append(record.AnnualMemberContributionsForFye) 'Accumulated Member Contributions exportString.Append(record.AccumulatedMemberContributions) 'Contribution Interest exportString.Append(record.ContributionInterest) 'Termination Date exportString.Append(record.TerminationDate) 'Rehired exportString.Append(record.Rehired) 'Ibo exportString.Append(record.Ibo) 'Retirement Date exportString.Append(record.RetirementDate) 'Estimated Accrued Benefit exportString.Append(record.EstimatedAccruedBenefit) 'Original Retirement Benefit exportString.Append(record.OriginalRetirementBenefit) 'Option Code exportString.Append(record.OptionCode) 'Retirement Code exportString.Append(record.RetirementCode) 'Sex Of Beneficiary exportString.Append(record.SexOfBeneficiary) 'Beneficiary Birth Date exportString.Append(record.BeneficiaryBirthDate) 'Childs Monthly Benefit exportString.Append(record.ChildsMonthlyBenefit) 'Childs Birth Date exportString.Append(record.ChildsBirthDate) 'Years Of Military Service exportString.Append(record.YearsOfMilitaryService) 'Beneficiarys SSN exportString.Append(record.BeneficiarysSSN) 'Total Accumulated Years Of Service Purchased exportString.Append(record.TotalAccumulatedYearsOfServicePurchased) 'End Line exportString.AppendLine() End Sub

Doug's co-worker may have been in my high school class.

What's particularly ironic about this overcommenting is the method name: BuildActuarial04_14Row. What does the 04_14 mean? Why does this merit its own code path? Is that a date? A special internal code? Are there 05_14 rows?

There's a big code smell on that part, because it implies a whole suite of ap_Actuarial...Result classes that probably have the same fields, but don't have the same interface, and someone hacked together this private polymorphism.

[Advertisement] BuildMaster allows you to create a self-service release management platform that allows different teams to manage their applications. Explore how!