CodeSOD: A Loop in the String |
Robert was browsing through a little JavaScript used at his organization, and found this gem of type conversion.
//use only for small numbers
function StringToInteger (str) {
var int = -1;
for (var i=0; i<=100; i++) {
if (i+"" == str) {
int = i;
break;
}
}
return int;
}
So, this takes our input str
, which is presumably a string, and it starts counting from 0 to 100. i+""
coerces the integer value to a string, which we compare against our string. If it’s a match, we’ll store that value and break out of the loop.
Obviously, this has a glaring flaw: the 100
is hardcoded. So what we really need to do is add a search_low
and search_high
parameter, so we can write the for loop as i = search_low; i <= search_high; i++
instead. Because that’s the only glaring flaw in this code. I can’t think of any possible better way of converting strings to integers. Not a one.
Метки: CodeSOD |
CodeSOD: Nullable Knowledge |
You’ve got a decimal value- maybe. It could be nothing at all, and you need to handle that null gracefully. Fortunately for you, C# has “nullable types”, which make this task easy.
Ian P’s co-worker made this straightforward application of nullable types.
public static decimal ValidateDecimal(decimal? value)
{
if (value == null) return 0;
decimal returnValue = 0;
Decimal.TryParse(value.ToString(), out returnValue);
return returnValue;
}
The lack of indentation was in the original.
The obvious facepalm is the Decimal.TryParse
call. If our decimal has a value, we could just return it, but no, instead, we convert it to a string then convert that string back into a Decimal
.
But the real problem here is someone who doesn’t understand what .NET’s nullable types offer. For starters, one could make the argument that value.HasValue()
is more readable than value == null
, though that’s clearly debatable. That’s not really the problem though.
The purpose of ValidateDecimal
is to return the input value, unless the input value was null, in which case we want to return 0
. Nullable types have a lovely GetValueOrDefault()
method, which returns the value, or a reasonable default. What is the default for any built in numeric type?
0
.
This method doesn’t need to exist, it’s already built in to the decimal?
type. Of course, the built-in method almost certainly doesn’t do a string conversion to get its value, so the one with a string is better, is it knot?
Метки: CodeSOD |
Internship of Things |
Mindy was pretty excited to start her internship with Initech's Internet-of-Things division. She'd been hearing at every job fair how IoT was still going to be blowing up in a few years, and how important it would be for her career to have some background in it.
It was a pretty standard internship. Mindy went to meetings, shadowed developers, did some light-but-heavily-supervised changes to the website for controlling your thermostat/camera/refrigerator all in one device.
As part of testing, Mindy created a customer account on the QA environment for the site. She chucked a junk password at it, only to get a message: "Your password must be at least 8 characters long, contain at least three digits, not in sequence, four symbols, at least one space, and end with a letter, and not be more than 10 characters."
"Um, that's quite the password rule," Mindy said to her mentor, Bob.
"Well, you know how it is, most people use one password for every site, and we don't want them to do that here. That way, when our database leaks again, it minimizes the harm."
"Right, but it's not like you're storing the passwords anyway, right?" Mindy said. She knew that even leaked hashes could be dangerous, but good salting/hashing would go a long way.
"Of course we are," Bob said. "We're selling web connected thermostats to what can be charitably called 'twelve-o-clock flashers'. You know what those are, right? Every clock in their house is flashing twelve?" Bob sneered. "They can't figure out the site, so we often have to log into their account to fix the things they break."
A few days later, Initech was ready to push a firmware update to all of the Model Q baby monitor cameras. Mindy was invited to watch the process so she could understand their workflow. It started off pretty reasonable: their CI/CD system had a verified build, signed off, ready to deploy.
"So, we've got a deployment farm running in the cloud," Bob explained. "There are thousands of these devices, right? So we start by putting the binary up in an S3 bucket." Bob typed a few commands to upload the binary. "What's really important for our process is that it follows this naming convention. Because the next thing we're going to do is spin up a half dozen EC2 instances- virtual servers in the cloud."
A few more commands later, and then Bob had six sessions open to cloud servers in tmux
. "Now, these servers are 'clean instances', so the very first thing I have to do is upload our SSH keys." Bob ran an ssh-copy-id
command to copy the SSH key from his computer up to the six cloud VMs.
"Wait, you're using your personal SSH keys?"
"No, that'd be crazy!" Bob said. "There's one global key for every one of our Model Q cameras. We've all got a copy of it on our laptops."
"All… the developers?"
"Everybody on the team," Bob said. "Developers to management."
"On their laptops?"
"Well, we were worried about storing something so sensitive on the network."
Bob continued the process, which involved launching a script that would query a webservice to see which Model Q cameras were online, then ssh
ing into them, having them curl
down the latest firmware, and then self-update. "For the first few days, we leave all six VMs running, but once most of them have gotten the update, we'll just leave one cloud service running," Bob explained. "Helps us manage costs."
It's safe to say Mindy learned a lot during her internship. Mostly, she learned, "don't buy anything from Initech."
Метки: Feature Articles |
Error'd: Intentionally Obtuse |
"Normally I do pretty well on the Super Quiz, but then they decided to do it in Latin," writes Mike S.
"Uh oh, this month's AWS costs are going to be so much higher than last month's!" Ben H. writes.
Amanda C. wrote, "Oh, neat, Azure has some recommendations...wait...no...'just kidding' I guess?"
"Here I never thought that SQL Server log space could go negative, and yet, here we are," Michael writes.
"I love the form factor on this motherboard, but I'm not sure what case to buy with it," Todd C. writes, "Perhaps, if it isn't working, I can just give it a little kick?"
Maarten C. writes, "Next time, I'll name my spreadsheets with dog puns...maybe that'll make things less ruff."
Метки: Error'd |
CodeSOD: Swimming Downstream |
When Java added their streams API, they brought the power and richness of functional programming styles to the JVM, if we ignore all the other non-Java JVM languages that already did this. Snark aside, streams were a great addition to the language, especially if we want to use them absolutely wrong.
Like this code Miles found.
See, every object in the application needs to have a unique identifier. So, for every object, there’s a method much like this one:
/**
* Get next priceId
*
* @return next priceId
*/
public String createPriceId() {
List ids = this.prices.stream().map(m -> m.getOfferPriceId()).collect(Collectors.toList());
for (Integer i = 0; i < ids.size(); i++) {
ids.set(i, ids.get(i).split("PR")[1]);
}
try {
List intIds = ids.stream().map(id -> Integer.parseInt(id)).collect(Collectors.toList());
Integer max = intIds.stream().mapToInt(id -> id).max().orElse(0);
return "PR" + (max + 1);
} catch (Exception e) {
return "PR" + 1;
}
}
The point of a stream is that you can build a processing pipeline: starting with a list, you can perform a series of operations but only touch each item in the stream once. That, of course, isn’t what we do here.
First, we map
the prices to extract the offerPriceId
and convert it into a list. Now, this list is a set of strings, so we iterate across that list of IDs, to break the "PR"
prefix off. Then, we’ll map
that list of IDs again, to parse the strings into integers. Then, we’ll cycle across that new list one more time, to find the max value. Then we can return a new ID.
And if anything goes wrong in this process, we won’t complain. We just return an ID that’s likely incorrect- "PR1"
. That’ll probably cause an error later, right? They can deal with it then.
Everything here is just wrong. This is the wrong way to use streams
- the whole point is this could have been a single chain of function calls that only needed to iterate across the input data once. It’s also the wrong way to handle exceptions. And it’s also the wrong way to generate IDs.
Worse, a largely copy/pasted version of this code, with the names and prefixes changed, exists in nearly every model class. And these are database model classes, according to Miles, so one has to wonder if there might be a better way to generate IDs…
Метки: CodeSOD |
CodeSOD: Seven First Dates |
Your programming language is PHP, which represents datetimes as milliseconds since the epoch. Your database, on the other hand, represents datetimes as seconds past the epoch. Now, your database driver certainly has methods to handle this, but can you really trust that?
Nancy found some code which simply needs to check: for the past week, how many customers arrived each day?
$customerCount = array();
$result2 = array();
$result3 = array();
$result4 = array();
$min = 20;
$max = 80;
for ( $i = $date; $i < $date + $days7 + $day; $i += $day ) {
$first_datetime = date('Y-m-d H:i',substr($i - $day,0,-3));
$second_datetime = date('Y-m-d H:i',substr($i,0,-3));
$sql = $mydb ->prepare("SELECT
COUNT(DISTINCT Customer.ID) 'Customer'
FROM Customer
WHERE Timestamp BETWEEN %s AND %s",$first_datetime,$second_datetime);
$output = $mydb->get_row($sql);
array_push( $customerCount, $output->Customer == null ? 0 : $output->Customer);
}
array_push( $result4, $customerCount );
array_push( $result4, $result2 );
array_push( $result4, $result3 );
return $result4;
If you have a number of milliseconds and you wish to convert it to seconds, you might do something silly and divide by 1,000, but here we have a more obvious solution: substr
the last three digits off to create our $first_datetime
and $second_datetime
.
Using that, we can prepare a separate query for each day, looping across them to populate $customerCount
.
Once we’ve collected all the counts in $customerCount
, we then push that into $result4
. And then we push the empty $result2
into $result4
, followed by the equally empty $result3
, at which point we can finally return $result4
.
There’s no $result1
, but it looks like $customerCount
was a renamed version of that, just by the sequence of declarations. And then $min
and $max
are initialized but never used, and from that, it’s very easy to understand what happened here.
The original developer copied some sample code from a tutorial, but they didn’t understand it. They knew they had a goal, and they knew that their goal was similar to the tutorial, so they just blundered about changing things until they got the results they expected.
Nancy threw all this out and replaced it with a GROUP BY
query.
Метки: CodeSOD |
CodeSOD: Bunny Bunny |
When you deploy any sort of complicated architecture, like microservices, you also end up needing to deploy some way to route messages between all the various bits and bobs in your application. You could build this yourself, but you’ll usually use an off-the-shelf product, like Kafka or RabbitMQ.
This is the world Tina lives in. They have a microservice-based architecture, glued together with a RabbitMQ server. The various microservices need to connect to the RabbitMQ, and thus, they need to be able to check if that connection were successful.
Now, as you can imagine, that would be a built-in library method for pretty much any MQ client library, but if people used the built-in methods for common tasks, we’d have far fewer articles to write.
Tina’s co-worker solved the “am I connected?” problem thus:
def are_we_connected_to_rabbitmq():
our_node_ip = get_server_ip_address()
consumer_connected = False
response = requests.get("http://{0}:{1}@{2}:15672/api/queues/{3}/{4}".format(
self.username,
self.password,
self.rabbitmq_host,
self.vhost,
self.queue))
if response and response.status_code == 200:
js_response = json.loads(response.content)
consumer_details = js_response.get('consumer_details', [])
for consumer in consumer_details:
peer_host = consumer.get('channel_details', {}).get(
'peer_host')
if peer_host == our_node_ip:
consumer_connected = True
break
return consumer_connected
To check if our queue consumer has successfully connected to the queue, we send an HTTP request to one of RabbitMQ’s restful endpoints to find a list of all of the connected consumers. Then we check to see if any of those consumers has our IP address. If one does, that must be us, so we must be connected!
Метки: CodeSOD |
CodeSOD: A Truly Painful Exchange |
Java has a boolean type, and of course it also has a parseBoolean
method, which works about how you'd expect. It's worth noting that a string "true" (ignoring capitalization) is the only thing which is considered true, and all other inputs are false. This does mean that you might not always get the results you want, depending on your inputs, so you might need to make your own boolean parser.
Adam H has received the gift of legacy code. In this case, the code was written circa 2002, and the process has been largely untouched since. An outside vendor uploads an Excel spreadsheet to an FTP site. And yes, it must be FTP, as the vendor's tool won't do anything more modern, and it must be Excel because how else do you ship tables of data between two businesses?
The Excel sheet has some columns which are filled with "TRUE"
and "FALSE"
. This means their process needs to parse those values in as booleans. Or does it…
public class BooleanParseUtil {
private static final String TRUE = "TRUE";
private static final String FALSE = "FALSE";
private BooleanParseUtil() {
//private because class should not be instantiated
}
public static String parseBoolean(String paramString) {
String result = null;
if (paramString != null) {
String s = paramString.toUpperCase().trim();
if (ParseUtilHelper.isPositive(s)) {
result = TRUE;
} else if (ParseUtilHelper.isNegative(s)) {
result = FALSE;
}
} else {
result = FALSE;
}
return result;
}
//snip
}
Note the signature of parseBoolean
: it takes a string and it returns a string. If we trace through the logic: a null input is false, a not-null input that isPositive
is "TRUE"
, one that isNegative
is "FALSE"
, and anything else returns null
. I'm actually pretty sure that's a mistake, and is exactly the kind of thing that happens when you follow the "single return rule"- where each method has only one return
statement. This likely is a source of heisenbugs and failed processing runs.
But wait a second, isPositive
sure sounds like it means "greater than or equal to zero". But that can't be what it means, right? What are isPositive
and isNegative
actually doing?
public class ParseUtilHelper {
private static final String TRUE = "TRUE";
private static final String FALSE = "FALSE";
private static final Set positiveValues = new HashSet<>(
Arrays.asList(TRUE, "YES", "ON", "OK", "ENABLED", "ACTIVE", "CHECKED", "REPORTING", "ON ALL", "ALLOW")
);
private static final Set negativeValues = new HashSet<>(
Arrays.asList(FALSE, "NO", "OFF", "DISABLED", "INACTIVE", "UNCHECKED", "DO NOT DISPLAY", "NOT REPORTING", "N/A", "NONE", "SCHIMELPFENIG")
);
private ParseUtilHelper() {
//private constructor because class should not be instantiated
}
public static boolean isPositive(String v) {
return positiveValues.contains(v);
}
public static boolean isNegative(String v) {
return negativeValues.contains(v) || v.contains("DEFERRED");
}
//snip
}
For starters, we redefine constants that exist over in our BooleanParseUtil
, which, I mean, maybe we could use different strings for TRUE
and FALSE
in this object, because that wouldn't be confusing at all.
But the real takeaway is that we have absolutely ALL of the boolean values. TRUE, YES, OK, DO NOT DISPLAY, and even SCHIMELPFENIG, the falsest of false values. That said, I can't help but think there's one missing.
In truth, this is exactly the sort of code that happens when you have a cross-organization data integration task with no schema. And while I'm sure the end users are quite happy to continue doing this in Excel, the only tool they care about using, there are many, many other possible ways to send that data around. I suppose we should just be happy that the process wasn't built using XML? I'm kidding, of course, even XML would be an improvement.
Метки: CodeSOD |
Error'd: Choice is but an Illusion |
"If you choose not to decide which button to press, you still have made a choice," Rob H. wrote.
"If you have a large breed cat, or small dog, the name doesn't matter, it just has to get the job done," writes Bryan.
Mike R. wrote, "Thanks Dropbox. Becuase your survey can't add, I missed out on my chance to win a gift card. Way to go guys..."
"There was a magnitude 7.1 earthquake near Ridgecrest, CA on 7/5/2019 at 8:25PM PDT. I visited the USGS earthquakes page, clicked on the earthquake link, and clickedd on the 'Did you feel it?' link, because we DID feel it here in Sacramento, CA, 290 miles away," Ken M. wrote, "Based on what I'm seeing though, I think they may call it a 'bat-quake' instead."
Benjamin writes, "Apparently Verizon is trying to cast a spell on me because I used too much data."
Daniel writes, "German telekom's customer center site is making iFrames sexy again."
Метки: Error'd |
CodeSOD: Close to the Point |
Lena inherited some C++ code which had issues regarding a timeout. While skimming through the code, one block in particular leapt out. This was production code which had been running in this state for some time.
if((pFile) && (pFile != (FILE *)(0xcdcdcdcd))) {
fclose(pFile);
pFile = NULL;
}
The purpose of this code is, as you might gather from the call to fclose
, to close a file handle represented by pFile
, a pointer to the handle. This code mostly is fine, but with one, big, glaring “hunh?” and it’s this bit here: (pFile != (FILE *)(0xcdcdcdcd))
(FILE *)(0xcdcdcdcd)
casts the number 0xcdcdcdcd
to a file pointer- essentially it creates a pointer pointing at memory address 0xcdcdcdcd. If pFile
points to that address, we won’t close pFile
. Is there a reason for this? Not that Lena could determine from the code. Did the 0xcdcdcdcd
come from anywhere specific? Probably a previous developer trying to track down a bug and dumping addresses from the debugger. How did it get into production code? How long had it been there? It was impossible to tell. It was also impossible to tell if it was secretly doing something important, so Lena made a note to dig into it later, but focused on solving the timeout bug which had started this endeavor.
Метки: CodeSOD |
CodeSOD: What a Happy Date |
As is the case with pretty much any language these days, Python comes with robust date handling functionality. If you want to know something like what the day of the month is? datetime.now().day
will tell you. Simple, easy, and of course, just an invitation for someone to invent their own.
Jan was witness to a little date-time related office politics. This particular political battle started during a code review. Klaus had written some date mangling code, relying heavily on strftime
to parse dates out to strings and then parse them back in as integers. Richard, quite reasonably, pointed out that Klaus was taking the long way around, and maybe Klaus should possibly think about doing it in a simpler fashion.
“So, you don’t understand the code?” Klaus asked.
“No, I understand it,” Richard replied. “But it’s far too complicated. You’re doing a simple task- getting the day of the month! The code should be simple.”
“Ah, so it’s too complicated, so you can’t understand it.”
“Just… write it the simple way. Use the built-in accessor.”
So, Klaus made his revisions, and merged the revised code.
import datetime
# ...
now = datetime.datetime.now() # Richard
date = now.strftime("%d") # Richard, this is a string over here
date_int = int(date) # day number, int("08") = 8, so no problem here
hour = now.hour # Richard :)))))
hour_int = int(hour) # int hour, e.g. if it's 22:36 then hour = 22
Richard did not have a big :)))))
on his face when he saw that in the master branch.
Метки: CodeSOD |
This Process is Nuts |
A great man once said "I used to be over by the window, and I could see the squirrels, and they were merry." As pleasing of a sight as that was, what if the squirrels weren't merry?
Grady had an unpleasant experience with bushy-tailed rodents at a former job. Before starting at the Fintech firm as a data scientist, he was assured the Business Intelligence department was very advanced and run by an expert. They needed Grady to manipulate large data sets and implement machine learning to help out Lenny, the resident BI "expert". It quickly became apparent that Lenny didn't put the "Intelligence" in Business Intelligence.
Lenny was a long-term contractor who started the BI initiative from the ground-up. His previous work as a front-end developer led to his decision to use PHP for the ETL process. This one-of-a-kind monstrosity made it as unstable as a house of cards in a hurricane and the resultant data warehouse was more like a data cesspool.
"This here is the best piece of software in the whole company," Lenny boasted. "They tell me you're really smart, so you'll figure out how it works on your own. My work is far too important and advanced for me to be bothered with questions!" Lenny told Grady sternly.
Grady, left to fend for himself, spent weeks stumbling through code with very few comments and no existing documentation. He managed to deduce the main workflow for the ETL and warehouse process and it wasn't pretty. The first part of the ETL process deleted the entire existing data warehouse, allowing for a "fresh start" each day. If an error occurred during the ETL, rather than fail gracefully, the whole process crashed without restoring the data warehouse that was wiped out.
Grady found that the morning ETL run failed more often than not. Since Lenny never bothered to stroll in until 10 AM, the people that depended on data warehouse reports loudly complained to Grady. Having no clue how to fix it, he would tell them to be patient. Lenny would saunter in and start berating him "Seriously? Why haven't you figured out how to fix this yet?!" Lenny would spend an hour doing damage control, then disappear for a 90 minute lunch break.
One day, an email arrived informing everyone that Lenny was no longer with the company after exercising an obscure opt-out clause in his contract. Grady suddenly became the senior-most BI developer and inherited Lenny's trash pile. Determined to find the cause of the errors, he dug into parts of the code Lenny strictly forbade him to enter. Hoping to find any semblance of logging that might help, he scoured for hours.
Grady finally started seeing commands called "WritetoSkype". It sounded absurd, but it almost seemed like Lenny was logging to a Skype channel during the ETL run. Grady created a Skype account and subscribed to LennysETLLogging. All he found there was a bunch of dancing penguin emoticons, written one at a time.
Grady scrolled and scrolled and scrolled some more as thousands of dancing penguins written during the day's run performed for him. He finally reached the bottom and found an emoticon of a squirrel eating an acorn. Looking back at the code, WritetoSkype sent (dancingpenguin) when a step succeeded and (heidy) when a step failed. It was far from useful logging, but Grady now had a clear mission - Exterminate all the squirrels.
Метки: Feature Articles |
CodeSOD: Some Kind of Magic |
We all have our little bits of sloppiness and our bad habits. Most of us have more than one. One place I'm likely to get lazy, especially as I'm feeling my way around a problem, is with magic numbers. I always mean to go back and replace them with a constant, but sometimes there's another fire you need to put out and you just don't get back to it till somebody calls it out in a code review.
Then, of course, there are the folks who go too far. I once got a note complaining that I shouldn't have used 2*PI
, but instead should have created a new constant, TAU
. I disavow the need for tau, but my critic said magic numbers, like two, were disallowed, so I said "ciao" and tau is there now.
Angela A, who's had some experience with bad constants before, has found a new one.
// Decimal constant for value of 1
static constant float THIRTY = 30.0f;
The decimal constant for the value of 1 is THIRTY.
Метки: CodeSOD |
Error'd: Nice Day for Golf (in Hades) |
"A coworker was looking up what the weather was going to be like for his tee time. He said he’s definitely wearing shorts," writes Angela A.
"I guess whenever a company lists welding in their LinkedIn job posting you know that they're REEAALLY serious about computer hardware," Andrew I. writes.
Chris A. wrote, "It was game, set, and match, but unfortunately, someone struck out."
Bruce C. writes, "I'm not surprised that NULL is missing some deals....that File Not Found person must be getting it all."
"Learning to use Docker with the 'Get Started' tutorials and had to wonder...is there some theme here?" Dave E. wondered.
"Ever type up an email and hit 'send' too early? Well...here's an example," writes Charlie.
Метки: Error'd |
CodeSOD: Null Thought |
These days, it almost seems like most of the developers writing code for the Java Virtual Machine aren’t doing it in Java. It’s honestly an interesting thing for programming language development, as more and more folks put together languages with fundamentally different principles from Java that still work on the JVM.
Like Kotlin. Kotlin blends functional programming styles with object-oriented principles and syntactic features built around writing more compact, concise code than equivalent Java. And it’s not even limited to Java- it can compile down into JavaScript or even target LLVM.
And since you can write bad code in any language, you can write bad code in Kotlin. Keith inherited a Kotlin-developed Android app.
In Kotlin, if you wanted to execute some code specifically if a previous step failed, you might use a try/catch exception handler. It’s built into Kotlin. But maybe you want to do some sort of error handling in your pipeline of function calls. So maybe you want something which looks more like:
response.code
.wasSuccess()
.takeIf { false }
?.run { doErrorThing(it) }
wasSuccess
in this example returns a boolean. The takeIf
checks to see if the return value was false
- if it wasn’t, the takeIf
returns a null, and the run
call doesn’t execute (the question mark is our nullable operator).
Kotlin has a weird relationship with nulls, and unless you’re explicit about where you expect nulls, it is going to complain at you. Which is why Keith got annoyed at this block:
/**
* Handles error and returns NULL if an error was found, true if everything was good
*/
inline fun Int.wasSuccessOrNull() : Boolean? {
return if (handleConnectionErrors(this))
null
else
true
}
/**
* Return true if any connection issues were found, false if everything okay
*/
fun handleConnectionErrors(errorCode: Int) : Boolean {
return when (errorCode)
{
Error.EXPIRED_TOKEN -> { requiresSignIn.value = true; true}
Error.NO_CONNECTION -> { connectionIssue.value = true; true}
Error.INACTIVE_ACCOUNT -> { inactiveAccountIssue.value = true; true}
Error.BAD_GATEWAY -> { badGatewayIssue.value = true; true}
Error.SERVICE_UNAVAILABLE -> { badGatewayIssue.value = true; true}
else -> {
if (badGatewayIssue.value == true) {
badGatewayIssue.value = false
}
noErrors.value = true
false
}
}
}
wasSuccessOrNull
returns true, if the status code is successful, otherwise it returns… null? Why a null? Just so that a nullable ?.run…
call can be used? It’s a weird choice. If we’re just going to return non-true/false values from our boolean methods, there are other options we could use.
But honestly, handleConnectionErrors
, which it calls, is even more worrisome. If an error did occur, this causes a side effect. Each error condition sets a variable outside of the scope of this function. Presumably these are class members, but who knows? It could just as easily be globals.
If the error code isn’t an error, we explicitly clear the badGatewayIssue
, but we don’t clear any of the other errors. Presumably that does happen, somewhere, but again, who knows? This is the kind of code where trying to guess at what works and what doesn’t is a bad idea.
Метки: CodeSOD |
CodeSOD: A Long Conversion |
Let’s talk a little bit about .NET’s TryParse
method. Many types, especially the built in numerics, support it, alongside a Parse
. The key difference between Parse
and TryParse
is that TryParse
bakes the exception handling logic in it. Instead of using exceptions to tell you if it can parse or not, it returns a boolean value, instead.
If, for example, you wanted to take an input, and either store it as an integer in a database, or store a null, you might do something like this:
int result;
if (int.TryParse(data, out result)) {
rowData[column] = result;
} else {
rowData[column] = DBNull.Value;
}
There are certainly better, cleaner ways to handle this. Russell F. has a co-worker that has a much uglier way to handle this.
private void BuildIntColumns(string data, DataRow rowData, int startIndex, int length, string columnName, FileInfo file, string tableName)
{
if (data.Trim().Length > startIndex)
{
try
{
int resultOut;
if (data.Substring(startIndex, length).Trim() == "" || string.IsNullOrEmpty(data.Substring(startIndex, length).Trim()))
{
rowData[columnName] = DBNull.Value;
}
else if (int.TryParse(data.Substring(startIndex, length).Trim(), out resultOut) == false)
{
rowData[columnName] = DBNull.Value;
}
else
{
rowData[columnName] = Convert.ToInt32(data.Substring(startIndex, length).Trim());
}
}
catch (Exception e)
{
rowData[columnName] = DBNull.Value;
SaveErrorData(file, data, e.Message, tableName);
}
}
}
private void BuildLongColumns(string data, DataRow rowData, int startIndex, int length, string columnName, FileInfo file, string tableName)
{
if (data.Trim().Length > startIndex)
{
try
{
int resultOut;
if (data.Substring(startIndex, length).Trim() == "" || string.IsNullOrEmpty(data.Substring(startIndex, length).Trim()))
{
rowData[columnName] = DBNull.Value;
}
else if (int.TryParse(data.Substring(startIndex, length).Trim(), out resultOut) == false)
{
rowData[columnName] = DBNull.Value;
}
else
{
rowData[columnName] = Convert.ToInt64(data.Substring(startIndex, length).Trim());
}
}
catch (Exception e)
{
rowData[columnName] = DBNull.Value;
SaveErrorData(file, data, e.Message, tableName);
}
}
}
Here’s a case where the developer knows that methods like int.TryParse
and string.IsNullOrEmpty
exist, but they don’t understand them. More worrying, every operation has to be on a Substring
for some reason, which implies that they’re processing strings which contain multiple fields of data. Presumably that means there’s a mainframe with fixed-width records someplace in the backend, but certainly splitting while converting falls is a bad practice.
For bonus points, compare the BuildIntColumns
with BuildLongColumns
. There’s an extremely subtle bug in the BuildLongColumns
- specifically, they still do an int.TryParse
, but this isn’t an int
, it’s a long
. If you actually tried to feed it a long integer, it would consider it invalid.
Russell adds: “I found this in an old file – no clue who wrote it or how it came to exist (I guess I could go through the commit logs, but that sounds like a lot of work).”
Метки: CodeSOD |
CodeSOD: Break my Validation |
Linda inherited an inner-platform front-end framework. It was the kind of UI framework with an average file size of 1,000 lines of code, and an average test coverage of 0%.
Like most UI frameworks, it had a system for doing client side validation. Like most inner-platform UI frameworks, the validation system was fragile, confusing, and impossible to understand.
This code illustrates some of the problems:
/**
* Modify a validator key, e.g change minValue or disable required
*
* @param fieldName
* @param validatorKey - of the specific validator
* @param key - the key to change
* @param value - the value to set
*/
modifyValidatorValue: function (fieldName, validatorKey, key, value) {
if (!helper.isNullOrUndefined(fieldName)) {
// Iterate over fields
for (var i in this.fields) {
if (this.fields.hasOwnProperty(i)) {
var field = this.fields[i];
if (field.name === fieldName) {
if (!helper.isNullOrUndefined(validatorKey)) {
if (field.hasOwnProperty('validators')) {
// Iterate over validators
for (var j in field.validators) {
if (field.validators.hasOwnProperty(j)) {
var validator = field.validators[j];
if (validator.key === validatorKey) {
if (!helper.isNullOrUndefined(key) && !helper.isNullOrUndefined(value)) {
if (validator.hasOwnProperty(key)) {
validator[key] = value;
}
}
break;
}
}
}
}
}
break;
}
}
}
}
}
What this code needs to do is find the field for a given name, check the list of validators for that field, and update a value on that validator.
Normally, in JavaScript, you’d do this by using an object/dictionary and accessing things directly by their keys. This, instead, iterates across all the fields on the object and all the validators on that field.
It’s smart code, though, as the developer knew that once they found the fields in question, they could exit the loops, so they added a few break
s to exit. I think those break
s are in the right place. To be sure, I’d need to count curly braces, and I’m worried that I don’t have enough fingers and toes to count them all in this case.
According to the git log, this code was added in exactly this form, and hasn’t been touched since.
Метки: CodeSOD |
An Indispensible Guru |
Business Intelligence is the oxymoron that makes modern capitalism possible. In order for a company the size of a Fortune 500 to operate, key people have to know key numbers: how the finances are doing, what sales looks like, whether they're trending on target to meet their business goals or above or below that mystical number.
Once upon a time, Initech had a single person in charge of their Business Intelligence reports. When that person left for greener pastures, the company had a problem. They had no idea how he'd done what he did, just that he'd gotten numbers to people who'd needed them on time every month. There was no documentation about how he'd generated the numbers, nothing to pass on to his successor. They were entirely in the dark.
Recognizing the weakness of having a single point of failure, they set up a small team to create and manage the BI reporting duties and to provide continuity in the event that somebody else were to leave. This new team consisted of four people: Charles, a senior engineer; Liam, his junior sidekick; and two business folks who could provide context around what numbers were needed where and when.
Charles knew Excel. Intimately. Charles could make Excel do frankly astonishing things. Our submitter has worked in IT for three decades, and yet has never seen the like: spreadsheets so chock-full with array formulae, vlookups, hlookups, database functions, macros, and all manner of cascading sheets that they were virtually unreadable. Granted, Charles also had Microsoft Access. However, to Charles, the only thing Access was useful for was the initial downloading of all the raw data from the IBM AS/400 mainframe. Everything else was done in Excel.
Nobody doubted the accuracy of Charles' reports. However, actually running a report involved getting Excel primed and ready to go, hitting the "manual recalculate" button, then sitting back and waiting 45 minutes for the various formulae and macros to do all the things they did. On a good day, Charles could run five, maybe six reports. On a bad day? Three, at best.
By contrast, Liam was very much the "junior" role. He was younger, and did not have the experience that Charles did. That said, Liam was a smart cookie. He took one look at the spreadsheet monstrosity and knew it was a sledgehammer to crack a walnut. Unfortunately, he was the junior member of the engineering half of the team. His objections were taken as evidence of his inexperience, not his intelligence, and his suggestions were generally overlooked.
Eventually, Charles also left for bigger and brighter things, and Liam inherited all of his reports. Almost before the door had stopped swinging, Liam solicited our submitter's assistance in recreating just one of Charles' reports in Access. This took a combined four days; it mostly consisted of the submitter asking "So, Sheet 1, cell A1 ... where does that number come from?", and Liam pointing out the six other sheets they needed to pivot, fold, spindle, and mutilate in order to calculate the number. "Right, so, Sheet 1, cell A2 ... where does that one come from?" ...
Finally, it was done, and the replacement was ready to test. They agreed to run the existing report alongside the new one, so they could determine that the new reports were producing the same output as the old ones. Liam pressed "manual recalculate" while our submitter did the honors of running the new Access report. Thirty seconds later, the Access report gave up and spat out numbers.
"Damn," our submitter muttered. "Something's wrong, it must have died or aborted or something."
"I dunno," replied Liam, "those numbers do look kinda right."
Forty minutes later, when Excel finally finished running its version, lo and behold the outputs were identical. The new report was simply three orders of magnitude faster than the old one.
Enthused by this success, they not only converted all the other reports to run in Access, but also expanded them to run Region- and Area- level variants, essentially running the report about 54 times in the same time it took the original report to run once. They also set up an automatic distribution process so that the reports were emailed out to the appropriate department heads and sales managers. Management was happy; business was happy; developers were happy.
"Why didn't we do this sooner?" was the constant refrain from all involved.
Liam was able to give our submitter the real skinny: "Charles used the long run times to prove how complex the reports were, and therefore, how irreplaceable he was. 'Job security,' he used to call it."
To this day, Charles' LinkedIn profile shows that he was basically running Initech. Liam's has a little more humility about the whole matter. Which just goes to show you shouldn't undersell your achievements in your resume. On paper, Charles still looks like a genius who single-handedly solved all the BI issues in the whole company.
Метки: Feature Articles |
Error'd: The Parameter was NOT Set |
"Spotted this in front of a retro-looking record player in an Italian tech shop. I don't think anybody had any idea how to categorize it so they just left it up to the system," Marco D. writes.
George C. wrote, "Never thought it would come to this, but it looks like LinkedIn can't keep up with all of my connections!"
"Apparently opening a particular SharePoint link using anything else other than Internet Explorer made Excel absolutely lose its mind," wrote Arno P.
Dima R. writes, "OH! My bad, Edge, I only tried to access a file:// URL while I was offline."
"This display at the Vancouver airport doesn't have a lot of fans," Bruce R. wrote.
"Woo hoo! This is what I'm talking about! A septillion percentage gain!!" John G. writes.
Метки: Error'd |
The Hardware Virus |
Jen was a few weeks into her new helpdesk job. Unlike past jobs, she started getting her own support tickets quickly—but a more veteran employee, Stanley, had been tasked with showing her the ropes. He also got notification of Jen's tickets, and they worked on them together. A new ticket had just come in, asking for someone to replace the DVI cable that'd gone missing from Conference Room 3. Such cables were the means by which coworkers connected their laptops to projectors for presentations.
Easy enough. Jen left her cube to head for the hardware "closet"—really, more of a room crammed full of cables, peripherals, and computer parts. On a dusty shelf in a remote corner, she spotted what she was looking for. The coiled cable was a bit grimy with age, but looked serviceable. She picked it up and headed to Stanley's cube, leaning against the threshold when she got there.
"That ticket that just came in? I found the cable they want. I'll go walk it down." Jen held it up and waggled it.
Stanley was seated, facing away from her at first. He swiveled to face her, eyed the cable, then went pale. "Where did you find that?"
"In the closet. What, is it—?"
"I thought they'd been purged." Stanley beckoned her forward. "Get in here!"
Jen inched deeper into the cube. As soon as he could reach it, Stanley snatched the cable out of her hand, threw it into the trash can sitting on the floor beside him, and dumped out his full mug of coffee on it for good measure.
"What the hell are you doing?" Jen blurted.
Stanley looked up at her desperately. "Have you used it already?"
"Uh, no?"
"Thank the gods!" He collapsed back in his swivel chair with relief, then feebly kicked at the trash can. The contents sloshed around inside, but the bin remained upright.
"What's this about?" Jen demanded. "What's wrong with the cable?"
Under the harsh office lighting, Stanley seemed to have aged thirty years. He motioned for Jen to take the empty chair across from his. Once she'd sat down, he continued nervously and quietly. "I don't know if you'll believe me. The powers-that-be would be angry if word were to spread. But, you've seen it. You very nearly fell victim to it. I must relate the tale, no matter how vile."
Jen frowned. "Of what?"
Stanley hesitated. "I need more coffee."
He picked up his mug and walked out, literally leaving Jen at the edge of her seat. She managed to sit back, but her mind was restless, wondering just what had her mentor so upset.
Eventually, Stanley returned with a fresh mug of coffee. Once he'd returned to his chair, he placed the mug on his desk and seemed to forget all about it. With clear reluctance, he focused on Jen. "I don't know where to start. The beginning, I suppose. It fell upon us from out of nowhere. Some say it's the spawn of a Sales meeting; others blame a code review gone horribly wrong. In the end, it matters little. It came alive and spread like fire, leaving destruction and chaos in its wake."
Jen's heart thumped with apprehension. "What? What came alive?"
Stanley's voice dropped to a whisper. "The hardware virus."
"Hardware virus?" Jen repeated, eyes wide.
Stanley glared. "You're going to tell me there's no such thing, but I tell you, I've seen it! The DVI cables ..."
He trailed off helplessly, reclining in his chair. When he straightened and resumed, his demeanor was calmer, but weary.
"At some godforsaken point in space and time, a single pin on one of our DVI cables was irrevocably bent. This was the source of the contagion," he explained. "Whenever the cable was plugged into a laptop, it cracked the plastic composing the laptop's DVI port, contorting it in a way that resisted all mortal attempt at repair. Any time another DVI cable was plugged into that laptop, its pin was bent in just the same way as with the original cable.
"That was how it spread. Cable infected laptop, laptop infected cable, all with vicious speed. There was no hope for the infected. We ... we were forced to round up and replace every single victim. I was knee-deep in the carnage, Jen. I see it in my nightmares. The waste, the despair, the endless reimaging!"
Stanley buried his head in his hands. It was a while before he raised his haunted gaze again. "I don't know how long it took, but it ran its course; the support tickets stopped coming in. Our superiors consider the matter resolved ... but I've never been able to let my guard down." He glanced warily at the trash can, then made eye contact with Jen. "Take no chances with any DVI cables you find within this building. Buy your own, and keep them with you at all times. If you see any more of those—" he pointed an accusing finger at the bin "—don't go near them, don't try taking a paperclip to them. There's everything to lose, and nothing to gain. Do you understand?"
Unable to manage words, Jen nodded instead.
"Good." The haunted expression vanished in favor of grim determination. Stanley stood, then rummaged through a desk drawer loaded with office supplies. He handed Jen a pair of scissors, and armed himself with a brassy letter opener.
"Our job now is to track down the missing cable that resulted in your support ticket," he continued. "If we're lucky, someone's absent-mindedly walked off with it. If we're not, we may find that this is step one in the virus' plan to re-invade. Off we go!"
Jen's mind reeled, but she sprang to her feet and followed Stanley out of the cubicle, telling herself to be ready for anything.
Метки: Feature Articles |