CodeSOD: Hanging By a Thread |
Wojciech has to maintain a C++ service that, among other things, manages a large pool of TCP sockets. These sockets are, as one might expect, for asynchronous communication. They mostly sit idle for a long time, wait for data to arrive, then actually do work.
As one might expect, this is handled via threads. Spin up a thread for the sockets, have the thread sleep until data arrives on the socket- usually by select
ing, wake up and process the data. This kind of approach keeps CPU usage down, ensures that the rest of the program remains responsive to other events, and is basically the default practice for this kind of problem. And that's exactly what the threads in Wojciech's program did.
So where's the WTF? Well, it's not in the child threads, it's in the parent thread that kicks them off.
while (childThreadIsWorking) {}
Instead of maybe polling the child thread, or sleeping the parent thread, or doing any of the many other options, this just busy-waits until the child thread is done, defeating the entire purpose of spinning up child threads.
Unfortunately for Wojciech, this pattern was spammed all through the code base. The easiest way to minimize its consequences without completely restructuring how the program handled asynchronous operations was to add 10ms sleeps into the body of the while loop. That alone took things from 100% CPU utilization down to 10% at peak usage.
Метки: CodeSOD |
CodeSOD: Cole's Law of Authentication |
Cabbages are an interesting vegetable, especially as they're one of those subtypes of brassica, that endlessly polymorphic family of plants which includes everything from cauliflower to Brussels sprouts and likely Audrey II.
Gabe was doing for work for a family of academic institutions, and ended up drawing the short straw and working on their scholarship application system. For an application that just needed to maintain a list of candidates and their grades, it was… difficult to support. But it also introduced Gabe to an entirely new use of cabbage: authorization.
function CheckRightsOnTheCurrentPage()
{
// Beware ! . "Cabbage" is necesarry for the authentication to work corectly. THIS IS NOT A JOKE !
if ( strpos( strtolower("Cabbage" . $_SERVER["PHP_SELF"]) , "/admin" ) )
{
if ( ( !isset($_SESSION["IS_ADMIM"] ) ) || ($_SESSION["IS_ADMIM"] == 0 ) )
{
header("HTTP/1.x 403 Forbidden");
header("Content-Type: text/html; charset=iso-8859-1");
header("Expires: Mon, 01 Jan 1990 05:00:00 GMT");
header("Cache-Control: no-store, no-cache, must-revalidate");
header("Cache-Control: post-check=0, pre-check=0", FALSE);
header("Pragma: no-cache");
die();
}
}
return true;
}
A comment which warns me "this is not a joke" is perhaps the greatest- and worst- comment I could ever hope to see.
$_SERVER["PHP_SELF"]
returns the path to the currently executing script. Which, in this example, is located at /admin/admin.php
. So if you check for the strpos
of /admin
, it's zero- which is false. Anything non-zero would be true. So our developer took the easy solution- instead of trying to understand what they were doing or why they were doing it, they just prepended "cabbage" to the string, ensuring that /admin
is never the first thing in the string.
Cabbage based authentication is clearly the WTF, but don't worry- everything about how they handle a failure is wrong. First, they try and build up the error message by directly manipulating the header, along with a bunch of piled-on attempts to keep the error from getting cached, only do die()
at the end.
Also, pedantically, the comment is wrong: the user is already authenticated, we are checking their authorization: is the authenticated user an admin?
THIS IS NOT A JOKE, indeed.
https://thedailywtf.com/articles/cole-s-law-of-authentication
Метки: CodeSOD |
CodeSOD: Bad Parenting |
Conditionals are hard. So are data structures. And today's anonymous submission gives us both of those.
var myParent = this.Parent;
bool parentRemoved = false;
if (myParent is SomeClass asSomeClass)
{
myParent = myParent.Parent;
myParent.Children.Remove(asSomeClass);
parentRemoved = true;
}
if (!parentRemoved)
{
myParent.Children.Remove(this);
}
There are a few pieces to this one, so let's start by focusing in on the conditional expressions first:
if (myParent is SomeClass asSomeClass)
{
parentRemoved = true;
}
if (!parentRemoved)
{
}
This is an else. I mean, it isn't an else, but logically, you could just write this as an if-else expression. This is just a head scratcher, but perhaps not the largest WTF ever. Also note the syntactic sugar- asSomeClass
is a variable declared and assigned in the if
statement, scoped to the first block.
The data-structure element is much worse. You can sort of infer it from the code, but our submitter makes it explicit. This code is part of the implementation of a tree, and the tree has bidirectional links between nodes- children reference their parent, parents reference their children. And this code, ostensibly, removes a node in the middle and reparents its children- but it does it all wrong, because it doesn't actually update the parent references. At most, it stores them in a temporary variable.
The more you look at it, the less sense the choices make. There's no accessor or methods for interacting with the tree as a data structure- this code reaches directly into properties of its parent node, which is just bad encapsulation. That is SomeClass
check is clearly meant to implement polymorphism without actually using inheritance, which is bad- but also, the logical flow is "if the parent is a certain kind of node, delete the parent and reparent the child (without doing it correctly), but if the parent isn't that kind of class, just delete the child" which is… a confusing choice.
And finally, this likely wreaks havoc on garbage collection. While the grandparent releases its reference to its child, the node we're operating on never releases its reference to the parent. Not only does the tree rapidly lose integrity, it also loses any sort of coherent memory management.
I'm suspicious that maybe a developer implementing their own tree data structure instead of using a built-in one might be going down the wrong path. Our submitter closes out the story thus:
I'm working on replacing all these edits with use of accessor methods that ensure tree integrity… and it is… involving a lot more files than I wanted.
Метки: CodeSOD |
CodeSOD: Switching to Booleans |
If you understand booleans in C, then you know that you don't really understand booleans in C. That's the case with Bodo's co-worker, anyway. They either don't understand booleans, or they don't understand their business requirements.
switch (getBool()) {
case false:
/* do something... */
break;
case true:
/* do something else... */
break;
default:
break;
}
Now, at first glance, this looks like a really badly written if
. getBool
returns a boolean value, and then we switch on that value. A boolean can only have two values… right?
Well, in C, the bool
type is just a wrapper around an integer- usually a byte
- with a few helper macros. Whether you're using stdbool
(available since C99), or some homebrew version is a bit of a mess, but the important thing is that bool
s may hold any integer.
Now, all that said, it's still definitely a code smell to see this swifch, it's just not quite as dumb as it looks. In fact, it's dumber.
This code exists in a safety critical industrial system. And that system has a rule: any unexpected values means the system should perform a full stop immediately. After all, if things are at the point where a function which returns either a 0
or a 1
starts returning something else, you can't trust the program anymore. Stop and wait for an operator to recover it.
So that default
, arguably, should be there. It should also do something.
Метки: CodeSOD |
Error'd: No Time Like the Past Future Present |
Метки: Error'd |
CodeSOD: A Bit of Javascript |
We've recently discussed how bit masks can confuse people. Argle's in a position to inherit some of that confused code.
In this case, Argle's company inherited some NodeJS code from a customer who was very upset with the previous development team they had hired. It was a mix of NodeJS with some custom hardware involved.
Like many of us, the first thing Argle's team did was just pull up the code and skim the documentation. It seemed thorough and complete. But it wasn't until they started looking at the implementation that they started to see the true horrors.
Someone had reimplemented all of the bitwise functions as methods. And the core pattern revolved around the bitTest
function:
function bitTest( value, bit ) {
let temp = value.toString(2);
temp = temp.split('').reverse().join('');
let i;
for( i=temp.length; i<32; i++ )
temp += '0';
if ( temp[bit] == '1' )
return true;
else
return false;
}
This turns an integer into a string of its binary representation, splits it into an array to reverse the array to rejoin it into an array (gotta think about endianness!), and then pads the string out to 32 characters long, all so that it can finally ask if temp[bit] == '1'
is true.
But think about the readability benefits! Instead of writing the cryptic if (variable & (1 << bit))
, which is nigh unreadable, you can now say if (bitTest(variable, bit))
which doesn't actually convey any more information and in fact conceals a gigantic pile of string manipulation for a very, very simple check.
I expect this code to be uploaded to NPM and turned into a microframework that ends up powering 75% of web applications by the end of the week, as a dependency-of-a-dependency-of-a-dependency.
Метки: CodeSOD |
CodeSOD: Holiday Sample Pack |
Today, let's whip up a holiday sampler of some short snippets. We start with this anonymously supplied ternary from a C program, which certainly terned my head:
return (accounts == 1 ? 1 : accounts)
If accounts
equals 1, return 1, otherwise return accounts
. Perfect.
On the other hand, submitter Born 2 Ruby, Forced 2 PHP was hired to work on a Ruby on Rails application, but after starting, discovered that the RoR app was an aspiration and the day to day work was to keep the legacy PHP application from falling over in production. So as it turns out, this submitter should have said "no", when they said "yes", much like this code:
function yesorno($in) {
if($in == 'YES') return 'YES';
else return 'NO';
}
And finally, another anonymous submitter sent us this comment they wrote in their code, which I think speaks both for themselves and for anyone who even inherits that code.
// i should have commented this stupid code
Метки: CodeSOD |
CodeSOD: Very Productive Code |
Today's anonymous submitter sends us a pile of code that exists to flummox us. It's consfuing, weird, and I don't understand it, even as I understand it. So we're going to walk through this packet of Python in steps, to see if we can understand what's going on.
First is this handy helper function, to turn a value into a number:
def to_number(s):
try:
if not s:
return 0
number = float(s)
return int(number) if number.is_integer() else number
except (TypeError, ValueError):
return s
This doesn't start too badly. If the value is falsy (None
, for example), return 0. If it has a value, though, turn it into a float. If the resulting float doesn't have a decimal part, convert it into an integer. I've got some complaints about a method returning two different types, but I understand the urge.
But then we get to the exception handler. If, for some reason, this conversion fails we return the input. This method shouldn't be named to_number
, it should be named to_number_if_the_input_is_a_number_otherwise_we_return_the_input
, because that's a behavior that's guaranteed to confuse someone someday.
Okay, I understand that method, so let's look into another.
@staticmethod
def get_product_property(product_id, property):
from utils import to_number
prop = ProductProperty.objects.filter(product_id=product_id, property=property).first()
value = None if prop is None else prop.value
if not value:
return [0, 0] if property.lower() in PROPERTIES_TO_SPLIT else 0
if property.lower() in PROPERTIES_TO_SPLIT:
return list(map(to_number, value.split(",")))
return to_number(value)
One of the first rules in Python: put all your imports in one place, and that place is definitely not at the start of a method. Now, admittedly, this can boost startup times if it's an expensive module to load, but it also means the first call to this method will take a lot longer.
Okay, so we take a product_id
and the property
and search the ProductProperty
lookup table to find the first match, so we can use the value
stored in the DB. Then, we get to something terrible. There is a PROPERTIES_TO_SPLIT
constant, containing a list of property IDs. Because sometimes, the data in the database is a list of values as a comma-separated string. Sometimes it isn't. This is worse than stringly typed data, it's stringly-multiply-typed data.
But that explains why to_number
returns the input on an error- sometimes we pass it an array of numbers.
So now we get to the centerpiece of this sample.
@classmethod
def get_closest_named_product(cls, product):
from core.products import PRODUCTS
"""
product = {"interval": (123, 456), "value": (789, )}
named_products = {
"some_name": {"interval": (987, 654), "value": (321, )},
"some_other_name": {"interval": (123, 456), "value": (789, )},
...
}
-> returns "some_other_name"
do this by multiplying the product dollar values and finding the named_products
with a product that's closest to it
"""
def values_product(item):
return functools.reduce(operator.mul, itertools.chain(*item.values()))
product_product = values_product(product) # 123 * 456 * 789
named_products = {
k: values_product(v)
for k, v in PRODUCTS.items()
} # {"some_name": 987 * 654 * 321, ...}
def key(item):
return abs(product_product - item)
selected_product = min(named_products.values(), key=key)
product_name = {v: k for k, v in named_products.items()}
return product_name[selected_product]
This helpfully has a very thorough code sample in the documentation. Well, maybe not helpfully, because I have no idea what's going on. A product
is apparently a combination of an interval and a value, which are dollar values, so I don't think this is time series data. The way we determine if two products are "close" is based on "multiplying the product dollar values". Why? I don't understand what this is for. Maybe the code will make it clearer.
We create a convenience function value_product
with does some functools
and iterntools
magic to flatten a product and multiply all the numbers together.
Then we repeat this for all the products in our PRODUCTS
constant, which we imported at the top (again). But wait, a PRODUCTS
constant? Yes! There's a hard-coded list of products in our application code- our application code which queries a database. Well, that's fantastic.
Then we chuck another convenience function, key
in there: just take the absolute value of our product_product
's difference from the item
in question. We use that to (ab)use the min
function to search for the minimum difference. Finally, we invert the relationship between keys and values, so that we can lookup the product based on that minimum difference… wait, does this mean that the products in named_products
don't store the whole interval
/value
pair? I don't understand. Does this code even work?
Our submitter writes:
Not that anyone seems to care, this code has been in production for 6 years already.
In this case, the code does work, sometimes, but it's extremely error prone. And the original author knows this. They wrote some unit tests, and commented the unit test for to_number
like so:
# XXX: utils.to_number is returning same string if conversion failed
# FIXME: Either update unit test expectation or function return value
Метки: CodeSOD |
CodeSOD: Enabled and Disabled |
A large part of programming is about communicating intent. The code you write needs to communicate your intent for what the code is supposed to do, both to the computer itself but also to all of the humans which come after you. So much of what we mean when we talk about code style and "clear names" is really about making your intent comprehensible to anyone else who looks at the code.
When the intent of the code isn't clear, we often rely on comments to describe that intent.
Which brings us to this comment that Noreen found.
// IsEnabled and IsDisabled are two different concepts.
// IsEnabled affects whether the element can be focused.
// IsDisabled affects the visual look of the element,
// while allowing it to still be focusable for accessibility purposes.
In most places you encounter "enabled" and "disabled" states, they're usually- but not always- mirrors of each other. A UI element that is enabled is not disabled, and vice versa. Here we have one of those rare exceptions, where it sounds like they don't have anything to do with each other?
In this case, IsEnabled
is about whether a UI element can be interacted with or not. IsDisabled
changes the appearance, but not whether or not it's focusable, "for accessibility" reasons.
I don't know exactly what they mean by that. I suspect they're trying to use the word "disabled" as a proxy for "accessible", which I don't think is a great choice in how we use language. But ignoring the language police aspects, it's also just confusing. As Noreen puts it:
I know naming things is hard, but really? We couldn't do anything about this? This was the best option we could think of?
Метки: CodeSOD |
Error'd: SOL |
This column is experimenting with AI editing. Please provide feedback in the comments if you think the AIs should continue to edit this column or should be replaced by Genuine People Personalities.
Dashing off a quick submission, the self-styled The Beast in Back quips "Breaking Ubuntu's snap is apparently a snap. It's quite a dashed annoyance, actually, since it dies if you precede any argument string with dashes. Go figure."
SOL Brian R. bemoans "Want to know how to use 'out of luck' in a sentence? Sorry, guess you are, um, out of luck."
Fiddler M'at'e messages "Another one from the good ol' days of Windows XP and writable CDs: can you count the number of different languages in this dialog box?" This AI doesn't know why the image below features a flaming ruin, or what it has to do with any missing river. Please help.
Fire-loving Rafa de la Torre confounds the editors with this poser. "While doing a quiz in a Prometheus course, I got this question and had to stop to ponder if I should start responding with 'exporter' to yes/no questions."
But innovative thinker Totty falls upon on a solution. "Is there a third option? Maybe ____?" Try "exporter", Totty.
Метки: Error'd |
CodeSOD: An XML Parser |
Since we were discussing XML earlier this week, it's a good time to take a peek at this submission from Mark.
Before we get into it, though, we need to talk briefly about some different XML parsing philosophies. XML documents are frequently large and deeply nested trees, and once loaded into memory, consume a lot of it. So there are two methods we might use to parse XML. We might parse and load the entire DOM- having all of that expensive processing and memory usage. The advantage is that we have the whole structure in memory, and can quickly and efficiently navigate through it. The other option is to read it as a stream, and process it one node at a time. This is both faster and a lot gentler on memory, but it means you have forward-only access to the contents.
Or, you can just do what Mark's co-worker did:
public void ReadXml(XmlReader reader)
{
string xml = reader.ReadOuterXml();
XElement element = XElement.Parse(xml);
…
}
This method accepts an XmlReader
, which is C#'s implementation of that forward-only, stream-oriented version of an XML parser. They then extract the OuterXml
as a string- the contents of the file- and hand it off to XElement.Parse
, which is the DOM-oriented, extremely expensive method of parsing.
For bonus points, they've greatly increased the memory footprint, since this has to have the whole file in memory as a string, and then the whole file in memory as an XML DOM tree. This is extra offensive as XElement
has a Load
method, which can take an XmlReader
and parse the file without loading the whole string into memory. And, more than that, since the XElement
type represents an XML element, it's great for holding portions of an XML document, but since we're loading (presumably) a whole document, the XmlDocument
class (which also has a Load
method) is probably the correct one to use.
The final note is where this XML data is coming from- it's coming from another C# application, using the same underlying model library, and the objects in question were tagged with .NET's serialization attributes. That's a long way of saying ".NET autogenerated code for serializing/deserializing this data, and this method doesn't need to exist at all."
In short, given the challenge of "loading XML from a document", they've managed to make the wrong choice at every opportunity.
Метки: CodeSOD |
CodeSOD: Counting on Switching |
Frequent contributor Argle found some code written at his current company a "long time ago", but unfortunately in a galaxy far too close to our own.
If rsRecordSetD1T3("ItemState")<>"N" then
Select Case rsRecordSetD1T3("ItemState")
Case "R", "L"
tally=tally+1
Case "B"
tally=tally+2
Case "N"
tally=tally+0
Case Else
tally=tally+0
End Select
…
End If
We start with an If
: only enter this branch if ItemState
is definitely not equal to "N". Then we enter a switch.
If ItemState
is "R" or "L", we add one to tally
. If it's "B", we add two. If it's "N", we add zero, and wait, we need to pause here a moment.
First, we know it can't possibly be "N". We just checked that above. But then, we also… add zero. Which is effectively a no-op, at least in terms of its impact on program state.
And then, in a Case Else
, e.g. for all other values, we also add zero.
This is very much the kind of code that "evolved". Probably, at one point, case "N" and the else actually did something. Then the if-not-equal branch was added. But then someone else decided to make "N" do nothing, but wasn't confident enough to remove it. Then someone else did the same on the Else
. Reading this code is like being a paleontologist finding a tooth and extrapolating from that an entire living creature.
In this case, that creature is a beast that hopefully goes extinct soon.
Метки: CodeSOD |
CodeSOD: A Bit of a Misunderstanding |
Mark B sends us another sample from a big-ball-o-wtf.
Let's take a look at two variables. One variable is named taskDetail
. The other variable is named binaryTaskDetail
. You might ask yourself: what are the differences between these two?
Well, one of them is a C# int
- 32-bit integer. The other is a C# long
- a 64-bit integer. And, as the name implies, binaryTaskDetail
is meant to be a binary representation of the number.
"Wait," you might ask, "aren't all integers just binary representations under the hood? Can't you just bitshift or mask or whatever you need to do? Do you mean they want to output binary data? Like as a print statement? Now I'm confused."
I'm sorry I confused you. Let's take a look at the code, and everything will be clear.
binaryTaskDetail = Int64.Parse(Convert.ToString(taskDetail, 2));
Let's say that taskDetail
contained the number 434. We use Convert.ToString
to convert it into a string, but note the 2
in the parameters- we're doing that in base two, so that gives us a binary representation, as a string: 110110010
. Then we parse the string back into an integer. Which, you'll note, the string doesn't advertize what base it's in, so it turns into the integer 110110010
.
So, we convert a number into a binary representation as a string, then parse that string as a decimal. It goes without saying that 0b110110010
and 110110010
are not the same number. In this example, binaryTaskDetail
now contains the binary value 110100100000010010100111010
.
Why is any of this happening? I have no idea. The most Mark can offer is: "This time the developer knew that you are supposed to do things in binary for… reasons, but not much else."
https://thedailywtf.com/articles/a-bit-of-a-misunderstanding
Метки: CodeSOD |
CodeSOD: Shipping a Gallon of Soap |
While JSON's strength is its (relative) simplicity, XML's strength was always its power. Combined with an XML schema, you can create strongly typed documents with arbitrary data-types. For example, it's easy to define, oh, I don't know, a date time field in a way that isn't stringly typed.
Of course, that's what also creates its complexity. XML is big, and bloated, and expensive to parse. Which brings us back to the old days of SOAP- the Simple Object Access Protocol, essentially an XML remote procedure call. It was powerful, but complex, with lots of interlocking standards. With its Web Service Description Language (WSDL), the service could even be self documenting, laying out all the objects and fields you could interact with.
With all that web service tooling, it's trivial to expose core elements in your object model as web service end points, which is great if your object model is clean and easy to understand. It sorta sucks if you're the vendor Paul C ends up needing to work with.
This is an API exposed by a shipping service, and it's clear that "consistency" wasn't part of its original goal. So they have fields like
and
. That gives you the hope that maybe at least they standardized on Hungarian notation, but don't worry, in other places it's just called AccountNumber
. Similarly, there's
and
, which caused no end of confusion, especially as the WSDL wasn't always accurate.
But the sample Paul wanted us to look at was this one:
<WayBillInfo>
<AccountNumber>23962AccountNumber>
<Waybill>SH0000317Waybill>
<Date>2022-11-09T00:00:00+02:00Date>
<Time>1900-01-01T09:06:05.11+02:00Time>
<Hub>JNBHub>
<Code>73Code>
<Description>Added via WebserviceDescription>
<MovementDescription>Added via WebserviceMovementDescription>
WayBillInfo>
Specifically, look at the
and fields. The date field holds a date formatted as a string- with a time portion that is ignored. Similarly, the time field holds a time formatted as a string with a date part that's ignored.
"Why couldn't these be the same field?" you might ask. Or, "Why couldn't they use the schema to define a better date or time representation?" Because they didn't care is likely the correct answer to both those questions.
Paul sums it up: "There's lots of SOAP, but I don't feel very clean."
Метки: CodeSOD |
Error'd: Classic Errord: Having Fun With Words |
Error'd are eternal. This one's from way back when. Original
M. T. wants to expandify your vocabulation!
According to Edwin, he couldn't even click OK because the software wasn't responding. It closed itself after about two minutes, so I guess it didn't matter.
Finally, E. J. sent in this mess:
https://thedailywtf.com/articles/classic-errord-having-fun-with-words
Метки: Error'd |
CodeSOD: Common Variables |
It's important to be prepared- but not too prepared. A common trap developers fall into is "premature abstraction"- trying to solve the general case of a problem when you only need to solve a very specific case.
Frequent contributor Argle sends us some very old BASIC code. The task was to convert this ancient language into C#.
Someone decided that, instead of waiting to learn specifically what variables they'd eventually need, they'd just declare all the variables they could ever want right at the top.
Which gives us this block, included in its entirety.
COMMON PRICE1, PIECE1, PRICE2, PIECE2, PRICE3, PIECE3, PRICE4, PIECE4
COMMON PRICE5, PIECE5, PRICE6, PIECE6, PRICE7, PIECE7, PRICE8, PIECE8
COMMON PRICE9, PIECE9, PRICE10, PRICE11, PIECE11, PRICE12, PIECE12
COMMON PRICE13, PIECE13, PRICE14, PIECE14, PRICE15, PIECE15, PRICE16, PIECE16
COMMON PRICE17, PIECE17, PRICE18, PIECE18, PRICE19, PIECE19, PRICE20
COMMON PRICE21, PIECE21, PRICE22, PIECE22, PRICE23, PIECE23, PRICE24, PIECE24
COMMON PRICE25, PIECE25, PRICE26, PIECE26, PRICE27, PIECE27, PRICE28, PIECE28
COMMON PRICE29, PIECE29, PRICE30, PRICE31, PIECE31, PRICE32, PIECE32
COMMON PRICE33, PIECE33, PRICE34, PIECE34, PRICE35, PIECE35, PRICE36, PIECE36
COMMON PRICE37, PIECE37, PRICE38, PIECE38, PRICE39, PIECE39, PRICE40
COMMON PRICE41, PIECE41, PRICE42, PIECE42, PRICE43, PIECE43, PRICE44, PIECE44
COMMON PRICE45, PIECE45, PRICE46, PIECE46, PRICE47, PIECE47, PRICE48, PIECE48
COMMON PRICE49, PIECE49, PRICE50, PRICE51, PIECE51, PRICE52, PIECE52
COMMON PRICE53, PIECE53, PRICE54, PIECE54, PRICE55, PIECE55, PRICE56, PIECE56
COMMON PRICE57, PIECE57, PRICE58, PIECE58, PRICE59, PIECE59, PRICE60
COMMON PRICE61, PIECE61, PRICE62, PIECE62, PRICE63, PIECE63, PRICE64, PIECE64
COMMON PRICE65, PIECE65, PRICE66, PIECE66, PRICE67, PIECE67, PRICE68, PIECE68
COMMON PRICE69, PIECE69, PRICE70, PRICE71, PIECE71, PRICE72, PIECE72
COMMON PRICE73, PIECE73, PRICE74, PIECE74, PRICE75, PIECE75, PRICE76, PIECE76
COMMON PRICE77, PIECE77, PRICE78, PIECE78, PRICE79, PIECE79, PRICE80
COMMON PRICE81, PIECE81, PRICE82, PIECE82, PRICE83, PIECE83, PRICE84, PIECE84
COMMON PRICE85, PIECE85, PRICE86, PIECE86, PRICE87, PIECE87, PRICE88, PIECE88
COMMON PRICE89, PIECE89, PRICE90, PRICE91, PIECE91, PRICE92, PIECE92
COMMON PRICE93, PIECE93, PRICE94, PIECE94, PRICE95, PIECE95, PRICE96, PIECE96
COMMON PRICE97, PIECE97, PRICE98, PIECE98, PRICE99, PIECE99, PRICE100
COMMON PRINSUL, PRPARTS
COMMON PART1$, PRIWIRE$, SECWIRE$
COMMON MAT1$, MAT2$, MAT3$, MAT4$, MAT5$, MAT6$, MAT7$, MAT8$, MAT9$
COMMON MAT11$, MAT12$, MAT13$, MAT14$, MAT15$, MAT16$, MAT17$, MAT18$, MAT19$
COMMON MAT21$, MAT22$, MAT23$, MAT24$, MAT25$, MAT26$, MAT27$, MAT28$, MAT29$
COMMON MAT31$, MAT32$, MAT33$, MAT34$, MAT35$, MAT36$, MAT37$, MAT38$, MAT39$
COMMON MAT41$, MAT42$, MAT43$, MAT44$, MAT45$, MAT46$, MAT47$, MAT48$, MAT49$
COMMON MAT51$, MAT52$, MAT53$, MAT54$, MAT55$, MAT56$, MAT57$, MAT58$, MAT59$
COMMON MAT61$, MAT62$, MAT63$, MAT64$, MAT65$, MAT66$, MAT67$, MAT68$, MAT69$
COMMON MAT71$, MAT72$, MAT73$, MAT74$, MAT75$, MAT76$, MAT77$, MAT78$, MAT79$
COMMON MAT81$, MAT82$, MAT83$, MAT84$, MAT85$, MAT86$, MAT87$, MAT88$, MAT89$
COMMON MAT91$, MAT92$, MAT93$, MAT94$, MAT95$, MAT96$, MAT97$, MAT98$, MAT99$
COMMON WINDINGLENGTH, SHORTERWINDINGLENGTH, FILLER, FILLERSHORTERWINDINGLENGTH
COMMON WINDINGLENGTH$, SHORTERWINDINGLENGTH$, PARTS$
COMMON ADD1, ADD2, ADD3, ADD4, ADD5, ADD6, ADD7, ADD8, ADD9, ADD10
COMMON ADD11, ADD12, ADD13, ADD14, ADD15, ADD16, ADD17, ADD18, ADD19, ADD20
COMMON ADD21, ADD22, ADD23, ADD24, ADD25, ADD26, ADD27, ADD28, ADD29, ADD30
COMMON ADD31, ADD32, ADD33, ADD34, ADD35, ADD36, ADD37, ADD38, ADD39, ADD40
COMMON ADD41, ADD42, ADD43, ADD44, ADD45, ADD46, ADD47, ADD48, ADD49, ADD50
COMMON ADD51, ADD52, ADD53, ADD54, ADD55, ADD56, ADD57, ADD58, ADD59, ADD60
COMMON ADD61, ADD62, ADD63, ADD64, ADD65, ADD66, ADD67, ADD68, ADD69, ADD70
COMMON ADD71, ADD72, ADD73, ADD74, ADD75, ADD76, ADD77, ADD78, ADD79, ADD80
COMMON ADD81, ADD82, ADD83, ADD84, ADD85, ADD86, ADD87, ADD88, ADD89, ADD90
COMMON ADD91, ADD92, ADD93, ADD94, ADD95, ADD96, ADD97, ADD98, ADD99, ADD100
COMMON ADD101, ADD102, ADD103, ADD104, ADD105, ADD106, ADD107, ADD108, ADD109, ADD110
COMMON ADD111, ADD112, ADD113, ADD114, ADD115, ADD116, ADD117, ADD118, ADD119, ADD120
COMMON ADD121, ADD122, ADD123, ADD124, ADD125, ADD126, ADD127, ADD128, ADD129, ADD130
COMMON ADD131, ADD132, ADD133, ADD134, ADD135, ADD136, ADD137, ADD138, ADD139, add140
COMMON ADD141, ADD142, ADD143, ADD144, ADD145, ADD146, ADD147, ADD148, ADD149, ADD150
COMMON ADD151, ADD152, ADD153, ADD154, ADD155, ADD156, ADD157, ADD158, ADD159, ADD160
COMMON ADD161, ADD162, ADD163, ADD164, ADD165, ADD166, ADD167, ADD168, ADD169, ADD170
COMMON ADD171, ADD172, ADD173, ADD174, ADD175, ADD176, ADD177, ADD178, ADD179, ADD180
COMMON ADD181, ADD182, ADD183, ADD184, ADD185, ADD186, ADD187, ADD188, ADD189, ADD190
COMMON ADD191, ADD192, ADD193, ADD194, ADD195, ADD196, ADD197, ADD198, ADD199, ADD200
COMMON ADD201, ADD202, ADD203, ADD204, ADD205, ADD206, ADD207, ADD208, ADD209, ADD210
COMMON ADD211, ADD212, ADD213, ADD214, ADD215, ADD216, ADD217, ADD218, ADD219, ADD220
COMMON ADD221, ADD222, ADD223, ADD224, ADD225, ADD226, ADD227, ADD228, ADD229, ADD230
COMMON ADD231, ADD232, ADD233, ADD234, ADD235, ADD236, ADD237, ADD238, ADD239, ADD240
COMMON ADD241, ADD242, ADD243, ADD244, ADD245, ADD246, ADD247, ADD248, ADD249, ADD250
COMMON ADD251, ADD252, ADD253, ADD254, ADD255, ADD256, ADD257, ADD258, ADD259, ADD260
COMMON ADD261, ADD262, ADD263, ADD264, ADD265, ADD266, ADD267, ADD268, ADD269, ADD270
COMMON ADD271, ADD272, ADD273, ADD274, ADD275, ADD276, ADD277, ADD278, ADD279, ADD280
COMMON ADD281, ADD282, ADD283, ADD284, ADD285, ADD286, ADD287, ADD288, ADD289, ADD290
COMMON ADD291, ADD292, ADD293, ADD294, ADD295, ADD296, ADD297, ADD298, ADD299, ADD300
COMMON ADD1$, ADD2$, ADD3$, ADD4$, ADD5$, ADD6$, ADD7$, ADD8$, ADD9$, ADD10$
COMMON ADD11$, ADD12$, ADD13$, ADD14$, ADD15$, ADD16$, ADD17$, ADD18$, ADD19$, ADD20$
COMMON ADD21$, ADD22$, ADD23$, ADD24$, ADD25$, ADD26$, ADD27$, ADD28$, ADD29$, ADD30$
COMMON ADD31$, ADD32$, ADD33$, ADD34$, ADD35$, ADD36$, ADD37$, ADD38$, ADD39$, ADD40$
COMMON ADD41$, ADD42$, ADD43$, ADD44$, ADD45$, ADD46$, ADD47$, ADD48$, ADD49$, ADD50$
COMMON ADD51$, ADD52$, ADD53$, ADD54$, ADD55$, ADD56$, ADD57$, ADD58$, ADD59$, ADD60$
COMMON ADD61$, ADD62$, ADD63$, ADD64$, ADD65$, ADD66$, ADD67$, ADD68$, ADD69$, ADD70$
COMMON ADD71$, ADD72$, ADD73$, ADD74$, ADD75$, ADD76$, ADD77$, ADD78$, ADD79$, ADD80$
COMMON ADD81$, ADD82$, ADD83$, ADD84$, ADD85$, ADD86$, ADD87$, ADD88$, ADD89$, ADD90$
COMMON ADD91$, ADD92$, ADD93$, ADD94$, ADD95$, ADD96$, ADD97$, ADD98$, ADD99$, ADD100$
COMMON ADD101$, ADD102$, ADD103$, ADD104$, ADD105$, ADD106$, ADD107$, ADD108$, ADD109$, ADD110$
COMMON ADD111$, ADD112$, ADD113$, ADD114$, ADD115$, ADD116$, ADD117$, ADD118$, ADD119$, ADD120$
COMMON ADD121$, ADD122$, ADD123$, ADD124$, ADD125$, AD1D26$, ADD127$, ADD128$, ADD129$, ADD130$
COMMON ADD131$, ADD132$, ADD133$, ADD134$, ADD135$, ADD136$, ADD137$, ADD138$, ADD139$, add140$
COMMON ADD141$, ADD142$, ADD143$, ADD144$, ADD145$, ADD146$, ADD147$, ADD148$, ADD149$, ADD150$
99% of these variables are never used. The hypothesis was that, once they were used, they'd get renamed into something more meaningful. If you skim through, you'll see that there are some named variables in that code, like PRIWIRE$
and SECWIRE$
. Those variables are not used anywhere in the code. But don't worry, PRICE31
is. Somewhere.
At least they'll never have to declare a new variable again.
Метки: CodeSOD |
CodeSOD: Years of Success |
Way back in late 2006, Cody inherited a Java application. Since launching in 2001, the application had been running in production without any notable problems. And then, one day, it suddenly started throwing out errors on some orders. And then, a little later, any time someone tried to place an order. This constituted a rather large issue, since processing new orders was vitally important for keeping the lights on.
The errors were validation errors, so Cody started by going to the line where the validation happened, and the exception was thrown:
if (!validateBeanData(order)) {
throw new OrderRequesterException(order.getPoNumber().trim(), "63", "Invalid Request Criteria");
}
The additional whitespace is in the original.
Okay, so what exactly is validateBeanData
doing?
private boolean validateBeanData(OrderRequestBean order) {
boolean status = true;
… // various checks are performed here
if (status) {
if (requestDueYear.equals("2001")
|| requestDueYear.equals("2002")
|| requestDueYear.equals("2003")
|| requestDueYear.equals("2004")
|| requestDueYear.equals("2005")
|| requestDueYear.equals("2006"))
status = true;
else
status = false;
}
…
return status;
}
The years were hardcoded covering all years between 2001 and 2006. Which was great, until customers started putting in orders that wouldn't fulfill until 2007. Maybe someone at the company had planned to eventually update those dates. Maybe someone was looking for job security. Maybe someone didn't expect any security at all, and just assumed the company would go under before this was a problem.
Whatever the case, it was easy for Cody to understand what the problem was.
Метки: CodeSOD |
CodeSOD: Check Out This Legacy App |
VisualBasic was… an interesting beast. The language wasn't great, and because object orientation was awkwardly bolted onto it, but it also was heavily integrated into Microsoft's ActiveX libraries (heavily object oriented), there were all sorts of interesting ways to break your program with it. Even better: it was designed to be easy so that "anyone" could use it.
Which leads to some of this code, from Dave. A number of years back, Dave was asked to try and convert an ancient VB6 application into something modern. Like all such conversions, the brief was: "make a new application that does exactly what the old application does, but nobody actually knows what the old application does because we never documented any requirements, just read the code".
Reading the code had some "fun" moments, like this one:
Public fun As Single
Public funprime As Single
As well as a few code smells:
Public p As Single
Public po As Single
Public Poo As Single
VisualBasic's killer feature, though, was its UI designer. Drag and drop widgets onto a canvas. Double click on them and it pops you right into an event handling method. Just write what happens when the button is clicked. Flip back to the UI designer, and you've got a handy dandy panel to control things like the color, the font, and most important- the name of the button that gets used in code. Because the UI designer can't intuit what name you want, so when you add a button or a checkbox, it just gets named something like Button11
. It's up to you to rename it btnSave
(per the convention in VB circles).
Of course, nothing made you do that. Which means you end up with code like:
Private Sub Check22_Click()
If Check22 = 1 Then
Check20 = 0
Check21 = 0
Check2.Enabled = True
Check9.Enabled = True
Check10.Enabled = True
Check11.Enabled = True
Check12.Enabled = False
Check12 = 0
End If
End Sub
Dave writes: "Waiter, check please!"
Метки: CodeSOD |
CodeSOD: Unit Test Coverage |
One of the fastest ways to get promoted in certain environments is through attrition. When a key player leaves, someone needs to step up and take over their work, somehow.
Well, at Antonio's workplace, the tech lead on several projects abruptly quit. This sent the Project Management Office into a spiral, as that one developer's tasks were on every critical path on their Gannt chart. This four-alarm panic escalated all the way up to the C-suite.
And that's how Antonio and a bunch of other developers got pulled into a series of meetings to turn them into "bug hunter-killers". Garth, the head of the PMO, laid out their mission.
"The root cause of the bugs are architectural flaws which have not yet been addressed. So we need a team that's going to go in, and salvage this project."
"Are there any unit tests?" Antonio asked. "Before we go on a big refactoring push, we should make sure we're not introducing regressions."
"Oh, don't worry about that. Our CI system is configured to reject builds under certain levels of coverage."
So Antonio dug into the code. The project was in Java, and pretty much every class was built using Lombok- a library that handles a bunch of boring stuff, like implementing constructors, toString
and equals
methods. With that in mind, let's look at their expansive code coverage in their unit tests:
@Test
public void shouldBeNotNullAndTestHashcode() throws ParseException {
Entity Entity = factory.manufacturePojo(Entity.class);
assertNotNull(Entity);
assertNotNull(Entity.toString());
assertNotNull(Entity.hashCode());
}
@Test
public void testTrueEquals() {
assertTrue(new Entity().equals(new Entity()));
}
@Test
public void testFalseEquals() {
Entity Entity = factory.manufacturePojo(Entity.class);
assertFalse(new Entity().equals(Entity));
}
Entity
has been anonymized, but in practice, this is the model for the majority of their unit tests: they test the code generated by Lombok. This keeps their code coverage metric pretty high, but unfortunately, doesn't actually test anything.
The "architectural flaws" the PMO is concerned about are mostly of the form "a sole developer did a rush job under changing requirements and unreasonable deadlines and the code is just a plain mess". Between that and no real tests, the PMO is going to have to learn to love the sound deadlines make when they fly by.
Метки: CodeSOD |
Error'd: Thanks |
We got several submissions this week related to Steam's website. Apparently it's popular with TDWTF readers. Is all of this a genuine indication of careless webdevs, or is Steam marketing just so fiendishly clever that they're tricking us into amplifying their presence? Feel free to speculate; I'm going with a corollary to Hanlon's razor. Never attribute to malice that which can adequately be explained by awarding every job to the lowest bidder.
First, Brad K. declared "I was surprised to see an email from Steam about a summer sale this late in the year. Then I saw, they got it right at the same time they got it wrong,"
Moments later, a probably-pseudonymous Smithers shared the same error, pointed out that Steam's website badly mangled the name of its recommended purchase, and in doing so, simultaneously leaked a view into his(?) personal game wishlist. TMI?
Further highlighting Steam's popularity Nasias Darktar ballyhooed "Praise the Emperor this Autumn with Steam's Autumn Sale, offering the devoted some fantastic discounts on the Warhammer franchise."
Traveling on from Steam's misadventures, Bruce W. wonders "So, Marriott, how much did my room cost?" I wonder what would happen if Bruce base64-decoded that text.
Finally, Marc W"urth shares a funny email. "Yeah, I can see it, GitLab!" I would like to see what happened when he followed the instructions to klicken Sie hier in order to im Webbrowser anzuzeigen
For those in the US, enjoy your long weekend (assuming you're fortunate enough to have the extra holiday). For those with more enlightened labor relations, we get it, but you can gloat in the comments section if you need to.
Метки: Error'd |