CodeSOD: The Truth and the Truth |
When Andy inherited some C# code from a contracting firm, he gave it a quick skim. He saw a bunch of methods with names like IsAvailable
or CanPerform…
, but he also saw that it was essentially random as to whether or not these methods returned bool
or string
.
That didn't seem like a good thing, so he started to take a deeper look, and that's when he found this.
public ActionResult EditGroup(Group group)
{
string fleetSuccess = string.Empty;
bool success = false;
if(action != null)
{
fleetSuccess = updateGroup(group);
}
else
{
fleetSuccess = Boolean.TrueString;
}
success = updateExternalGroup(group);
fleetSuccess += "&&&" + success;
if (fleetSuccess.ToLower().Equals("true&&&true"))
{
GetActivityDataFromService(group, false);
}
return Json(fleetSuccess, JsonRequestBehavior.AllowGet);
}
So, updateGroup
returns a string containing a boolean (at least, we hope it contains a boolean). updateExternalGroup
returns an actual boolean. If both of these things are true, than we want to invoke GetActivityDataFromService
.
Clearly, the only way to do this comparison is to force everything into being a string, with a &&&
jammed in the middle as a spacer. Uh, for readability, I guess? Maybe? I almost suspect someone thought they were inventing their own "and" operator and didn't want it to conflict with &
or &&
.
Or maybe, maybe their code was read aloud by Jeff Goldblum. "True, and-and-and true!" It's very clear they didn't think about whether or not they should do this.
Метки: CodeSOD |
CodeSOD: A Form of Reuse |
Writing code that is reusable is an important part of software development. In a way, we're not simply solving the problem at hand, but we're building tools we can use to solve similar problems in the future. Now, that's also a risk: premature abstraction is its own source of WTFs.
Daniel's peer wrote some JavaScript which is used for manipulating form inputs on customer contact forms. You know the sorts of forms: give us your full name, phone number, company name, email, and someone from our team will be in touch. This developer wrote the script, and offered it to clients to enhance their forms. Well, there was one problem: this script would get embedded in customer contact forms, but not all customer contact forms use the same conventions for how they name their fields.
There's an easy solution for that, involving parameterizing the code or adding a configuration step. There's a hard solution, where you build a heuristic that works for most forms. Then there's this solution, which… well…. Let me present the logic for handling just one field type, unredacted or elided.
for(llelementlooper=0; llelementlooper<document.forms[llformlooper2].elements.length; llelementlooper++)
{
var llelementphone = (document.forms[llformlooper2].elements[llelementlooper].name)
if ( llformphone == '' && ((llelementphone=='phone')
|| (llelementphone=='Phone')
|| (llelementphone=='phone')
|| (llelementphone=='mobilephone')
|| (llelementphone=='PHONE')
|| (llelementphone=='sPhone')
|| (llelementphone=='strPhone')
|| (llelementphone=='Telephone')
|| (llelementphone=='telephone')
|| (llelementphone=='tel')
|| (llelementphone=='si_contact_ex_field6')
|| (llelementphone=='phonenumber')
|| (llelementphone=='phone_number')
|| (llelementphone=='phoneTextBox')
|| (llelementphone=='PhoneNumber_num_25_1')
|| (llelementphone=='Telefone')
|| (llelementphone=='Contact Phone')
|| (llelementphone=='submitted[row_3][phone]')
|| (llelementphone=='edit-profile-phone')
|| (llelementphone=='contactTelephone')
|| (llelementphone=='f4')
|| (llelementphone=='Contact-Phone')
|| (llelementphone=='formItem_239')
|| (llelementphone=='phone_r')
|| (llelementphone=='PhoneNo')
|| (llelementphone=='LeadGen_ContactForm_98494_m0:Phone')
|| (llelementphone=='telefono')
|| (llelementphone=='ntelephone')
|| (llelementphone=='wtelephone')
|| (llelementphone=='watelephone')
|| (llelementphone=='form[telefoon]')
|| (llelementphone=='phone_work')
|| (llelementphone=='telephone-number')
|| (llelementphone=='ctl00$HeaderText$ctl00$PhoneText')
|| (llelementphone=='ctl00$ctl00$cphMain$cphInsideMain$widget1$ctl00$viewBiz$ctl00$phone$textbox')
|| (llelementphone=='ctl00$ctl00$ContentPlaceHolderBase$ContentPlaceHolderSideMenu$TextBoxPhone')
|| (llelementphone=='ctl00$SPWebPartManager1$g_c8bd31c3_e338_41df_bdbe_021242ca01c8$ctl01$ctl06$txtTextbox')
|| (llelementphone=='ctl00$ctl00$ctl00$ContentPlaceHolderDefault$MasterContentPlaceHolder$txtPhone')
|| (llelementphone=='curftelephone')
|| (llelementphone=='form[Telephone]')
|| (llelementphone=='tx_pilmailform_pi1[text][phone]')
|| (llelementphone=='ctl00$ctl00$templateMainContent$homeBanners$HomeBannerList$ctrLeads$txt_5_1')
|| (llelementphone=='ac_daytimeNumber')
|| (llelementphone=='daytime_phone')
|| (llelementphone=='r4')
|| (llelementphone=='ctl00$ContentPlaceHolderBody$Phone')
|| (llelementphone=='Fld10_label')
|| (llelementphone=='field333')
|| (llelementphone=='txtMobile')
|| (llelementphone=='form_nominator_phonenumber')
|| (llelementphone=='submitted[phone_no]')
|| (llelementphone=='submitted[phone]')
|| (llelementphone=='submitted[5]')
|| (llelementphone=='submitted[telephone_no]')
|| (llelementphone=='fields[Contact Phone]')
|| (llelementphone=='cf2_field_5')
|| (llelementphone=='a23786')
|| (llelementphone=='rpr_phone')
|| (llelementphone=='phone-number')
|| (llelementphone=='txt_homePhone')
|| (llelementphone=='your-number')
|| (llelementphone=='Contact_Phone')
|| (llelementphone=='ctl00$CPH_body$txtContactnumber')
|| (llelementphone=='profile_telephone')
|| (llelementphone=='item_meta[90]' && llfrmid==11823)
|| (llelementphone=='item_meta[181]' && llfrmid==26416)
|| (llelementphone=='input_4' && llfrmid==21452)
|| (llelementphone=='EditableTextField100' && llfrmid==13948)
|| (llelementphone=='EditableTextField205' && llfrmid==13948)
|| (llelementphone=='EditableTextField100' && llfrmid==13948)
|| (llelementphone=='EditableTextField166' && llfrmid==13948)
|| (llelementphone=='EditableTextField104' && llfrmid==13948)
|| (llelementphone=='cf2_field_4' && llfrmid==23878)
|| (llelementphone=='input_4' && llfrmid==24017)
|| (llelementphone=='cf_field_4' && llfrmid==15876)
|| (llelementphone=='cf5_field_5' && llfrmid==15876)
|| (llelementphone=='input_9' && llfrmid==17254)
|| (llelementphone=='input_2' && llfrmid==22954)
|| (llelementphone=='input_8' && llfrmid==23756)
|| (llelementphone=='input_3' && llfrmid==18793)
|| (llelementphone=='input_6' && llfrmid==24811)
|| (llelementphone=='input_3' && llfrmid==19880)
|| (llelementphone=='input_6' && llfrmid==19230)
|| (llelementphone=='input_3' && llfrmid==24747)
|| (llelementphone=='input_4' && llfrmid==25897)
|| (llelementphone=='text-481' && llfrmid==14451)
|| (llelementphone=='Form7111$formField_7576')
|| (llelementphone=='Form7168$formField_7673')
|| (llelementphone=='Form7116$formField_7592')
|| (llelementphone=='Form7150$formField_7645')
|| (llelementphone=='Form7153$formField_7655')
|| (llelementphone=='Form7119$formField_7600')
|| (llelementphone=='Form7123$formField_7608')
|| (llelementphone=='Form7161$formField_7665')
|| (llelementphone=='Form7176$formField_7690')
|| (llelementphone=='Form7172$formField_7681')
|| (llelementphone=='Form7113$formField_7584')
|| (llelementphone=='Form7106$formField_7568')
|| (llelementphone=='Form7111$formField_7576')
|| (llelementphone=='Form7136$formField_7628')
|| (llelementphone=='Form6482$formField_7621')
|| (llelementphone=='Form6548$formField_6988')
|| (llelementphone=='submitted[business_phone]')
|| (llelementphone=='tfa_3' && llfrmid==23388)
|| (llelementphone=='ContentObjectAttribute_ezsurvey_answer_4455_3633')
|| (llelementphone=='838ae21c-1f95-488f-a511-135a588a50fb_Phone')
|| (llelementphone=='plc$lt$zoneContent$pageplaceholder$pageplaceholder$lt$zoneRightContent$contentText$BizFormControl1$Bizform1$ctl00$Telephone$txt1st')
|| (llelementphone=='plc$lt$zoneContent$pageplaceholder$pageplaceholder$lt$zoneRightContent$contentText$BizFormControl1$Bizform1$ctl00$Telephone')
|| (llelementphone=='ctl00$ctl00$ctl00$ContentPlaceHolderDefault$ContentAreaPlaceholderMain$ctl02$ContactForm_3$TextBoxTelephone')
|| (llelementphone=='plc$lt$Content2$pageplaceholder1$pageplaceholder1$lt$Content$BizForm$viewBiz$ctl00$Phone_Number')
|| (llelementphone=='ctl00$ctl00$ContentPlaceHolder1$cphMainContent$C002$tbTelephone')
|| (llelementphone=='contact$tbPhoneNumber')
|| (llelementphone=='crMain$ctl00$txtPhone')
|| (llelementphone=='ctl00$PrimaryContent$tbPhone')
|| (llelementphone=='ff_nm_phone[]')
|| (llelementphone=='q5_phoneNumber5[phone]')
|| (llelementphone=='TechContactPhone')
|| (llelementphone=='referral_phone_number')
|| (llelementphone=='field8418998')
|| (llelementphone=='ctl00$Content$ctl00$txtPhone')
|| (llelementphone=='ctl00$PlaceHolderMain$ucContactUs$txtPhone')
|| (llelementphone=='m_field_id_4' && llfrmid==15091)
|| (llelementphone=='Field7' && llfrmid==23387)
|| (llelementphone=='input_4' && llfrmid==22578)
|| (llelementphone=='input_2' && llfrmid==11241)
|| (llelementphone=='input_7' && llfrmid==23633)
|| (llelementphone=='input_7' && llfrmid==22114)
|| (llelementphone=='input_4' && (llformalyzerURL.indexOf('demo') != -1) && llfrmid==17544)
|| (llelementphone=='input_4' && (llformalyzerURL.indexOf('contact') != -1) && llfrmid==17544)
|| (llelementphone=='field_4' && llfrmid==24654)
|| (llelementphone=='input_6' && llfrmid==24782)
|| (llelementphone=='input_4' && (llformalyzerURL.indexOf('contact-us') != -1) && llfrmid==16794)
|| (llelementphone=='input_3' && (llformalyzerURL.indexOf('try-and-buy') != -1) && llfrmid==16794)
|| (llelementphone=='input_4' && (llformalyzerURL.indexOf('contact-us') != -1) && llfrmid==23842)
|| (llelementphone=='input_4' && llfrmid==25451)
|| (llelementphone=='input_5' && llfrmid==24911)
|| (llelementphone=='input_3' && llfrmid==13417)
|| (llelementphone=='input_4' && llfrmid==23813)
|| (llelementphone=='input_4' && llfrmid==21483)
|| (llelementphone=='input_3' && llfrmid==25396)
|| (llelementphone=='input_3' && llfrmid==16175)
|| (llelementphone=='input_7' && llfrmid==25797)
|| (llelementphone=='input_4' && llfrmid==15650)
|| (llelementphone=='input_3' && llfrmid==22025)
|| (llelementphone=='input_3' && llfrmid==14534)
|| (llelementphone=='input_4' && llfrmid==25216)
|| (llelementphone=='input_5' && llfrmid==22884)
|| (llelementphone=='input_6' && llfrmid==25783)
|| (llelementphone=='text-747' && llfrmid==16324)
|| (llelementphone=='vfb-42' && llfrmid==24468)
|| (llelementphone=='vfb-33' && llfrmid==24468)
|| (llelementphone=='item_meta[57]' && llfrmid==25268)
|| (llelementphone=='item_meta[78]' && llfrmid==25268)
|| (llelementphone=='item_meta[85]' && llfrmid==25268)
|| (llelementphone=='item_meta[154]' && llfrmid==25268)
|| (llelementphone=='item_meta[220]' && llfrmid==25268)
|| (llelementphone=='item_meta[240]' && llfrmid==25268)
|| (llelementphone=='item_meta[286]' && llfrmid==25268)
|| (llelementphone=='fieldname5' && llfrmid==12535)
|| (llelementphone=='Question12' && llfrmid==24639)
|| (llelementphone=='ninja_forms_field_4' && llfrmid==19321)
|| (llelementphone=='EditableTextField' && llfrmid==15064)
|| (llelementphone=='form_fields[27]' && llfrmid==22688)
|| (llelementphone=='ctl00$body$phone')
|| (llelementphone=='ctl00$MainContent$txtPhone')
|| (llelementphone=='FreeTrialForm$Phone')
|| (llelementphone=='text-521ada035aa46')
|| (llelementphone=='C_BusPhone')
|| (llelementphone=='ctl00$ctl00$templateMainContent$pageContent$ctrLeads$txt_5_1')
|| (llelementphone=='ctl00$Modules$ctl00$rptFields$ctl06$1204')
|| (llelementphone=='ctl00$Modules$ctl00$rptFields$ctl06$1320')
|| (llelementphone=='ctl00$Modules$ctl00$rptFields$ctl07$1242')
|| (llelementphone=='ctl00$Modules$ctl00$rptFields$ctl07$1202')
|| (llelementphone=='ctl00$Modules$ctl00$rptFields$ctl08$1242')
|| (llelementphone=='ctl00$MainColumnPlaceHolder$uxPhone')
|| (llelementphone=='ctl00$MainContent$DropZoneTop$columnDisplay$ctl04$controlcolumn$ctl00$WidgetHost$WidgetHost_widget$IDPhone')
|| (llelementphone=='ctl00$ctl05$txtPhone')
|| (llelementphone=='ctl00$Modules$ctl00$rptFields$ctl07$1219')
|| (llelementphone=='LeadGen_ContactForm_33872_m419365:Phone')
|| (llelementphone=='F02220803')
|| (llelementphone=='h2c0f')
|| (llelementphone=='your_phone_number')
|| (llelementphone=='Question7')
|| (llelementphone=='Question51')
|| (llelementphone=='Question59')
|| (llelementphone=='Question35')
|| (llelementphone=='Question67')
|| (llelementphone=='field9740823')
|| (llelementphone=='message[phone]')
|| (llelementphone=='dnn$ctr1266$ViewKamakuraRegister$Phone')
|| (llelementphone=='phone1')
|| (llelementphone=='inf_field_Phone1')
|| (llelementphone=='hscontact_phone')
|| (llelementphone=='data[Contact][phone]')
|| (llelementphone=='fields[Phone]')
|| (llelementphone=='contact[PhoneNumber]')
|| (llelementphone=='phonename3')
|| (llelementphone=='UserPhone')
|| (llelementphone=='ctl00$MainBody$txtPhoneTech')
|| (llelementphone=='Telephone1')
|| (llelementphone=='PhoneNumber')
|| (llelementphone=='work_phone')
|| (llelementphone=='jform[contact_telephone]')
|| (llelementphone=='form[phone]')
|| (llelementphone=='RequestAQuote1$txtPhone')
|| (llelementphone=='06_Phone')
|| (llelementphone=='txtPhone')
|| (llelementphone=='field_location[und][0][phone]')
|| (llelementphone=='your-phone')
|| (llelementphone=='cmsForms_phone')
|| (llelementphone=='Txt_phonenumber')
|| (llelementphone=='businessPhone')
|| (llelementphone=='boxHomePhone')
|| (llelementphone=='HomePhone')
|| (llelementphone=='request-phone')
|| (llelementphone=='user[phone]')
|| (llelementphone=='DATA[PHONE]')
|| (llelementphone=='ctl00$ctl00$ctl00$cphContent$cphContent$cphContent$Phone')
|| (llelementphone=='ctl00$MainBody$Form1$obj11')
|| (llelementphone=='LeadGen_ContactForm_90888_m1467651:Phone')
|| (llelementphone=='Users[work]')
|| (llelementphone=='Question43')
|| (llelementphone=='aics_phone')
|| (llelementphone=='form[workphone]')
|| (llelementphone=='ctl00$ctl00$ContentPlaceHolder1$cphMainContent$C006$tbTelephone')
|| (llelementphone=='cntnt01fbrp__47')
|| (llelementphone=='submitted[phone_number]')
|| (llelementphone=='flipform_phone')
|| (llelementphone=='txtPhone')
|| (llelementphone=='ctl00$ContentPlaceHolder2$txtPhnno')
|| (llelementphone=='ctl00$ctl00$ContentPlaceHolder1$ContentPlaceHolder1$mainContentRegion$BizFormControl1$Bizform1$ctl00$Phone')
|| (llelementphone=='inpPhone')
|| (llelementphone=='j_phone')
|| (llelementphone=='m6e81afbrp__53')
|| (llelementphone=='item_meta[119]')
|| (llelementphone=='ctl00$ContentPlaceHolder_Content$dataPhone')
|| (llelementphone=='ctl00$generalContentPlaceHolder$ctrlContactUs$tbPhone')
|| (llelementphone=='ctl00$ctl00$ctl00$ContentPlaceHolderDefault$ContentPlaceHolder1$Contact_6$txtPhone')
|| (llelementphone=='ctl00$MainContent$tel')
|| (llelementphone=='dynform_element_3')
|| (llelementphone=='telephone_1')
|| (llelementphone=='cf_phone')
|| (llelementphone=='Lead_PrimaryPhone')
|| (llelementphone=='p_lt_zoneContent_wP_wP_lt_zonePageWidgets_RevolabsMicrosoftDynamicsCRMContactForm_1_txtBusinessPhone')
|| (llelementphone=='si_contact_ex_field2')
|| (llelementphone=='dnn$ctr458$XModPro$ctl00$ctl00$ctl00$Telephone')
|| (llelementphone=='ctl00$ctl06$txtTelephone')
|| (llelementphone=='dnn$ctr458$XModPro$ctl00$ctl00$ctl00$Telephone')
|| (llelementphone=='ctl00$ctl00$mainCopy$CPHCenter$ctl00$QuickRegControl_2$TBPhone')
|| (llelementphone=='LeadGen_ContactForm_38163_m457931:Phone')
|| (llelementphone=='LeadGen_ContactForm_29909_m371524:Phone')
|| (llelementphone=='LeadGen_ContactForm_32343_m395611:Phone')
|| (llelementphone=='LeadGen_ContactForm_31530_m388101:Phone')
|| (llelementphone=='LeadGen_ContactForm_27072_m349818:Phone')
|| (llelementphone=='LeadGen_ContactForm_28362_m354522:Phone')
|| (llelementphone=='LeadGen_ContactForm_28759_m358745:Phone')
|| (llelementphone=='LeadGen_ContactForm_32343_m395611:Phone')
|| (llelementphone=='LeadGen_ContactForm_33631_m415978:Phone')
|| (llelementphone=='LeadGen_ContactForm_30695_m380436:Phone')
|| (llelementphone=='LeadGen_ContactForm_29958_m372138:Phone')
|| (llelementphone=='LeadGen_ContactForm_31471_m387422:Phone')
|| (llelementphone=='LeadGen_ContactForm_32514_m397613:Phone')
|| (llelementphone=='LeadGen_ContactForm_29152_m362772:Phone')
|| (llelementphone=='LeadGen_ContactForm_32540_m397908:Phone')
|| (llelementphone=='pNumber')
|| (llelementphone=='organizer_phone')
|| (llelementphone=='ctl00$PlaceHolderMain$TrialDownloadForm$Phone')
|| (llelementphone=='ContactSubmission.Phone.Value')
|| (llelementphone=='ctl00$body$txtPhone')
|| (llelementphone=='p$lt$ctl03$pageplaceholder$p$lt$zoneCentre$editabletext$ucEditableText$widget1$ctl00$viewBiz$ctl00$Telephone$textbox')
|| (llelementphone=='ctl01_ctl00_pbForm1_ctl_phone_61f3')
|| (llelementphone=='ctl01$ctl00$ContentPlaceHolder1$ctl15$Phone')
|| (llelementphone=='p$lt$zoneContent$pageplaceholder$p$lt$zoneRightContent$contentText$ucEditableText$BizFormControl1$Bizform1$ctl00$Telephone$textbox')
|| (llelementphone=='ctl00$ctl00$ContentPlaceHolder$ContentPlaceHolder$ctl00$fPhone')
|| (llelementphone=='pagecolumns_0$form_B502CC1EC1644B38B722523526D45F36$field_6BCFC01A782747DF8E785B5533850EEB')
|| (llelementphone=='cf3_field_10')
|| (llelementphone=='r_phone')
|| (llelementphone=='c_phone')
|| (llelementphone=='cf-1[]')
|| (llelementphone=='frm_phone')
|| (llelementphone=='Patient_Phone_Number')
|| (llelementphone=='ctl00$PageContent$ctl00$txtPhone')
|| (llelementphone=='dnn$ctr398$FormMaster$ctl_6e49bedd138a4684a66b62dcb1a34658')
|| (llelementphone=='id_tel')
|| (llelementphone=='field_contact_tel[und][0][value]')
|| (llelementphone=='Phone:')
|| (llelementphone=='ContactPhone')
|| (llelementphone=='submitted[telephone]')
|| (llelementphone=='ctl00$ContentPlaceHolder1$ctl04$txtPhone')
|| (llelementphone=='ctl00$ContentPlaceHolder_pageContent$contact_phone')
|| (llelementphone=='264')
|| (llelementphone=='form_phone_number')
|| (llelementphone=='field8418998')
|| (llelementphone=='phoneTBox')
|| (llelementphone=='pagecontent_1$content_0$contentbottom_0$txtPhone')
|| (llelementphone=='application_0$PhoneTextBox')
|| (llelementphone=='submitted[phone_work]')
|| (llelementphone=='data[Lead][phone]')
|| (llelementphone=='a4475-telephone')
|| (llelementphone=='ctl00$Form$txtPhoneNumber')
|| (llelementphone=='signup_form_data[Phone]')
|| (llelementphone=='WorkPhone')
|| (llelementphone=='lldPhone')
|| (llelementphone=='web_form_1[field_102]value')
|| (llelementphone=='LeadGen_ContactForm_114694_m1832700:Phone')
|| (llelementphone=='phoneSalesForm')
|| (llelementphone=='fund_phone')
|| (llelementphone=='Phonepi_Phone')
|| (llelementphone=='field343')
|| (llelementphone=='cntnt01fbrp__48')
|| (llelementphone=='contact[phone]')
|| (llelementphone=='ctl00_ContentPlaceHolder1_ctl01_contactTelephoneBox_text')
|| (llelementphone=='ctl01$ctl00$ContentPlaceHolder1$ctl29$Phone')
|| (llelementphone=='plc$lt$content$pageplaceholder$pageplaceholder$lt$bodyColumnZone$LogilityContactUs$txtWorkPhone')
|| (llelementphone=='ctl00$ctl00$ctl00$cphBody$cphMain$cphMain$FormBuilder1$FormBuilderListView$ctrl4$FieldControl_Telephone')
|| (llelementphone=='ctl00$ctl00$ctl00$ContentPlaceHolderDefault$cp_content$ctl02$RenderForm_1$rpFieldsets$ctl00$rpFields$ctl04$126d33a3_9f7f_4583_8c94_5820d58fc030')
|| (llelementphone=='tx_powermail_pi1[uid1266]')
|| (llelementphone=='si_contact_ex_field3')
|| (llelementphone=='inc_contact1$txtPhone')
|| (llelementphone=='item2_tel_1')
|| (llelementphone=='LeadGen_ContactForm_15766_m0:Phone')
|| (llelementphone=='ctl00$ContentPlaceHolder1$txtPhone')
|| (llelementphone=='Default$Content$FormViewer$FieldsRepeater$ctl04$ctl00$ViewTextBox')
|| (llelementphone=='Default$Content$FormViewer$FieldsRepeater$ctl04$ctl00$ViewTextBox')
|| (llelementphone=='ctl00$SecondaryPageContent$C005$ctl00$ctl00$C002$ctl00$ctl00$textBox_write')
|| (llelementphone=='_u216318653597056311')
|| (llelementphone=='_u630018292785751084')
|| (llelementphone=='data[Contact][office_phone]')
|| (llelementphone=='ctl00$ctl00$cphMainContent$Content$txtPhone')
|| (llelementphone=='ctl00$ContentPlaceHolder1$txtTel')
|| (llelementphone=='item_5')
|| (llelementphone=='ques_21432')
|| (llelementphone=='phoneNum')
|| (llelementphone=='CONTACT_PHONE')
|| (llelementphone=='ff_nm_cf_phonetext[]')
|| (llelementphone=='WorkPhone')
) )
{
llformphone = (document.forms[llformlooper2].elements[llelementlooper].value);
if (llfrmid == debugid ) {alert('llformphone:'+llformphone+' llemailfound:'+llemailfound);}
}
If the name
property of the form element is equal to any one of the many many many items in this list, we can then extract the value
and stuff it into a variable. And, since this will almost certainly break all the time, it's got a convenient "set the debugid
and I'll spam alerts as I search the form".
Repeat this for every other field. It ends up being almost 2,000 lines of code, just to select the correct fields out of the forms.
Метки: CodeSOD |
CodeSOD: Exceptionally General |
Andres noticed this pattern showing up in his company's code base, and at first didn't think much of it:
try
{
/*code here*/
}
catch (Exception ex)
{
ExceptionManager.HandleException(ex);
throw ex;
}
It's not uncommon to have some sort of exception handling framework, maybe some standard logging, or the like. And even if you're using that, it may still make sense to re-throw the exception so another layer of the application can also handle it. But there was just something about it that got Andres's attention.
So Andres did what any curious programmer would do: checked the implementation of HandleException
.
public static Exception HandleException(Exception ex)
{
if (ex is ArgumentException)
return new InvalidOperationException("(ExceptionManager) Ocurri'o un error en el argumento.");
if (ex is ArgumentOutOfRangeException)
return new InvalidOperationException("(ExceptionManager) An error ocurred because of an out of range value.");
if (ex is ArgumentNullException)
return new InvalidOperationException("(ExceptionManager) On error ocurred tried to access a null value.");
if (ex is InvalidOperationException)
return new InvalidOperationException("(ExceptionManager) On error ocurred performing an invalid operation.");
if (ex is SmtpException)
return new InvalidOperationException("(ExceptionManager)An error ocurred trying to send an email.");
if (ex is SqlException)
return new InvalidOperationException("(ExceptionManager) An error ocurred accessing data.");
if (ex is IOException)
return new InvalidOperationException("(ExceptionManager) An error ocurred accesing files.");
return new InvalidOperationException("(ExceptionManager) An error ocurred while trying to perform the application.");
}
So, what this code is trying to do is bad: it wants to destroy all the exception information and convert actual meaningful errors into generic InvalidOperationException
s. If this code did what it intended to do, it'd be destroying the backtrace, concealing the origin of the error, and make the application significantly harder to debug.
Fortunately, this code actually doesn't do anything. It constructs the new objects, and return
s them, but that return value isn't consumed, so it just vanishes into the ether. Then our actual exception handler rethrows the original exception.
The old saying is "no harm, no foul", and while this doesn't do any harm, it's definitely quite foul.
Метки: CodeSOD |
Error'd: Punfree Friday |
Today's Error'd submissions are not so much WTF as simply "TF?" Please try to explain the thought process in the comments, if you can.
Plaid-hat hacker Mark writes "Just came across this for a Microsoft Security portal. Still trying to figure it out." Me, I just want to know what happens when you click "Audio".
Reader Wesley faintly damns the sender "Hey, at least they are being honest!" But is this real, or is it a phishing scam? And if it's real phishing, can it really be honest?
Surveyed David misses last week's trivial "None of the above". So do I, David.
Diligently searching, keyboard sleuth Paul T suspects his None key might be somewhere near his Any key, but he can't find that one either.
Finally, an EU resident who wishes to remain anonymous has warned us "Vodafone doesn't allow IT jokes to kids... And they might be right". Where did we go wrong, Vodafone nannies? Was it the C++?
Метки: Error'd |
CodeSOD: A True Leader's Enhancement |
Chuck had some perfectly acceptable C# code running in production. There was nothing terrible about it. It may not be the absolute "best" way to build this logic in terms of being easy to change and maintain in the future, but nothing about it is WTF-y.
if (report.spName == "thisReport" || report.spName == "thatReport")
{
LoadUI1();
}
else if (report.spName == "thirdReport" || report.spName == "thirdReportButMoreSpecific")
{
LoadUI2();
}
else
{
LoadUI3();
}
At worst, we could argue that using string-ly typed logic for deciding the UI state is suboptimal, but this code is hardly "burn it down" bad.
Fortunately, Chuck's team leader didn't like this code. So that team leader "fixed" it.
if ("thisReport, thatReport".Contains(report.spName))
{
LoadUI1();
}
else if ("thirdReport, thirdReportButMoreSpecific".Contains(spName))
{
LoadUI2();
}
else
{
LoadUI3();
}
So we keep the string-ly typed logic, but instead of straight equality comparisons, we change it into a Contains
check. A Contains
check on a string which contains all the possible report names, as a comma separated list. Not only is it less readable, and peforms significantly worse, but if spName
is an invalid value, we might get some fun, unexpected results.
Perhaps the team lead was going for an ["array", "of", "allowed", "names"]
and missed?
The end result though is that this change definitely made the code worse. The team lead, though, doesn't get their code reviewed by their peers. They're the leader, they have no peers, clearly.
https://thedailywtf.com/articles/a-true-leader-s-enhancement
Метки: CodeSOD |
CodeSOD: We All Expire |
Code, like anything else, ages with time. Each minor change we make to a piece of already-in-use software speeds up that process. And while a piece of software can be running for decades unchanged, its utility will still decline over time, as its user interface becomes more distant from common practices, as the requirements drift from their intent, and people forget what the original purpose of certain features even was.
Code ages, but some code is born with an expiration date.
For example, at Jose's company, each year is assigned a letter label. The reasons are obscure, and rooted in somebody's project planning process, but the year 2000 was "A". The year 2001 was "B", and so on. 2025 would be "Z", and then 2026 would roll back over to "A".
At least, that's what the requirement was. What was implemented was a bit different.
if DateTime.Today.year = 2010 then
year = "K"
else if DateTime.Today.year = 2011 then
year = "L"
else if DateTime.Today.year = 2012 then
year = "M"
else if DateTime.Today.year = 2013 then
year = "N"
else if DateTime.Today.year = 2014 then
year = "O"
else if DateTime.Today.year = 2015 then
year = "P"
else if DateTime.Today.year = 2016 then
year = "Q"
else if DateTime.Today.year = 2017 then
year = "R"
else if DateTime.Today.year = 2018 then
year = "S"
else if DateTime.Today.year = 2019 then
year = "T"
else if DateTime.Today.year = 2020 then
year = "U"
else if DateTime.Today.year = 2021 then
year = "V"
else if DateTime.Today.year = 2022 then
year = "W"
else if DateTime.Today.year = 2023 then
year = "X"
else if DateTime.Today.year = 2024 then
year = "Y"
else
year = "Z"
end if
For want of a mod
, 2026 was lost. But hey, this code was clearly written in 2010, which means it will work just fine for a decade and a half. We should all be so lucky.
Метки: CodeSOD |
CodeSOD: He Sed What? |
Today's code is only part of the WTF. The code is bad, it's incorrect, but the mistake is simple and easy to make.
Lowell was recently digging into a broken feature in a legacy C application. The specific error was a failure when invoking a sed
command from inside the application.
// use the following to remove embedded newlines: sed ':a;N;$!ba;s/\n,/,/g'
snprintf(command, sizeof(command),"sed -i ':a;N;$!ba;s/\n,/,/g' %s/%s.txt",path,file);
system(command);
While regular expressions have a reputation for being cryptic, this one is at least easy to read- or at least, easier to read than the pile of sed
flags that precede it. s/\n,/,/g
finds every newline character followed by a comma and replaces it ith just a comma. At least, that was the intent, but there's one problem with that- we're not calling sed
from inside the shell.
We're calling it from C, and C is going to interpret the \n
as a newline itself. The actual command which gets sent to the shell is:
sed -i ':a;N;$!ba;s/
,/,/g' /var/tmp/backup.txt
This completely broke one of the features of this legacy application. Specifically, as you might guess from the shell command above, the backup functionality. The application had the ability to backup its data in a way that would let users revert to prior application states or migrate to other hosts. The commit which introduced the sed
call broke that feature.
In 2018. For nearly three years, all of the customers running this application have been running it without backups.
Lowell sums it up:
The real WTF may be the first part of my reply: "Looks like backup was broken by a commit in December 2018. The 2014 version should work."
Метки: CodeSOD |
CodeSOD: Switching Your Template |
Many years ago, Kari got a job at one of those small companies that lives in the shadow of a university. It was founded by graduates of that university, mostly recruited from that university, and the CEO was a fixture at alumni events.
Kari was a rare hire not from that university, but she knew the school had a reputation for having an excellent software engineering program. She was prepared to be a little behind her fellow employees, skills-wise, but looked forward to catching up.
Kari was unprepared for the kind of code quality these developers produced.
First, let's take a look at how they, as a company standard, leveraged C++ templates. C++ templates are similar (though more complicated) than the generics you find in other languages. Defining a method like void myfunction
creates a function which can be applied to any type, so myfunction(5)
and myfunction("a string")
and myfunction(someClassVariable)
are all valid. The beauty, of course, is that you can write a template method once, but use it in many ways.
Kari provided some generic examples of how her employer leveraged this feature, to give us a sense of what the codebase was like:
enum SomeType
{
SOMETYPE_TYPE1
// ... more types here
};
template
void Function1() ;
template<>
void Function1()
{
// Implementation of Function1 for TYPE1 as a template specialization
}
template<>
void Function1()
{
// Implementation of Function1 for TYPE2 as a template specialization
}
// ... more specializations here
void CallFunction1(SomeType type) {
switch(type)
{
case SOMETYPE_TYPE1:
Function1();
break;
case SOMETYPE_TYPE2:
Function1();
break;
// ... I think you get the picture
default:
assert(false);
break;
}
}
This technique allows them to define multiple versions of a method call Function1
, and then decide which version needs to be invoked by using a type
flag and a switch statement. This simultaneously misses the point of templates and overloading. And honestly, while I'm not sure exactly what business problem they were trying to solve, this is a textbook case for using polymorphism to dispatch calls to concrete implementations via inheritance.
Which raises the question, if this is how they do templates, how do they do inheritance? Oh, you know how they do inheritance.
enum ClassType
{
CLASSTYPE_CHILD1
// ... more enum values here
};
class Parent
{
public:
Parent(ClassType type) : type_(type) { }
ClassType get_type() const { return type_; }
bool IsXYZSupported() const
{
switch(type_)
{
case CHILD1:
return true;
// ... more cases here
default:
assert(false);
return false;
}
}
private:
ClassType type_;
};
class Child1 : public Parent
{
public:
Child1() : Parent(CLASSTYPE_CHILD1) { }
};
// Somewhere else in the application, buried deep within hundreds of lines of obscurity...
bool IsABCSupported(Parent *obj)
{
switch(obj->get_type())
{
case CLASSTYPE_CHILD1:
return true;
// ... more cases here
default:
assert(false);
return false;
}
}
Yes, once again, we have a type flag and a switch statement. Inheritance would do this for us. They're reinvented the wheel, but this time, it's a triangle. An isosceles triangle, at that.
All that's bad, but the thing which elevates this code to transcendentally bad are the locations of the definitions of IsXYZSupported
and IsABCSupported
. IsXYZSupported
is unnecessary, something which shouldn't exist, but at least it's in the definition of the class. Well, it's in the definition of the parent class, which means the parent has to know each of its children, which opens up a whole can of worms regarding fragility. But there are also stray methods like IsABCSupported
, defined someplace else, to do something else, and this means that doing any tampering to the class hierarchy means tracking down possibly hundreds of random methods scattered in the code base.
And, if you're wondering how long these switch statements could get? Kari says: "The record I saw was a switch with approximately 100 cases."
Метки: CodeSOD |
Error'd: Everybody Has A Testing Environment |
“Some people,” said the sage, “are lucky enough to also have a completely separate environment for production.” Today's nuggets of web joy are pudding-proof.
Hypothetically hypochondriac STUDENTS[$RANDOM] gasped “I tried to look up information about Covid tests at the institution. Instead I found…this.”
An anonymous gastronome delivered this tasty morsel with a pun too cheesy to permit in this staid column. “It must have got lost in the mail.”
Hapless hirer Fred G. wonders “Why aren't we getting any resumes?” ruminating “it worked well enough on HR's machine!”
“Wrong.” snapped Scott B. testily.
Armchair analyst David accidentally unmasks this editor's archetype, exclaiming “I didn't even know this was one of the types!”
https://thedailywtf.com/articles/everybody-has-a-testing-environment
Метки: Error'd |
Announcing the launch of TFTs |
NFTs, or non-fungible tokens, are an exciting new application of Blockchain technology that allows us to burn down a rainforest every time we want to trade a string representing an artist's signature on a creative work.
Many folks are eagerly turning JPGs, text files, and even Tweets into NFTs, but since not all of us have a convenient rainforest to destroy, The Daily WTF is happy to offer at alternative, the Totally Fungible Token
A TFT is a unique identifier which we can generate for any file or group of files. It combines the actual data in the file(s) with a Universally Unique Identifier, and then condenses that data using a SHA-256 hashing algorithm. This guarantees that you have a unique token which represents that you have created a unique token for that data.
There are a few key advantages that TFTs offer. First, they're computationally very cheap to make, allowing even a relatively underpowered computer participate actively in the token ecosystem.
In addition, this breaks all dependencies on the blockchain, meaning that you don't need to use or spend cryptocurrency to create, purchase, or trade these tokens.
Most important: much like NFTs, a TFT is absolutely worthless, but we're not promoting these as some sort of arcane investment instrument, so there won't be any sort of bubble. The value of your TFT will remain essentially zero, for the entire life of your TFT. There is no volatility.
In the interests of efficiency, this also performs terribly on large files. How big is too big? That depends on your browser! Enjoy finding out what's too big to encode!
Use the button below to browse for a file on your computer, and this will generate a unique token showing that you generated a unique token. Feel free to share, sell, or trade these tokens with your friends! No information about your files is in the token, so it's guaranteed to be completely meaningless! Give it away, sell it, just write it down on a napkin, your TFT is yours to use as you please!
Token:
Grab the source and tweak the algorithm yourself!. Also, for convenience, there's a dedicated TFT page which, long term, is probably a better tool than this article, since you'll be wanting to play with TFTs for a long time yet, I imagine.
Potential optimizations include: streaming the file conversion so we don't have to have the whole thing in memory, or just replacing all of this with a random number, which would likely be just as good.
The TFT for the tft generator code is tft.js;dbbbf23d24502dba96913f18fa031203977b48db090a4a52ff6258a2c3bceecc
, so enjoy!
https://thedailywtf.com/articles/announcing-the-launch-of-tfts
Метки: Feature Articles |
CodeSOD: A Query About Parsing |
Query strings are, as the name implies, strings. Now, pretty much any web application framework is going to give you convenient access to the query string as a dictionary or map, because at the end of the day, it is just key/value pairs.
But what's the fun in that? Glen's application needed to handle query strings in the form: zones=[pnw,apac]&account_id=55
. Now, enclosing the list in square brackets is a little odd, but actually makes sense when you see how they parse this:
act.zones = account_values.Substring(account_values.IndexOf("zones=") +
"zones=".Length + 1, (
account_values.IndexOf("]",
account_values.IndexOf("zones=") + "zones=".Length + 1) -
(account_values.IndexOf("zones=") + "zones=".Length + 1)
)
).Split(',');
Oh, wait, I'm sorry. Did I say "made sense"? I meant "you can see why they did it, but it's still not a good reason."
This was originally on one line, line-breaks were added to try and make it more readable, and likely failed to do so.
This code takes a substring, first by finding where zones=
appears in the string, then skipping past its length + 1- which skips the first square bracket. Then it grabs until the ]
. But to find the correct ]
, we have another stack of IndexOf
calls to make sure that we start searching after zones=
. Once we've grabbed that whole section, we can finally Split
on commas, and have our lovely list.
Now, unless area names can contain an &
, which I suppose is possible, there's no reason to do the []
wrapper, as we could just grab the index of the &
instead. While we're at it, we could also not write this as a one-liner, and use some variables to cut down on the IndexOf
calls. Of course, there's no reason to do any of this, so maybe instead of thinking up a better version, I should just let it be.
Метки: CodeSOD |
CodeSOD: A Warning Label |
An anti-pattern I've seen too many times is using display text to drive logic. For example, I've see things like:
void btnClick(Object sender, EventArgs evt) {
if (((Button)sender).Text.Contains("Done")) {
…
}
}
That is, of course, bad on its face, an awful sign of other WTFs which may be lurking, and an instant "nope". Today's anonymous submitter inherited some code from an outsourcing company, and they "gracefully" avoided this anti-pattern.
private bool IsDone()
{
if(labelNext.DatabaseKey == "Common.Done")
{
return true;
}
return false;
}
Unlike the obviously terrible version I pseudocoded above, this version at least supports internationalization. In this case, the DatabaseKey
is a tag that can be used to look up a localized text string for the displayed label. So while this is marginally better, it's still driving program logic by looking at what's being displayed by the UI, instead of having the display driven by program logic.
Of course, I'm mostly annoyed because IsDone
, even as implemented, could be a one-liner: return labelNext.DatabaseKey == "Common.Done"
.
Fortunately for our submitter, there's a _done
field that does handle tracking application state. So it was a trivial exercise for our submitter to replace the implementation with:
private bool IsDone()
{
return _done;
}
Метки: CodeSOD |
Reinventing the Wheel |
There are often two types of software development departments mentioned: the kind where software is the product, and the kind where software enhances or sells the product. ChemCo is a third type: a physical chemistry lab, one with extensive customization of lab setups and computer-controlled devices that need to be programmed, as well as a need for statistics and simulations to handle the results. The team includes one C/LabVIEW magician, one Octave specialist, one Java developer, and one Python scripter. Therefore, most of the computer-controlled setups have LabVIEW GUIs and C DLLs for the logic, though some have Python over top of the DLLs instead.
The year is 2016, and ChemCo has just bought a new signal generator. The team's newest engineer, one who knows only Perl, Octave, and R, is tasked with writing a DLL that can be called from their LabView application to control the generator. Faced with learning either C or C++, he chooses C because "the K&R is less than 300 pages while Prata has more than 1200."
The device speaks SCPI, but since ChemCo has a real problem with Not Invented Here syndrome, the new guy writes the library from scratch. During the period that follows, he discovers:
stdio
is unsuitable for serial ports in general because it buffers and tries to seek()
, so the library gets its own I/O layer that speaks native WIN32 or POSIX handles instead of doing fprintf(port, ":system:beeper:state %d", enabled)
.'\0'
byte for its replies, depending on whether it's connected via USB or a Bluetooth serial port, which his code mistook for a string terminator.Soon, our intrepid newbie discovers IUP and writes his own simple GUI for "testing reasons." On the lab's distro of Linux, IUP uses GTK as its backend, which calls setlocale(LC_ALL, "")
on startup. Now all the string formatting is ruined, as the system is set to use a decimal comma instead of a decimal point. So the newbie rewrites his string layer of the library, starting with a temporary setlocale(LC_ALL, "C")
and then back on every string operation, continuing with a hand-grown parser/formatter for numbers, and finally settling on a small piece of C++ code employing std::basic_ios::imbue
.
It's around this time that the LabView magician notices that the generator is designed for high-impedance inputs, meaning it can't even control the low-impedance hardware. The project goes dormant.
The year is now 2020. Not only has the lab gotten some new hardware with high-impedance inputs, there's a grant deadline approaching too fast for comfort, so they need that signal generator up and running yesterday. The LabVIEW magician decides it's easier to reimplement just the parts he needs instead of pulling in the no-longer-newbie's bug-laden mess of a project. He asks a few questions (such as "So it's just strings in both directions?" and "I can't use stdio
?") and sets off working on his own code.
There's no time for testing; it's grant season nownownow, and they need results without a moment to lose. Besides, testing means having test rigs or test hardware, and the only hardware they have in the lab is production hardware. Another LabVIEW monstrosity is built, and a long procedure involving at least 5 different pieces of hardware all from different manufacturers begins. Motors go "wrrrrr", lasers go "click-click", camera goes "huummmmmmmmm". The assembled techs let out their breaths and go to lunch, letting the program run its 30-minute schedule.
When they return, the system is in disarray. The signal generator had apparently stopped responding to commands, left the laser on, and let it cut all the way through the sample. Scratching their heads, they finally determine that the generator returns to normal when something drains the serial port of all pending input—something the newbie's library was doing to determine success, but the experienced tech hadn't bothered with. The magician asks a few more questions ("So it only replies with 'ok\r\n'
or '?x\r\n'
? So it's always four bytes?"), twiddles some code, and declares the problem solved: pending input is drained on startup, and four bytes are read every time a command is sent.
Experienced C devs will have guessed the issue by now, but our techs were too busy high-fiving to notice the problem. They run another test, take another lunch break, and the program fails again, this time further in the sequence—exactly 5 times the number of commands previously run, in fact. After more head-scratching, the newbie takes a look at the hex dump of what the generator sent back and finally notices the problem:
'o' 'k' '\r' '\n' '\0' 'o' 'k' '\r' '\n' '\0' 'o' 'k' '\r' '\n' '\0' 'o' 'k' '\r' '\n' '\0'
That pesky terminating '\0'
was back, and the newbie hadn't thought to warn the magician about it because it had been so long he'd plum forgotten. That wasn't the only problem they had to fix, but it was the most memorable for sure. Nevertheless, they made their grant deadline with half a day to spare, and the work continued on. Never a dull moment when you're working with custom chemistry equipment, that's for sure!
Метки: Feature Articles |
Error'd: Truthiness |
No loops, no branches, barely a pun and almost free of alliteration.
Big-box shopper Worf discovers "This Chromebook's screensaver crash might be Truth In Advertising."
Scott L., who secretly prefers the surprises of online shopping, confesses "when I clicked on Show Shipping Details I was hoping there'd be a tracking number AND it wouldn't be revealed to me. I love suspense!"
Egyptologist Stuart Longland asserts "Jira has apparently abandoned alphabets in favor of hieroglyphs."
An anonymous epistomologist admits "I always mark None of the above when it's the uppermost option. Viva vacuity!"
At rock bottom, nihilist Don M. despairs "I have tried both no and yes but alas IBM remains unhappy with my profile"
Метки: Error'd |
CodeSOD: Constant Success |
Dan was reviewing some PHP code written by a co-worker, as part of taking on a project. The code was in “support” mode, rarely receiving changes, getting bug fixes only when absolutely necessary, and nobody really wanted to be the person responsible for it.
One of those “not absolutely necessary” bugs was that sometimes, it just didn’t save data. The user would enter a product listing, hit save, get a success message back, but the listing wouldn’t actually be saved. No one had really dug into it, because having the end user do double data entry didn’t bother anyone but the end user.
While thinking about that, Dan found this:
if ($query!=false) {
$msg = 'This listing has successfully been saved!';
} else {
//$msg = 'There was an error saving the listing. Please try again later.';
$msg = 'This listing has successfully been saved!';
}
At some point, someone complained about getting too many error messages when saving. So the previous developer “fixed” this, by making sure that they never saw an error message again. This code had been in production for years.
Метки: CodeSOD |
CodeSOD: The Threat of Existence |
Imagine, if you will, that you see a method called FileExists
. It takes a string
input called strPath
, and returns a bool
. Without looking at the implementation, I think you'd have a very good sense of what it's supposed to do. Why, you wouldn't even expect much documentation, because the name and parameters are really clear about what the method actually does.
Unless this method was in the C# codebase "AK" inherited. In that case, the behavior of FileExists
might surprise you:
public bool FileExists(string strPath)
{
try
{
File.WriteAllText(strPath, "blah");
return true;
}
catch (Exception ex)
{/* do nothing */}
return false;
}
This method invokes File.WriteAllText
, which does more or less what you'd expect, unlike this method.
So what this method actually does is overwrite the contents of whatever is in strPath
with "blah"
. If it can do that write, then it returns true. If it can't do that write, perhaps because the directory doesn't exist, or the user doesn't have permission, then it returns false.
Normally, we think of "FileExists" as a check, but in this case, it's more of a threat. By the time this method executes, that file will definitely exist, if it's at all possible to create that file. And if the contents of that file were important before you called this method, well they certainly aren't now.
Метки: CodeSOD |
CodeSOD: For Gotten About Loops |
Today's sample comes from Vasiliy, with no real explanation for where it is, or where it comes from. Frankly though, it doesn't need much setup.
switch (checks) {
case 0:
return new object[] { checkerSummary };
case 1:
return new object[] { "", checkerSummary };
case 2:
return new object[] { "", "", checkerSummary };
case 3:
return new object[] { "", "", "", checkerSummary };
case 4:
return new object[] { "", "", "", "", checkerSummary };
case 5:
return new object[] { "", "", "", "", "", checkerSummary };
case 6:
return new object[] { "", "", "", "", "", "", checkerSummary };
case 7:
return new object[] { "", "", "", "", "", "", "", checkerSummary };
case 8:
return new object[] { "", "", "", "", "", "", "", "", checkerSummary };
case 9:
return new object[] { "", "", "", "", "", "", "", "", "", checkerSummary };
case 10:
return new object[] { "", "", "", "", "", "", "", "", "", "", checkerSummary };
case 11:
return new object[] { "", "", "", "", "", "", "", "", "", "", "", checkerSummary };
case 12:
return new object[] { "", "", "", "", "", "", "", "", "", "", "", "", checkerSummary };
case 13:
return new object[] { "", "", "", "", "", "", "", "", "", "", "", "", "", checkerSummary };
case 14:
return new object[] { "", "", "", "", "", "", "", "", "", "", "", "", "", "", checkerSummary };
case 15:
return new object[] { "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", checkerSummary };
case 16:
return new object[] { "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", checkerSummary };
case 17:
return new object[] { "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", checkerSummary };
case 18:
return new object[] { "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", checkerSummary };
case 19:
return new object[] { "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", checkerSummary };
case default:
return new object[] { checkerSummary };
}
If I were to give the developer responsible the benefit of the doubt, I'd think they just heard about "loop unrolling" and decided that they needed to do that for performance reasons. More likely, they forgot loops existed for a brief moment, or couldn't figure out how to pad an array with empty strings using a loop. Which brings us to another question: why is it so important that this array get padded with checkerSummary
on the end? I don't know, but that strikes me as a code smell hinting at other, deeper WTFs.
Метки: CodeSOD |
News Roundup: UI That Looks Like $900 Million Bucks |
User experience, and its related topic of user interface design, are important. How important? Well the US government’s General Service Administration (GSA) took the time to build a website to explain what it is. What other proof do we need? Well not only did the GSA build a website, but they invested in the SEO necessary to make it top of Google organic search, right below the featured snippet from interaction-design.org.
Which is why the saga and ongoing story of Citibank’s debt repayment blunder all the more amazing. Here’s a quick recap:
A screenshot of the UI:
That shudder you feel? It’s the fear you have from seeing a facsimile of this UI in your office.
It’s hard to feel sad for banks because of the whole 2009 recession thing, but it is interesting that a) Citibank utilized such a confusing UI for something as important as loan payment distribution, b) utilized an OFFSHORE CONTRACTOR to handle such an important piece of work, and c) did not have the proper controls in place to manage such a massive screw up. (I think there’s also some lesson about our country’s overreliance on freelance or contractor work, but that’s probably for another time.) Perhaps when preparing for the Federal Reserve stress tests, they could add in internal process stress tests as well.
This story reminds me of the 38 minutes of terror that residents of Hawaii lived in when they received a push notification that ballistic missiles were on their way, but was instead caused by a push of the wrong button on the Emergency Alert System.
Or the software bug that is causing hundreds of inmates at Arizona prisons convicted of nonviolent offenses who should be released early for completing voluntary classes from getting their due release. The nearly $24 million contract spent on developing the software for managing the prison populations had a known bug that no one seemed to have the budget to fix, even with repeated warnings of a possible issue. With nearly 14k bugs logged and $1,137/hour developer time, I hesitate to be in the backlog prioritization meetings.
These software disasters boil down to 4 software development issues:
Each of these requires an organization which cares about these issues. Sadly, there’s no shortcut for teaching them that, but as the disasters we discussed show, they should care.
Oh, and I know I said I would discuss freelance or contractor work in a future blog post, but definitely don’t solely rely on workers who are not employees of your company, and may not have your company’s best interests in mind, to handle such sensitive tasks without oversight.
In slightly related UI news, try not to use tools that are inserted into the body to reach for that Snickers bar. Or maybe this is just another case of an end user bypassing obstacles in the UI with the tools at their disposal?
https://thedailywtf.com/articles/ui-that-looks-like-900-million-bucks
Метки: News Roundup |
Error'd: {Obscure Reference Here} |
Today's Error'd submissions all center around another common pitfall of the modern web application: failed text substitutions and the ensuing unintentional hilarity.
Antipodean Tony B. sends a Scot sir slur, writing "The mighty Beeb dumbs it down?"
Mind you, when I searched to verify the actor's ID, Google informed me this was an image of "Hair loss — Symptoms and Causes".
Another ailing Scott shares an ominous medical missive, but fortunately "In an unusual outcome, what appeared to be spam was in fact legitimate."
We hope you are well, Scott, and only the doc's spox's sick.
A muffed mail merge miffs Malhar, moaning "Didn't replace the street name in the first line and didn't bother to check before mailers went out. More like Do It Wrong." We are reminded that this pernicious pattern predates the Web, perhaps even paper. We may yet find preserved pressed placeholders among Pompeii's cooked clay.
Clint complains of Spectrum that "I don't even have a payment in process!
And finally, VJ Goh closes out this week's set with an inspiring tale of Canadian heroism. Writes he "This headline really buried the lede. They'd get a lot more clicks if people knew this was about Normal 0 false false false EN-US X-NONE X-NONE A"
Метки: Error'd |
CodeSOD: Reaching for Private Parts |
Jaco was adding some caching to a Java application. Quite wisely, Jaco wrote plenty of tests around his change, ran the test suite, and confirmed everything was green. It ran fine in testing, but when it went to production, everything failed.
Well, as it turned out, the configuration for the production environment loaded slightly different Java classes. One of those "only-loaded-in-production" modules did this:
private static void setRawInstance(final CacheManager cacheManager) {
lock.lock();
try {
final Field field = CacheManager.class.getDeclaredField("singleton");
field.setAccessible(true);
field.set(CacheManager.class, cacheManager);
} catch (Exception e) {
logger.error(e.getMessage(), e);
} finally {
lock.unlock();
}
}
A safe rule of thumb when it comes to using Java's "reflection" features is that you definitely don't use them unless you absolutely need them, and if you think you need them, you're probably wrong. Or, to put it another way: YAGNIEIYTYD. You ain't gonna need it, especially if you think you do.
This code uses reflection to access a private, static property declared on the CacheManager
class. It removes the private
check, using setAccessible
, and then sets the static property equal to the instance cacheManager
passed in.
In other words, this respects your class's privacy about the same amount as the AdTech industry respects yours.
Why was this in production? Why hadn't it broken anybody else's code? How long had this time bomb been sitting out there? That's a mystery.
Jaco sums up this code: "Essentially, the programmer was overwriting the private field of a 3rd party to do his dirty work. Arrogant, dangerous and completely unnecessary."
Метки: CodeSOD |