The Daily WTF: Curious Perversions in Information Technology

Mr. TA · 2024-09-19 Reply Admin

Inside the returned dictionary, there's a key value pair containing another dictionary, and so on....

MaxiTB · 2024-09-19 Reply Admin

For the younger generations here around, this article is about the old HttpSessionState class (https://learn.microsoft.com/en-us/dotnet/api/system.web.sessionstate.httpsessionstate?view=netframework-4.8.1).

Son in the spirit of ancient coding practices:

public string RetrieveSessionString(string sessionName)
{
  var result = Session[sessionName];
  return result == null ? null : result.ToString();
}

The second version is just stupid; that's not how indexer work in .net - there doesn't need to be an underlying type or a base type and assuming so even if implemented internally that way pretty much sets you up for future failure. And then if you wrap everything into an exception scope, you will never actually notice internal changes as well - the code just silently breaks and most likely the developer that has to fix that crap will /* insert most horrible murder method */ the person who wrote that initially.

Jaime · 2024-09-19 Reply Admin

Let's also mention that this code doesn't really solve any problems.

The most likely bugs related to accessing Session data; the session has been reset and now the data isn't there, spelling error in session variable name, mishandling the data type. In addition, it would be really useful to easily determine where a particular session variable is used. Creating a method to get any session variable doesn't make any of these situations any easier to handle.

However, if you create a wrapper class and access session variables through it, then you can solve all of them. My most recent implementation of this end up looking like this when used:

var x = State.Session.LoginTime

This allows me to right click on the property and do "Find References" and get all (and only) the code that accesses this one session variable. It allows me to strongly type the property and to write any necessary conversion or access code only once. It allows me to handle the "no data" case on a variable by variable basis. Some should return empty strings if missing, some properties should be nullable, and maybe some should throw exceptions. It lets the compiler validate the variable name.

I can't tell you how many times I've seen these dumb wrapper classes that may reduce a bit of code replication, but don't solve any of the "real problems". The absolute worst part is that so many programmers don't even understand what problems are costing them the most time and creating the most bugs.

2024-09-19 Reply Admin

I have a regular argument with a cow-orker who is of the opinion that code should be self-documenting. Properly named functions and parameters are really what we need, not extraneous comments. While I agree that comments can often be noise (int price; // declare a price integer) programmers are equally disposed to writing incorrectly named functions and parameters. Of course, we've all read this: https://thedailywtf.com/articles/get_words_from_a_number_which_is_passed_as_a_perimeter_into_this_function . Of course, I argue in favor of comments simply because a programmer is at least forced to think about some kind of documentation standard.

2024-09-19 Reply Admin

"...functions which are name RetrieveBlahAsType are generally an antipattern. Either there should be some generics, ..."

Generics didn't appear in .Net until version 2 so it's not impossible that this is framework 1.1 code that never got refactored.

tom103 · 2024-09-19 Reply Admin

Catching a NullReferenceException is gross 🤬. It's not an exception you should catch, it's an exception you don't let happen in the first place. Another WTF is that (Dictionary<string, string>)Session[sessionName] will never throw a NRE. Casting null to a reference type just returns null.

Medinoc · 2024-09-20 Reply Admin

Maybe that's just my hot take- I just hate it when functions return something converted away from its canonical representation; I can do that myself, thank you.

I have two remarks on this:

I do not consider reference type downcasts "conversions away from canonical representation": If anything, the downcasted type is more canonical than Object.
When any step of the conversion or downcast can fail, I much prefer to factorize it in a function where I can centralize the error management or error reporting; it's a lot more informative to say Session variable Foo not found. or Session variable Foo: expected FooType, got BarType. than Object reference not set to an instance of an object.

Addendum 2024-09-20 05:23: I agree about this being best done with generics, though.

Remy Porter · 2024-09-20 Reply Admin

My stance on "self-documenting code" is this: the code is the only reliable source which describes what the code does. Supplementing that with comments, documentation, etc. is good, but those are supplements. We should strive to write self-documenting code because we can't rely on anything else to tell us what the code does.

WTFGuy · 2024-09-21 Reply Admin

Heck, once a system has been in production for more than a couple weeks we can't reliy on any of the documentation to tell us what the code ought to be doing. Every feature list, spec, story board, completed ticket, etc., might have been obsoleted by a user-requested legit change correctly implemented.

A Managed Session

Leave a comment on “A Managed Session”