The Theater of the Mind |
Hamza has some friends in the theater business. These friends had an in-house developed Java application to manage seating arrangements, and they had some problems with it. They had lots of problems with it. So Hamza cut them a deal and agreed to take a look.
There were the usual litany of problems: performance was garbage, features bugged out if you didn’t precisely follow a certain path, it crashed all the time, etc. There was also an important missing feature.
Seats in a theater section lay out roughly in a grid. Roughly- not every row has exactly the same number of seats, sometimes there are gaps in the middle of the row to make way for the requirements of the theater itself, and the application could handle that- for one arrangement. Seats could be removed to create standing-room sections, seats could be added to rows, seats could be reserved for various purposes, and so on. None of this was supported by the application.
Hamza dug into the code which rendered the seating arrangements. Did it read from a database? Did it read from a config file? Did it have a hard-coded array?
for (int r = 0; r < NUMROWS; r++) {
for (int c = 0; c < NUMCOLS; c++) {
Rectangle rec = new Rectangle(24, 24, Color.rgb(204, 0, 0));
fat[r][c] = new Seat();
rec.setStroke(Color.BLACK);
changeRecColor(rec, r, c);
StackPane s = new StackPane();
s.getChildren().add(rec);
gridPane.add(s, c, r);
if (
r == 14 ||
c == 0 ||
c == 1 && r != 23 ||
c == 2 && r != 23 ||
c == 3 && (r != 22 && r != 23) ||
c == 4 && (r != 20 && r != 21 && r != 22 && r != 23) ||
c == 5 && (r != 12 && r != 13 && r != 18 && r != 19 && r != 20 && r != 21 && r != 22 && r != 23) ||
c == 6 && (r != 10 && r != 11 && r != 12 && r != 13 && r != 16 && r != 17 && r != 18 && r != 19 && r != 20 && r != 21 && r != 22 && r != 23) ||
c == 7 && (r != 8 && r != 9 && r != 10 && r != 11 && r != 12 && r != 13 && r != 16 && r != 17 && r != 18 && r != 19 && r != 20 && r != 21 && r != 22 && r != 23) ||
c == 8 && (r == 0 || r == 1 || r == 2 || r == 3 || r == 4 || r == 5 || r == 24) ||
c == 9 && (r == 0 || r == 1 || r == 2 || r == 3 || r == 24) ||
c == 10 && (r == 1 || r == 24) ||
(c == 11 || c == 12 || c == 13) && r == 24 ||
(c == 14 || c == 15 || c == 16) && r == 24 ||
c == 17 ||
c == 18 ||
c == 19 && (r != 0) ||
c == 20 && (r == 15 || r == 23 || r == 24) ||
c == 21 && (r == 23 || r == 24) ||
(c == 22 || c == 23) && (r == 23 || r == 24) ||
(c == 24 || c == 25 || c == 26 || c == 27 || c == 28 || c == 29 || c == 30) && (r == 23 || r == 24) ||
(c == 31 || c == 32 || c == 33 || c == 34 || c == 35) && (r == 23 || r == 24) ||
c == 36 && (r != 0 && r != 16 && r != 17 && r != 18 && r != 19 && r != 20 && r != 21 && r != 22) ||
c == 37 && (r != 23) ||
c == 38 && (r != 23) ||
c == 39 && (r == 15 || r == 16 || r == 17 || r == 18 || r == 19 || r == 20 || r == 21 || r == 22 || r == 24) ||
c == 40 && r == 24 ||
(c == 41 || c == 42 || c == 43 || c == 44) && r == 24 ||
c == 45 && (r == 1 || r == 24) ||
c == 46 && (r < 14 && (r == 0 || r == 1 || r == 2 || r == 3) || r == 24) ||
c == 47 && (r < 14 && (r == 0 || r == 1 || r == 2 || r == 3 || r == 4 || r == 5) || r == 24) ||
c == 48 && (r != 8 && r != 9 && r != 10 && r != 11 && r != 12 && r != 13 && r != 15 && r != 16 && r != 17 && r != 18 && r != 19 && r != 20 && r != 21 && r != 22 && r != 23) ||
c == 49 && (r != 10 && r != 11 && r != 12 && r != 13 && r != 16 && r != 17 && r != 18 && r != 19 && r != 20 && r != 21 && r != 22 && r != 23) ||
c == 50 && (r != 12 && r != 13 && r != 16 && r != 17 && r != 18 && r != 19 && r != 20 && r != 21 && r != 22 && r != 23) ||
c == 51 && (r < 14 || r > 14 && (r == 15 || r == 16 || r == 17 || r == 24)) ||
c == 52 && (r != 20 && r != 21 && r != 22 && r != 23) ||
c == 53 && (r != 22 && r != 23) ||
c == 54 ||
r > 24
) {
rec.setFill(Color.WHITE);
fat[r][c] = null;
}
}
}No, it had a 37 line if condition.
Hamza writes: “Refactoring this application was fun. Not.”
[Advertisement]
Otter - Provision your servers automatically without ever needing to log-in to a command prompt. Get started today!
|
Метки: Feature Articles |
CodeSOD: A Load of ProductCodes |
“Hey, Kim H, can you sit in on a tech-screen for a new hire?”
The Big Boss had a candidate they wanted hired, but before the hiring could actually happen, a token screening process needed to happen. Kim and a few other staffers were pulled in to screen the candidate, and the screen turned into a Blue Screen, because the candidate crashed hard. Everyone in the room gave them a thumbs down, and passed their report up the chain to the Big Boss.
The Big Boss ignored their comments and hired the candidate anyway. A week later, this ended up in source control:
public static ProductCodeModel GetProductCode(int id) {
for (int i = 0; i < GetProductCodes().size(); i++){
if(i==id) return GetProductCodes().get(i);
}
return null;
}Obviously, the loop is unnecessary. The real kicker is that GetProductCodes loads its data out of a file each time it’s called. It contains thousands of lines, which means to access any individual product, the entire file has to be read into memory, id times, and if it's the last product code in the datafile, you have read the file N times.
[Advertisement]
ProGet can centralize your organization's software applications and components to provide uniform access to developers and servers. Check it out!
|
Метки: CodeSOD |
CodeSOD: My Condition is Complicated |
Anneke’s organization is the sort of company where “working” takes precedence over “working well”. Under-staffed, under-budgeted, and under unrealistic deadlines, there simply isn’t any emphasis on code quality. The result is your pretty standard pile of badness: no tests, miles of spaghetti code, fragile features and difficult to modify implementations.
Recently, the powers that be discovered that they could hire half a dozen fresh-out-of-school developers on the cheap, and threw a bunch of fresh-faced kids into that mountain of garbage with no supervision. And that’s how this happened.
XmlNodeList nodeList = NodeListFromSomewhere();
do
{
// A short operation which is very cheap to perform
}
while (index < nodeList.Count
&&
(
(isPolicy
&& (nodeList.Item(index).CloneNode(true).SelectSingleNode(xpathPolicyNo, XmlNsMngr).InnerText.Replace("\n", "").Replace("\t", "").Replace("\r", "") == policyNumberSystem)
&& (nodeList.Item(index).CloneNode(true).SelectSingleNode(xpathDocumentType, XmlNsMngr).InnerText.Replace("\n", "").Replace("\t", "").Replace("\r", "").ToUpper().Trim() != packageContent)
)
||
(!jePolica
&& (nodeList.Item(index).CloneNode(true).SelectSingleNode(xpathOfferNumber, XmlNsMngr).InnerText.Replace("\n", "").Replace("\t", "").Replace("\r", "") == offerNumber)
&& (nodeList.Item(index).CloneNode(true).SelectSingleNode(xpathDocumentType, XmlNsMngr).InnerText.Replace("\n", "").Replace("\t", "").Replace("\r", "").ToUpper().Trim() != packageContent)
&& (nodeList.Item(index).CloneNode(true).SelectSingleNode(xpathPolicyNo, XmlNsMngr).InnerText.Replace("\n", "").Replace("\t", "").Replace("\r", "") == "")
)
)While Anneke hasn’t had a chance to profile this code, they’re almost certain that the body of the loop is cheaper to evaluate than the loop condition, and given the number of regexes chained on, and nodes getting cloned, that’s not implausible. When Anneke found this code, whitespace wasn’t used to make it readable- it was just one extremely long line of code. Anneke’s only contribution to this section of code was to improve the indenting, because as mentioned before: if it works, it ships, and the code, as it stands, works.
[Advertisement]
Forget logs. Next time you're struggling to replicate error, crash and performance issues in your apps - Think Raygun! Installs in minutes. Learn more.
https://thedailywtf.com/articles/my-condition-is-complicated
|
Метки: CodeSOD |
CodeSOD: Eine Kleine ProductListItems |
Art received a job offer that had some generous terms, and during the interview process, there was an ominous sense that the hiring team was absolutely desperate for someone who had done anything software related.
Upon joining the team, Art found out why. Two years ago, someone had decided they needed to create a web-based storefront, and in a fit of NIH syndrome, it needed to be built from scratch. Unfortunately, they didn't have anyone working at the company with a web development background or even a software development background, so they just threw a book on JavaScript at the network admin and hoped for the best.
Two years on, and they didn't have a working storefront. But they did have code like this:
productListItems= function(zweier,controll){
var cartProductListItems = [];
if (zweier=="zweier"){
controll.destroyItems();
for (var y = 0; y <= 999; y=y+2) {
controll.addItem(new sap.ui.core.ListItem({ text: y+2, key: y+2 }));
}
controll.setSelectedKey(2)
}
else if(zweier=="einer"){
controll.destroyItems();
for (var _i = 0; _i <= 999; _i++) {
controll.addItem(new sap.ui.core.ListItem({ text: _i+1, key: _i+1 }));
}
controll.setSelectedKey(1)
}
else{
for (var _i = 0; _i <= 999; _i++) {
cartProductListItems.push(new sap.ui.core.ListItem({ text: _i+1, key: _i+1 }));
}
}
return cartProductListItems;
};
controll is the on-screen control we're populating with list items. zweier controls the skip pattern- if it's "zweier", then skip by two. If it's "einer", skip by one, and if it's neither, populate an array. Return the array, populated or otherwise, at the end.
Now, someone didn't like that function, so they implemented an alternative which takes more parameters:
productListItems2= function(zweier,index,response,model){
var cartProductListItems = [];
if (model!="" && model!=undefined ){
var data = response.oModel.getProperty(response.sPath)
var mindestbestellmenge = data.BOMRABATT;
if (mindestbestellmenge!="" && mindestbestellmenge != undefined){
if (mindestbestellmenge.toString().length == 1){
//do nothing
}else{
mindestbestellmenge=mindestbestellmenge.split(".")[0]
}
}
if (mindestbestellmenge!="1" && mindestbestellmenge!="0" && mindestbestellmenge != undefined && data.VERPACKUNGSEINHEIT=="ZS"){
var mindestbestellmenge = parseInt(mindestbestellmenge);
cartProductListItems.push(new sap.ui.core.ListItem({ text: mindestbestellmenge, key: mindestbestellmenge }));
}else{
cartProductListItems.push(new sap.ui.core.ListItem({ text: 1, key: 1 }));
}
return cartProductListItems
}
else{
if (zweier=="zweier"){
cartProductListItems.push(new sap.ui.core.ListItem({ text: 2, key: 2 }));
}
else if(zweier=="einer"){
cartProductListItems.push(new sap.ui.core.ListItem({ text: 1, key: 1 }));
}
else{
cartProductListItems.push(new sap.ui.core.ListItem({ text: 1, key: 1 }));
}
}
return cartProductListItems;
};
Okay, once again, we do some weird munging to populate a list, but we still have this bizarre zweier variable. Which, by the way, ein is one, zwei is two, so they're obviously using the spelled out version of a number instead of the actual number, and I could do with a little g'suffa at this point.
But you know what? This wasn't enough. They had to add another version of productListItems.
productListItems4 = function( control, min, max, step ){
step = +step || 1;
min = +min ? +min + ( +min % +step ) : +step;
max = +max || 999;
var items = [];
var ListItem = sap.ui.core.ListItem;
var i;
for ( i = min; i <= max; i += step )
items.push( new ListItem({ text: i, key: i }) );
if ( control ) {
control.removeAllItems();
items.forEach( control.addItem.bind( control ) );
control.setSelectedKey( min );
}
return items;
};
This one is kinda okay. I don't love it, but it just about makes sense. But wait a second, why is it productListItems4? What happened to 3?
productListItems3= function(oControll, sLosgroesse){
var anzahl = window.anzahl;
var cartProductListItems = [];
if(oControll){
if(sLosgroesse>1 || anzahl>1){
if (sLosgroesse>1){
oControll.destroyItems();
for (var y = 1; y <= 999; y++) {
oControll.addItem(new sap.ui.core.ListItem({ text: y*sLosgroesse, key: y*sLosgroesse }));
}
oControll.setSelectedKey( sLosgroesse || anzahl || 1 );
// oControll.setSelectedKey(2)
}
if(anzahl)
if(anzahl>1){
oControll.destroyItems();
for (var y = 1; y <= 999; y++) {
oControll.addItem(new sap.ui.core.ListItem({ text: y*anzahl, key: y*anzahl }));
}
oControll.setSelectedKey( sLosgroesse || anzahl || 1 );
// oControll.setSelectedKey(2)
}
}
else{
oControll.destroyItems();
for (var y = 1; y <= 999; y++) {
oControll.addItem(new sap.ui.core.ListItem({ text: y, key: y }));
}
}
}
else{
if(sLosgroesse>1){
for (var _i = 1; _i <= 999; _i++) {
cartProductListItems.push(new sap.ui.core.ListItem({ text: _i*sLosgroesse, key: _i*sLosgroesse }));
}
}
else{
for (var _i = 1; _i <= 999; _i++) {
cartProductListItems.push(new sap.ui.core.ListItem({ text: _i, key: _i }));
}
}
}
return cartProductListItems;
};
Oh. There it is. I'm sorry I asked. Nice use of the anzahl global variable. Some global variables are exactly what this needed. In this case, it holds the skip number again, so a riff on einer and zweier but without spelling things out.
A quick search for calls to any variation of productListItems shows that there are 2,000 different invocations to one of these methods, and a sampling shows that it's a little of all of them, depending on the relative age of a given code module.
[Advertisement]
Ensure your software is built only once and then deployed consistently across environments, by packaging your applications and components. Learn how today!
https://thedailywtf.com/articles/eine-kleine-productlistitems
|
Метки: CodeSOD |
Error'd: Latin is Making a Comeback? |
"Well, if I need an email template, lucky me, I now have one handy," writes Paul C.
"I was shopping around for refrigerators and after seeing this, I would definitely take delivery on the $19.99 model!" writes
Ryan L. wrote, "I should probably consider renewing the warranty, but I have %days% to think about it."
"Thus are the side effects of having a currency converter extension installed," Shahim M. writes.
Yuna M. wrote, "Ever check news sites sometime between 4:00 AM and when people start getting in to work? You may see some weird stuff."
"'Yes' means yes, and 'Print Info' means yes," writes Michael R. ,"Immediately after selecting 'Print Info' I received an SMS text from CVS, and then a comically small receipt printed with opt-out instructions."
[Advertisement]
ProGet supports your applications, Docker containers, and third-party packages, allowing you to enforce quality standards across all components. Download and see how!
|
Метки: Error'd |
CodeSOD: Boldly Leaping Over the Span |
No one writes HTML anymore. We haven’t for years. These days, your HTML is snippets and components, templates and widgets. Someplace in your application chain, whether server-side or client-side, or even as part of a deployment step, if you’re using a static site generator, some code mashes those templates together and you get some output.
This has some side effects, like div abuse. Each component needs its own container tag, but we often nest components inside each other. Maybe there’s a span in there. If the application is suitably HTML5-y, maybe it’s sections instead.
Andy stumbled across a site which was blocking right clicking, so Andy did what any of us would do: pulled up the debugging tools and started exploring the DOM tree.
Welcome to the [redacted]
Maybe this is a chain of components, maybe it’s a runaway loop, maybe it’s just stupid.
With all those font-weight directives stacked up there, I’m tempted to say that’s mighty bold, but with a font-weight: 400, that’s honestly not bold at all. Well, it’s a bold use of span tags, I suppose.
[Advertisement]
Ensure your software is built only once and then deployed consistently across environments, by packaging your applications and components. Learn how today!
https://thedailywtf.com/articles/boldly-leaping-over-the-span
|
Метки: CodeSOD |
CodeSOD: Round Two |
John works for a manufacturing company which has accrued a large portfolio of C++ code. Developed over the course of decades, by many people, there’s more than a little legacy cruft and coding horrors mixed in. Frustrated with the ongoing maintenance, and in the interests of “modernization”, John was tasked with converting the legacy C++ into C#.
Which meant he had to read through the legacy C++.
In the section for creating TPS reports, there were two functions, TpsRound and TpsRound2. The code between the two of them was nearly identical- someone had clearly copy/pasted and made minor tweaks.
CString EDITFORM::TpsRound2(double dbIntoConvert)
{
// This Stub calculates the rounding based
// upon this company standards as
// outlined in TPS 101 Conversion Rules and
// dual dimensioning practices
CString csHold1,csHold2;
int decimal, sign,ChkDigit;
long OutVal;
char *buffer2;
char *outbuff;
outbuff = " ";
buffer2 = " ";
if (dbIntoConvert == 0)
{
// return CString("0.00");
}
buffer2 = _fcvt( dbIntoConvert, 7, &decimal, &sign );
buffer2[decimal] = '.';
csHold2 = XvertDecValues(dbIntoConvert);
if (m_round == FALSE)
{
return csHold2;
}
ChkDigit = atoi(csHold2.Mid(2,1));
OutVal = atol(csHold2.Left(2));
if (ChkDigit >= 5)
{
OutVal++;
}
if (OutVal >= 100)
{
buffer2[decimal] = '0';
decimal++;
buffer2[decimal] = '.';
}
ltoa(OutVal,outbuff,10);
buffer2[0]=outbuff[0];
buffer2[1]=outbuff[1];
int jj=2; // this value is the ONLY difference to `TpsRound()`
while (jj < decimal)
{
buffer2[jj++]='0';
}
csHold1 = CString(buffer2).Left(decimal);
return csHold1;
}At its core, this is just string-mangling its way through some basic rounding operations. Writing your own C++ rounding is less of a WTF than it might seem, simply because C++ didn’t include standard rounding methods for most of its history. The expectation was that you’d implement it yourself, for your specific cases, as there’s no one “right” way to round.
This, however, is obviously the wrong way. The code is actually pretty simple, just cluttered with a mix of terrible variable names, loads of string conversion calls, and a thick layer of not understanding what they’re doing.
To add to the confusion, buffer2 holds the results of _fcvt- a method which converts a floating point to a string. csHold2 holds the results of XvertDecValues, which also returns a floating point converted to a string, just… a little differently.
CString EDITFORM::XvertDecValues(double incon1)
{
char *buffer3;
char *outbuff;
int decimal, sign;
buffer3 = " ";
outbuff = "00000000000000000000";
buffer3 = _fcvt(incon1, 7, &decimal, &sign );
if (incon1 == 0)
{
// return CString("0.00");
}
int cnt1,cnt2,cnt3;
cnt1 = 0;
cnt2 = 0;
cnt3 = decimal;
if (cnt3 <= 0)
{
outbuff[cnt2++]='.';
while (cnt3 < 0)
{
outbuff[cnt2++]='0';
cnt3++;
}
}
else
{
while (cnt1 < decimal)
{
outbuff[cnt2] = buffer3[cnt1];
cnt1++;
cnt2++;
}
outbuff[cnt2] = '.';
cnt2++;
}
while (cnt1 < 15)
{
outbuff[cnt2] = buffer3[cnt1];
cnt1++;
cnt2++;
}
outbuff[cnt2] = '\0';
return CString(outbuff);
}So, back in TpsRound2, csHold2 and buffer2 both hold a string version of a floating point number, but they both do it differently. They’re both referenced. Note also the check if (OutVal >= 100)- OutVal holds the leftmost two characters of csHold2- so it will never be greater than or equal to 100.
John’s orders were to do a 1-to–1 port of functionality. “Get it working in C#, then we can refactor.” So John did. And then once it was working in C#, he threw all this code away and replaced it with calls to C#’- built in rounding and string formatting methods, which were perfectly fine for their actual problems.
[Advertisement]
ProGet can centralize your organization's software applications and components to provide uniform access to developers and servers. Check it out!
|
Метки: CodeSOD |
CodeSOD: Tern The Bool Around |
Some say that the only reason I like ternary code snippets is that it gives me an opportunity to make the title a “tern” pun.
…
They’re not wrong.
I’m actually a defender of ternaries. Just last week, I wrote this line of C++ code:
ControllerState response = allMotorsIdle() ? READY : NOT_READY;That's a good use of ternaries, in my opinion. It's a clear translation- if all motors are idle, we're ready, otherwise we're not, keep waiting. Simple, easy to read, and turns what really is one idea (if we're idle, we're ready) into one line of code. Ternaries can make code more clear.
Which brings us to this anonymous submission.
disableEstimateSent: function () {
let surveyCompleted = (modalControl.survey[0].Survey_Complete == undefined || modalControl.survey[0].Survey_Complete == false) ? false : true;
return !surveyCompleted;
}
This is the perfect storm of bad choices. First, we have a long, complex expression in our ternary. I mean, not all that long, it’s only got two clauses, but boy howdy are we digging down the object graph. For the same object. Twice. An object which is either false or undefined, which in JavaScript booleans, undefined is also falsy. So if Survey_Complete is falsy, we store false in surveyCompleted… and then return !surveyCompleted.
Extra variables, double negatives, ugly ternaries. This is a work of art.
Our anonymous submitter of course went all Banksy and shredded this work of art and replaced it with a more prosaic return !modalControl.survey[0].Survey_Complete.
[Advertisement]
Utilize BuildMaster to release your software with confidence, at the pace your business demands. Download today!
|
Метки: CodeSOD |
A Floating Date |
Enterprise integration is its own torturous brand of software development. Imagine all the pain of inheriting someone else's code, but now that code is proprietary, you can't modify it, poorly documented, and exposes an API that might solve somebody's problem, but none of the problems you have, and did I say poorly documented? I meant "the documentation is completely inaccurate and it's possible that this was intentional".
Michael was working on getting SAP integrated to their existing legacy systems. This meant huge piles of bulk data loading, which wasn't so bad- they had a third party module which promised to glue all this stuff together. And in early testing phases, everything went perfectly smooth.
Of course, this was a massive enterprise integration project for a massive company. That guaranteed a few problems that were unavoidable. First, there were little teams within business units who weren't using the documented processes in the first place, but had their own home-grown process, usually implemented in an Excel file on a network drive, to do their work. Tracking these down, prying the Excel sheet out of their hands, and then dealing with the fallout of "corporate coming in and changing our processes for no reason" extended the project timeline.
Well, it extended how much time the project actually needed, which brings us to the second guaranteed problem: the timeline was set based on what management wanted to have happen, not based on what was actually possible or practical. No one on the technical side of things was consulted to give an estimate about required effort. A go-live date of October 8th was set, and everything was going to happen on October 8th- or else.
The project was, against all odds, on track to hit the ridiculous target. Until it went into UAT- and that's when Michael started catching issues from users. Dates were shifting. In the source system, the date might be November 21st, but in SAP it was November 20th. The 23rd turned into the 24th. The 25th also turned into the 24th.
Michael was under a time crunch, and trapped between a rock (the obtuse legacy system), a hard place (SAP), and a hydraulic press (the third-party data import module). There was a pattern to the errors, though, and that pattern pointed to a rounding error.
"Wait, a rounding error?" Michael wondered aloud. Now, they did use numbers to represent dates. The "Japanese" notation, which allowed them to store "November 21st, 2018" as 20181121. That's a super common approach to encoding a date as a 32-bit integer. As integers, of course, there was no rounding. They were integers on the legacy side, they were integers on the SAP side- but what about in the middle? What was the third party import module doing?
As a test, Michael whipped up a little two-line program to test:
float _date = Integer.parseInt("20181121");
System.out.println((int)_date);
//Outputs: 20181120
Of course. This is a standard feature of IEEE floating point arithmetic. This hadn't been happening in early testing because they safely avoided date/numbers "large" enough. It was only when they moved into UAT and started using real data that the bug became apparent. For some reason, the data import module was passing integer data straight through floats, probably out of a misguided attempt to be "generic".
Michael raised the issue with the vendor, suggested that the vendor should check for casts to float, and pointed out that he was under an extreme time crunch. The vendor, to their credit, tracked down the bug and had a patched version to Michael within two days.
Working in the enterprise space, Michael has seen too many applications which store currency values as floats, leading to all sorts of accounting-related messes. This, however, is the only time he's seen that happen with dates.
[Advertisement]
Ensure your software is built only once and then deployed consistently across environments, by packaging your applications and components. Learn how today!
|
Метки: Feature Articles |
Error'd: Let's Hope it's Only a Test |
"When the notification system about the broken NYC MTA is broken, does that make the MTA meta-broken?" writes T.S.
"I must be 'Y' years old by now, right?" writes Louis I.
Josh W. wrote, "The fact that Fifth Third Bank handles money should be able to handle numbers correctly."
"I received this sextortion email last night, but I'm guessing the spammer hopes I have my own spinning software in order to decipher it!" Nigel writes.
Romaji A. wrote, "Maybe label printers print get bored of printing bar codes and it makes them go a little...crazy?"
"The Quantas in flight app is proud to announce that your plane's warp drives are fully activated," writes Tony B.
[Advertisement]
Continuously monitor your servers for configuration changes, and report when there's configuration drift. Get started with Otter today!
https://thedailywtf.com/articles/let-s-hope-it-s-only-a-test
|
Метки: Error'd |
CodeSOD: Break Out of your Parents |
When I first glanced at this submission from Thomas, I almost just scrolled right by. “Oh, it’s just another case where they put the same code in both branches of the conditional,” I said. Then I looked again.
if (obj.success) {
//#BZ7350
for (i = 0; i < parent.length; i++) {
try {
parent[i]['compositionExportResultMessage'](obj.success, obj.response, 'info');
break;
} catch (e) { }
}
}
else {
//#BZ7350
for (i = 0; i < parent.length; i++) {
try {
parent[i]['compositionExportResultMessage'](obj.success, obj.response, 'error');
break;
} catch (e) { }
}
}First, I want to give a little shout out to my “favorite” kind of ticket management: attaching a ticket number to a comment in your code. I can’t be certain, but I assume that’s what //#BZ7350 is doing in there, anyway. I’ve had the misfortune of working in places that required that, and explaining, “I can just put it in my source control comments,” couldn’t shift company policy.
Now, this is a case where nearly identical code is executed in either branch. The key difference is whether you pass info or error up to your parent.
Which… parent is apparently an array, which means it should at least be called parents, but either way that ends up being a weird choice. Sure, there are data structures where children can have multiple parents, but how often do you see them? Especially in web programming, which this appears to be, which mostly uses trees.
But fine, a child might have multiple parents. Those parents may or may not implement a compositionExportResultMessage, and if they do, it may or may not throw an exception when called. We want hopefully one parent to handle it, so take a close look at the loop.
We try to call compositionExportResultMessage on our parent. If it’s successful, we break. If it fails, we throw the exception away and try with the next parent in our array. Repeat until every parent has failed, or one has succeeded.
Thomas didn’t provide much context here, but I’m not sure how much we actually need. Something is really wrong in this code base, and based on how ticket #BZ7350 was closed, I don’t think it’s gonna get any better.
[Advertisement]
ProGet can centralize your organization's software applications and components to provide uniform access to developers and servers. Check it out!
|
Метки: CodeSOD |
Blind Leading the Blind |
Corporate Standards. You know, all those rules created over time by bureaucrats who think that they're making things better by mandating consistency. The ones that force you to take time to change an otherwise properly-functioning system to comply with rules that don't really apply in the context of the application, but need to be blindly followed anyway. Here are a couple of good examples.
Kevin L. worked on an application that provides driving directions via device-hosted map application. The device was designed to be bolted to the handlebars of a motorcycle. Based upon your destination and current coordinates, it would display your location and the marked route, noting things like distance to destination, turns, traffic circles and exit ramps. A great deal of effort was put into the visual design, because even though the device *could* provide audio feedback, on a motorcycle, it was impossible to hear.
One day, his boss, John, called him into a meeting. "I was just read the riot-act by HR. It seems that our application doesn't comply with corporate Accessibility Standards, specifically the standard regarding Braille Literature In Need of Description. You need to add screenreader support to the motorcycle map application. I estimate that it will take a few months of effort. We don't really have the time to spare, but we have to do it!"
Kevin thought about it for a bit and asked his boss if the company really wanted him to spend time to create functionality to provide verbal driving directions for blind motorcycle drivers.
That head-desk moment you're imagining really happened.
Of course, common sense had no bearing on the outcome, and poor Kevin had to do the work anyway.
While self-driving cars will eventually be commonplace, and no one will need directions, audible or otherwise. For now, though, Kevin at least knows that all the visually impaired motorcycle drivers can get to where they're going.
[Advertisement]
Continuously monitor your servers for configuration changes, and report when there's configuration drift. Get started with Otter today!
|
Метки: Feature Articles |
CodeSOD: An Error on Logging |
The beauty of a good logging system is that it allows you to spam logging messages all through your code, but then set the logging level at runtime, so that you have fine grained control over how much logging there is. You can turn the dial from, “things are running smooth in production, so be quiet,” to “WTF THINGS ARE ON FIRE GODS HELP US WHAT IS GOING ON CAN I LAUNCH A DEBUGGER ON THE PRODUCTION ENVIRONMENT PLEASE GOD”.
You might write something like this, for example:
LOG.error("Error generating file {}", getFileName(), e);This leverages the logging framework- if error logging is enabled, the message gets logged, otherwise the message is dropped. The string is autoformatted, replacing the {} with the results of getFileName().
That’s the code Graham wrote. Graham replaced this code, from another programmer who maybe didn’t fully grasp what they were doing:
if (LOG.isErrorEnabled()) {
LOG.error(String.format("Generating " + getFileName() + " :%s", e));
}There’s a pile of poorly understood things happening here. As stated, if isErrorEnabled is false, LOG.error doesn’t do anything. It also can handle string formatting, which isn’t what happens here, and what does happen here demonstrates a complete misunderstanding of what String.format does, as it both formats and concatenates.
Then Graham searched through that other programmer’s commits, only to find this basic pattern copy/pasted in everywhere, usually with inconsistent indentation. At least it was actual logging, and not a pile of System.out.printlns.
As soon as that though popped into his head, Graham searched again. There were a lot of System.out.printlns too.
[Advertisement]
Utilize BuildMaster to release your software with confidence, at the pace your business demands. Download today!
|
Метки: CodeSOD |
CodeSOD: Pointed Array Access |
I've spent the past week doing a lot of embedded programming, and for me, this has mostly been handling having full-duplex communication between twenty devices on the same serial bus. It also means getting raw bytes and doing the memcpy(&myMessageStructVariable, buffer, sizeof(MessageStruct)). Yes, that's not the best way, and certainly isn't how I'd build it if I didn't have full control over both ends of the network.
Of course, even with that, serial networks can have some noise and errors. That means sometimes I get a packet that isn't the right size, and memcpy will happily read past the end of the buffer, because my const uint8_t * buffer pointer is just a pointer, after all. It's on me to access memory safely. Errors result when I'm incautious.
Which brings us to Krzysztof`s submission. This code is deep inside of a device that's been on the market for twenty years, and has undergone many revisions, both hardware and software, over that time.
uint32_t tempHistVal1;
uint32_t tempHistVal2;
uint32_t tempHistVal3;
...
uint32_t tempHistVal20;
uint32_t get_avg_temp(){
uint32_t res=0;
uint32_t *ptr=&tempHistVal1;
for(uint32_t i=0;i<20;i++)
res+=ptr[i];
return res/20;
}
After the first few lines, you'll probably think to yourself, "that should probably be an array, shouldn't it?" Of course it should. The programmer who wrote this agrees with you.
The line uint32_t *ptr=&tempHistVal1 creates a pointer to the first variable. In C, the line between "pointer" and "array" is fuzzy, which means you can use the [] operator to "index" a pointer. So, the line res+=ptr[i] grabs the i-th 32-bit integer after the ptrs address.
Now it's likely that tempHistVal1 and tempHistVal2 are contiguous in memory. It's the most obvious way for the compiler to handle those variables. But there's no guarantee that's the case. The C specification guarantees that arrays represent contiguous blocks of memory, but not variables.
Krzysztof suggested that they change this to an array, but was shot down. "We don't change code that works!", management said. Krzysztof is left to prayto the gods of compilers and hardware platforms and memory alignment that these variables keep getting compiled in order, with no gaps.
[Advertisement]
ProGet can centralize your organization's software applications and components to provide uniform access to developers and servers. Check it out!
|
Метки: CodeSOD |
Error'd: Full Price not Allowed |
"When registering for KubeCon and CloudNativeCon, it's like they're saying: Pay full price? Oh no, we insist you use a discount code. No really. It's mandatory," writes Andy B.
Henry S. wrote, "I think this message should perhaps read Luxury Service Unavailable."
"At first glance, you may read the instruction to be 'check your dog file', but that is presently not the case," writes Daryl D.
Rich P. wrote, "Lite-On (a LED manufacturer located in Taiwan) seems to have given up on differentiating the countries across the Pacific..."
"Sorry glassdoor, I would be happy to leave a salary report for undefined, but my current contracts with null and NaN forbid me from doing so," writes Jeffrey King.
"You know, although my name is Bruce, my friends all call me undefined," Bruce R. wrote.
[Advertisement]
Ensure your software is built only once and then deployed consistently across environments, by packaging your applications and components. Learn how today!
|
Метки: Error'd |
CodeSOD: Off by Dumb Error |
“We’re bringing on my nephew, he’s super smart with computers, so you make sure he is successful!”
That was the long and short of how Reagan got introduced to the new hire, Dewey. Dewey’s keyboard only really needed three keys: CTRL, C, and V. They couldn’t write a line of code to save their life. Once, when trying to fumble through a FizzBuzz as a simple practice exercise, Dewey took to Google to find a solution. Because Dewey couldn’t quite understand how Google worked, instead of copy/pasting out of StackOverflow, they went to r/ProgrammerHumor and copied code out of a meme image instead.
Reagan couldn’t even just try and shove Dewey off on a hunt for a left-handed packet shifter in the supply closet, because Dewey’s patron was watching source control, and wanted to see Dewey’s brilliant commits showing up. Even if Reagan didn’t give Dewey any tasks, Dewey’s uncle did.
That’s how Dewey got stumped trying to fetch data from a database. They simply needed to read one column and present it as a series of HTML list items, using PHP.
This was their approach.
$sql = "SELECT information FROM table";
//yes, that is actually what Dewey named things in the DB
$result = $conn->query($sql);
$list = $result->fetch_assoc();
$i = 1;
$run = true;
while ( $list == true && $run != false ) {
if ( $list[$i] <= count($list) ) {
echo '' . $list[$i] . ' ';
$i++;
} else {
$last = array_pop(array_reverse($list));
echo '' . $last . ' ';
$run = false;
}
}Presumably, this is one of the cases where Dewey didn’t copy and paste code, because I don’t think anyone could come up with code like that on purpose.
The fundamental misunderstanding of loops, lists, conditions, arrays, and databases is just stunning. Somehow, Dewey couldn’t grasp that arrays started at zero, but blundered into a solution where they could reverse and pop the array instead.
Needless to say, Dewey never actually had any code get past the review stage. Shortly after this, Dewey got quietly shuffled to some other part of the organization, and Reagan never heard from them again.
[Advertisement]
ProGet supports your applications, Docker containers, and third-party packages, allowing you to enforce quality standards across all components. Download and see how!
|
Метки: CodeSOD |
CodeSOD: Ten Times as Unique |
James works with a financial services company. As part of their security model, they send out verification codes for certain account operations, and these have to be unique.
So you know what happens. Someone wrote their own random string generator, then wrapped it up into a for loop and calls it until they get a random string which is unique:
private string GetUniqueVerificationCode()
{
// Generate a new code up to 10 times and check for uniqueness - if it's unique jump out
// IRL this should only hit once, it;s a random 25 char string ffs but you can never be too careful :)
for(var tries = 0; tries < 10; tries++)
{
var code = RandomStringGenerator.GetRandomAlphanumericString(50);
if(!this.userVerificationCodeRepository.CodeExists(code))
{
return code;
}
}
throw new Exception("Unable to generate unique verification code.");
}It’s the details, here. According to the comment, we expect 25 characters, but according to the call, it looks like it’s actually 50- GetRandomAlphanumericString(50). If, after ten tries, there isn’t a unique and random code, give up and chuck an exception- an untyped exception, making it essentially impossible to catch and respond to in a useful way.
As the comment points out- the odds of a collision are exceedingly small- at least depending on how the “random alphanumeric string” is generated. Even with case insensitive “alphanumerics”, there are quadrillions of possible strings at twenty five characters. If it’s actually fifty, well, it’s a lot.
Now, sure, maybe there’s a bias in the random generation, making collisions more likely, but that’s why we try and design our applications to avoid generating the random numbers ourselves.
James pointed out that this was silly, but the original developer misunderstood, and thought the for loop was the silly part, so now the code looks like this:
private string GetUniqueVerificationCode()
{
var code = RandomStringGenerator.GetRandomAlphanumericString(50);
while (this.userVerificationCodeRepository.CodeExists(code))
{
code = RandomStringGenerator.GetRandomAlphanumericString(50);
}
return code;
}Might as well have gone all the way to a do...while. The best part is that regardless of which version of the code you use, since it’s part of a multiuser web application, there’s a race condition- the same code could be generated twice before being logged as an existing code in the database. That’s arguably more likely, depending on how the random generation is implemented.
Based on a little googling, I suspect that the GetRandomAlphanumericString was copy-pasted from StackOverflow, and I’m gonna bet it wasn’t one of the solutions that used a cryptographic source.
[Advertisement]
Forget logs. Next time you're struggling to replicate error, crash and performance issues in your apps - Think Raygun! Installs in minutes. Learn more.
|
Метки: CodeSOD |
CodeSOD: The UI Annoyance |
Daniel has a bit of a story. The story starts many months ago, on the very first day of the month.
Angular 1.x has something called a filter as a key concept. This is a delightfully misleading name, as it's more meant to be used as a formatting function, but because it takes any arbitrary input and converts it to any arbitrary output, people did use it to filter, which had all sorts of delightful performance problems in practice.
Well, Daniel found this perfectly sensible formatting filter. It's well documented. It's also wrong.
/**
* Given a timestamp in the format "2018-06-22T14:55:44+00:00", this filter
* returns a date in human-readable format following our style guide.
* Assuming the browser's timezone is EDT, the filter applied to the above string
* would return "Jun 22, 2018 10:55 AM".
*
When applicable, this filter returns "today at" or "yesterday at" as date abbreviations in lowercase.
* These can be capitalized using the "capitalize" filter above directly in an HTML file.
*/
ourApp.filter('ourTimestamp', ['$filter', function($filter) {
return function(timestamp) {
// Guard statement for when timestamp is null, undefined or empty string.
if (!timestamp) {
return '';
}
let TODAY = new Date();
let TODAY_YEAR = TODAY.getFullYear();
let TODAY_MONTH = TODAY.getMonth();
let TODAY_DAY = TODAY.getDate();
let TIMESTAMP_FORMAT = 'MMM d, y h:mm a';
let TIME_FORMAT = 'h:mm a';
let originalTimestampDate = new Date(timestamp);
let year = originalTimestampDate.getFullYear();
let month = originalTimestampDate.getMonth();
let day = originalTimestampDate.getDate();
let dateAbbreviation = null;
if (year === TODAY_YEAR && month === TODAY_MONTH && day === TODAY_DAY) {
dateAbbreviation = 'today at ';
} else if (year === TODAY_YEAR && month === TODAY_MONTH && day === (TODAY_DAY - 1)) {
dateAbbreviation = 'yesterday at ';
}
if (dateAbbreviation) {
return dateAbbreviation + $filter('date')(timestamp, TIME_FORMAT);
} else {
return $filter('date')(timestamp, TIMESTAMP_FORMAT);
}
};
This code, like so much bad code, touches dates. This time, its goal is to output a more friendly date- like, if an event happened today, it simply says, "today at" or if it happened yesterday, it says "yesterday at". On the first day of the month, this fails to output "yesterday at". The bug is simple to spot:
if (year === TODAY_YEAR && month === TODAY_MONTH && day === (TODAY_DAY - 1)) {
dateAbbreviation = 'yesterday at ';
}
On September first, this only outputs "yesterday at" for September zeroth, not August 31st.
Now, that's a simple brainfart bug, and it could be fixed quite easily, and there are many libraries which could be used. But Daniel ran a git blame to see who on the development team was responsible... only to find that it was nobody on the development team.
It probably isn't much of a shock to learn that this particular application has lots of little UI annoyances. There's a product backlog a mile long with all sorts of little things that could be better, but can be lived with, for now. Because it's a mile of things that can be lived with, they keep getting pushed behind things that are more serious, necessary, or just have someone screaming more loudly about them.
Sprint after sprint, the little UI annoyances keep sitting on the backlog. There's always another problem, another fire to put out, another new feature which needs to be there. The CTO kept trying to raise the priority of the little annoyances, and the line kept getting jumped. So the CTO just took matters into their own hands and put this patch into the codebase, and pushed through to release. As the CTO, they bypassed all the regular sign-off procedures. "The test suite passes, what could be wrong?"
Of course, it has its own little UI annoyance, in that it misbehaves on the first day of the month. The test suite, on the other hand, assumes that the code will run as intended. And the test suite actually uses the current date (and calculates yesterday using date arithmetic). Which means on the first day of the month, the test fails, breaking the build.
Unfortunately for Daniel and the CTO, this bug ended up on the backlog. Since it only impacts developers one day a month, and since it's pretty much invisible to the users, it's got a very low priority. It might get fixed, someday.
[Advertisement]
BuildMaster allows you to create a self-service release management platform that allows different teams to manage their applications. Explore how!
|
Метки: CodeSOD |
CodeSOD: Shell Out |
Developers sometimes fail to appreciate how difficult a job Operations really is. In companies that don't hold with newfangled DevOps, the division of labor often comes with a division of reputation as well. After all, developers do the hard work of making software. What are Ops guys even for? They don't make software. They don't generate leads or fix your desktop PC. Why bother paying for talented senior Ops professionals?
Spend a few days with the Ops team, however, and you start to see why you should pay them a little more than your average garbageman. The Ops lifecycle is a daily grind of deployments, patching, and sticking fingers in dykes, trying to keep that expensive cesspit the devs call "software" running. Simple tasks such as spinning up new infrastructure in AWS often get pushed to the back burner behind putting out fires and making sure critical maintenance tasks that didn't get done last year don't explode into flames.
Still, companies like to cut corners. Often, Ops folks have very little programming expertise and no training budget, meaning repetitive tasks are automated using cobbled-together bits of shell script found via Google. In the Ops world, a bit of Perl or Python is worth its weight in gold.
Today's snippet, as you can probably guess, is not in Perl or Python. It is instead in a common paradigm: Bash embedded in Perl. Likely, the original script was written by a senior who knows Perl, and this chunk was written by a strapped-for-time medior who didn't:
my $secs = `cut -f1 -d. /proc/uptime`;
$data{lastboottime} = strip(`date -d "$secs seconds ago" '+%Y'-'%m'-'%d'T'%T' 2>/dev/null`);
The point of this snippet is to gather the last time the machine booted; later code sends it to a central inventory system. The bug here is that the last boot time would drift by a second or so between updates—not because the machine had rebooted, but because the code gathering it was imprecise.
For those who spend their day at a higher level of abstraction, let me explain: we start by querying /proc/uptime, which I'll let the manpage explain:
This file contains information detailing how long the system has been on since its last restart. The output of /proc/uptime is quite minimal:
350735.47 234388.90
The first number is the total number of seconds the system has been up. The second number is how much of that time the machine has spent idle, in seconds.
We then use cut to snip the output, using a period as a delimiter and taking only the first field, meaning we take the floor of the uptime in seconds and throw away the rest. We store that in a Perl variable, then feed it back into Bash in the middle of a date format string so that it reads "X seconds ago." We then parse that, rearrange it into year-month-day, throw away any errors, and trim it to put back into Perl for the rest of the script to forward on.
Some days I feel this is the real reason DevOps was invented: a bunch of devs saw the code the Ops guys were writing and cringed so hard, they found themselves volunteering to write "Whatever code you need, man. Just ask me, I'll get it done for you. Please."
[Advertisement]
ProGet supports your applications, Docker containers, and third-party packages, allowing you to enforce quality standards across all components. Download and see how!
|
Метки: CodeSOD |
Error'd: This Movie is Rated S for Safe for SQL |
"Clearly the Light Cinema decided to ban unsafe sql characters from the cinema," wrote Simon, "Let's hope no one makes a film called 'Drop Table'."
Michael M. wrote, "King Soopers has an amazing algorithm when deciding just what tea to show me from their extensive database."
"I didn't know that there was a city named after a Java conversion error in zip code 85034, but if I want to go, hey, Greyhound can take me there," Celti B. writes.
"Even estimates.solar's site is weighing in on the current political climate!" wrote Russ S.
Derrick M. writes, "For a wireless keyboard to cost this much, it had better be able to reach into space."
"According to Microsoft, 13/100 is the new 12%," Roberta wrote.
[Advertisement]
Otter - Provision your servers automatically without ever needing to log-in to a command prompt. Get started today!
https://thedailywtf.com/articles/this-movie-is-rated-s-for-safe-for-sql
|
Метки: Error'd |
