CodeSOD collection

robo2

@BernieTheBernie uhm... https://dotnetfiddle.net/1nyq5j

BernieTheBernie

So, during the last days, Jenkins used to hang. Some where in the unit test section. Repeatedly. Not necessarily a specific test, but always in the test section.
The last time without any failing tests was back in ... summer 2019, I think. While I was on sick leave for week after a meeting where was present who told us about how important quality is blah blah blah some cow-orker destroyed some of my code and a test of it. I thanked him for his great achievement, and Fritz (head of quality) showed me the finger for doing so. I decided never to clean up their mess again. That is, the test still fails. 30 months later.
Of course, now the number of failing tests just increases. I do not know how many failures are my product already - it's just too big a number of failing tests to find the freshly broken one (also Jenkins fails at that!).
So, some how, there are some bugs in the tests which make Jenkins hang during the tests.
Kevin solved that: he took the list of all failing tests and commented them out.
Solved.

Applied Mediocrity

@BernieTheBernie This works well enough if Jenkins is assumed to be the name of one of your cow-orkers.

Arantor

@Applied-Mediocrity also if their name is Hudson.

BernieTheBernie

@Arantor You'd flush them down that river.

HardwareGeek

@BernieTheBernie said in CodeSOD collection:

Jenkins used to hang. ...
Kevin solved that:

I suggest hanging Kevin.

BernieTheBernie

@HardwareGeek Not the frist time I hear that suggestion...
But, in the end, we would miss some fun stories on TDWTF then...

topspin

Fuck, I hate other people's code.

	/** \brief Initialize operators.
	  By default only the unary minus operator is added.
	*/
	void Parser::InitOprt()
	{
		DefineInfixOprt(_T("-"), MathImpl<value_type>::UnaryMinus);
		DefineInfixOprt(_T("+"), MathImpl<value_type>::UnaryPlus);
	}

About a decade ago a cow-orker added a third party dependency for expression parsing.
Since nobody had to touch it and "it works", that's not been a problem so far. But I want to add some more stuff now and have to look ~~into the abyss~~ at the code. The style of the whole thing is well represented by the above snippet where someone apparently added a second line to a one line function, but of course didn't bother to update the also one line of documentation immediately above it.

It's also full of niceties like old-style non-discriminated unions, except for the one place a union would be more correct, i.e. where they store a "generic" function pointer that could be any of a certain number of function pointer types, but they store it as a void* instead, which is totally UB.

  /** \brief Callback type used for functions without arguments. */
  typedef value_type (*generic_fun_type)();
  /** \brief Callback type used for functions without arguments. */
  typedef value_type (*fun_type0)();
  /** \brief Callback type used for functions with a single arguments. */
  typedef value_type (*fun_type1)(value_type);
  /** \brief Callback type used for functions with two arguments. */
  typedef value_type (*fun_type2)(value_type, value_type);

  // [loads more]

// [... somewhere else ...]
    void *m_pFun;                   ///< Pointer to the callback function, casted to void

The whole thing is just yuck.

Now, of course, I'm not really surprised because said former cow-orker also had absolutetly no taste in code and would've probably written even worse things. And a quick google search comes up with half a dozen other libraries, but all of them (without actually investigating) also give me a feeling they're not particulary well designed, either. And I don't want to bring in some boost abomination (or roll my own with boost::spirit).

BernieTheBernie

@topspin Seems to be a lot of fun to work with generic_fun_type etc. I guess you don't value the value_types either.

BernieTheBernie

What kind of enum is that meant to be?

internal static class FieldType
{
    internal const string GROUP_MAIN = "Mainboard";
    internal const string GROUP_MOTOR = "Motor";
    internal const string GROUP_SENSORS = "Sensors";
    internal const string GROUP_CONTACTS = "Contacts";
    internal const string GROUP_INFRARED = "Infrared";
    internal const string GROUP_VIDEO = "Video";
    internal const string GROUP_FIRMWARE_MAINBOARD = "FirmwareMainboard";
    internal const string GROUP_FIRMWARE_MOTOR = "FirmwareMotor";

    internal const string MAINBOARD_HARDWARE_VERSION = "01HardwareVersion";
    internal const string MAINBOARD_FIRMWARE_VERSION = "02FirmwareVersion";
    internal const string MAINBOARD_EXT_E_EPROM = "03ExtEEprom";
    internal const string MAINBOARD_TIMEOUT_PS_SERVER ="05TimeoutPsServer";
    internal const string MAINBOARD_TIMEOUT_CONTACTS = "06TimeoutContacts";
    internal const string MAINBOARD_WATCHDOG_PS_SERVER = "08WatchdogPsServer";
    internal const string MAINBOARD_WATCHDOG_CONTACTS = "09WatchdogContacts";
    internal const string MAINBOARD_CODE = "0";

    internal const string SENSORS_READ = "20Read";
    internal const string SENSORS_ELECTRIC_VOLTAGE = "21ElectricVoltage";
    internal const string SENSORS_MAINBOARD_TEMPERATURE = "22MainboardTemperature";
    internal const string SENSORS_CAMERA_TEMPERATURE = "23CameraTemperature";
    internal const string SENSORS_AIR_TEMPERATURE= "24AirTemperature";
    internal const string SENSORS_AIR_FLOW = "25AirFlow";
    internal const string SENSORS_REF_SPOT = "26RefSpot";
    internal const string SENSORS_REF_SPAN = "27RefSpan";
    internal const string SENSORS_REF_ACTIVE = "28RefActive";

Etc etc.
Don't expect Kevin to even understand the basics of the enum concept of C#.

dkf

@topspin said in CodeSOD collection:

they store a "generic" function pointer that could be any of a certain number of function pointer types, but they store it as a void* instead, which is totally UB.

It's not UB on POSIX or Windows. Those platforms define data and function pointers to be the same size (because of how they do dynamic library loading).

topspin

@dkf true for free functions, but doesn't make it less ugly.
They could at least store the typedef value_type (*generic_fun_type)(); that's already named for this purpose instead of using a void*, even though a union would correctly work out the actual size required.

PleegWat

@topspin IIRC that would be invalid. The only meaningful cast for a function pointer is to void, and the only valid use for the resulting void pointer is casting it back to the exact same function type.

BernieTheBernie

What's a property?
Look at that special new idea by Kevin:

internal byte TestFlag
{
    get
    {
        if (m_TestFlag == 0)
        {
            m_TestFlag = 1;
        }
        else
        {
            m_TestFlag = 0;
        }
        UpdateFlag();
        return m_TestFlag;
    }
}

No setter. But the value changes whenever you call it. And has some side effects (UpdateFlag makes some other crap, too).

topspin

@BernieTheBernie the Principle of Most Surprise at work.

HardwareGeek

@topspin said in CodeSOD collection:

@BernieTheBernie the Principle of Most Surprise at work.

Which is not surprising to people who know Kevin.

Bulb

@topspin said in CodeSOD collection:

but of course didn't bother to update the also one line of documentation immediately above it.

Like about all code, about everywhere. I've come to think internal functions should not be documented, because the comments will end up being stale sooner or later and everybody will be ignoring them anyway.

This does look like a library that should be documented though, so that makes it worse.

@topspin said in CodeSOD collection:

About a decade ago a cow-orker added a third party dependency for expression parsing.

So it was added before (or just around) C++11 and since it already existed for some time by then, it was definitely written in pre-11 C++. And if the people who worked on it were a bit behind the development of the modern C++ style, it was really written in C-with-classes. The C-style idioms were par for the course back then.

@topspin said in CodeSOD collection:

I don't want to bring in some boost abomination

If you can use C++17, you could modernize to std::variant for tagged unions. And use std::function for function pointers unless you can't afford the polymorphic adapter it creates internally (which on the other hand allows it to take functors and closures.

topspin

@Bulb said in CodeSOD collection:

If you can use C++17, you could modernize to std::variant for tagged unions. And use std::function for function pointers unless you can't afford the polymorphic adapter it creates internally (which on the other hand allows it to take functors and closures.

By the time I "upgrade" someone else's code (there are newer versions of the library available, they're marginally more modern), I might as well roll my own.
I'm currently pondering my decision not to do that, but I really can't justify spending time rebuilding something that already "works". Maybe I'll ~~do it in my spare time~~subtract it from my TDWTF browsing budget.

Zerosquare

@Bulb said in CodeSOD collection:

you could modernize to std::variant

But if so, make sure that you are using protection.

Bulb

@dkf said in CodeSOD collection:

Also not enough inclined to murder his babies (i.e., to reconsider, rework and/or throw away code and design decisions that he was particularly proud of).

@Gribnit said in CodeSOD collection:

I attribute this to a loss of faith in Moloch in recent generations.

Fittingly, in Czech the word “moloch” is used as a derogatory term for something huge, dehumanized or difficult to adjust like a big bureaucracy, corporation, … or large application that nobody understands anymore.

PleegWat

@Bulb said in CodeSOD collection:

@dkf said in CodeSOD collection:

Also not enough inclined to murder his babies (i.e., to reconsider, rework and/or throw away code and design decisions that he was particularly proud of).

@Gribnit said in CodeSOD collection:

I attribute this to a loss of faith in Moloch in recent generations.

Fittingly, in Czech the word “moloch” is used as a derogatory term for something huge, dehumanized or difficult to adjust like a big bureaucracy, corporation, … or large application that nobody understands anymore.

From memory ( to look it up) the name occurs in the new testament, Jesus claiming you cannot worship both Him and the Moloch. I seem to recall he is a god of currency, greed, or something along those lines.

HardwareGeek

@PleegWat You're thinking of Mammon. Moloch is a pagan god mentioned in the Old Testament, the worship of whom seems to have involved human (child) sacrifice. Needless to say, the Judaeo-Christian God strongly condemns worship of the pagan god.

Moloch - Wikipedia

PleegWat

@HardwareGeek Ah, of course. Thanks.

Arantor

@HardwareGeek reminds me of Indiana Jones and the Fate of Atlantis with the little Atlantean statue in the intro sequence.

Edit, found it:

Zerosquare

This might not be a coincidence. The art in that game was inspired by a mix of real-world mythologies.

Arantor

@Zerosquare Wouldn't surprise me, especially given that Atlantis is implied to be pre-Christian in FoA.

Dammit why couldn't we have had FoA as the fourth Indy outing? So much better than fucking Crystal Skull Kingdom or whatever it's called. A Hollywood big budget Fate of Atlantis with Ford himself? I'd have so been in for that. Still would, really.

Zerosquare

@Arantor said in CodeSOD collection:

Dammit why couldn't we have had FoA as the fourth Indy outing? So much better than fucking Crystal Skull Kingdom or whatever it's called. A Hollywood big budget Fate of Atlantis with Ford himself? I'd have so been in for that. Still would, really.

Damn right. And I'd love to hear the music played by a real orchestra, too.

But of course, that will never happen. Especially now that franchise is owned by Disney, which won't do anything with it beside sending cease-and-desist letters to fans.

Arantor

@Zerosquare are we pretending there isn’t a fifth film coming that may well be as trashy as the fourth?

Zerosquare

Not all countries whose names include "democratic" are real democracies.
Not all movies whose names include "Indiana Jones" are real Indiana Jones movies

Filed under: the Mission Impossible effect

Arantor

@Zerosquare not all movies that contain Indiana Jones that are real Indiana Jones movies contain the words “Indiana” or “Jones” in the title.

homoBalkanus

@Arantor there were only ever 3 films

HardwareGeek

@homoBalkanus I've been reading the James Bond thread, and initially I put your post in that context. I was trying to figure out which 3 out of the 25 you might deem worthy.

Arantor

@homoBalkanus yes - Raiders of the Lost Ark, Indiana Jones and the Temple of Doom, Indiana Jones and the Last Crusade.

Thus my point: not all movies that contain Indiana Jones that are real Indy movies contain either of the words of the protagonist's name in the title

(The original movie poster for Raiders clearly indicates the intended title as 'Raiders of the Lost Ark' and not 'Indiana Jones and the Raiders of the Lost Ark' even if later iterations of the poster did list it as that; the original had a tagline such as "Indiana Jones - the new hero from the creators of JAWS and STAR WARS' rendering his name in the movie title unnecessary)

Tsaukpaetra

status: wondering when Indiana came and swapped code SOD conversation with self-flagellation....

Gribnit

@Tsaukpaetra said in CodeSOD collection:

status: wondering when Indiana came and swapped code SOD conversation with self-flagellation....

This thread must have have been built over some prior thread, of archeological interest. Like Paris and its sewers, but without the Paris.

izzion

@Tsaukpaetra said in CodeSOD collection:

status: wondering when Indiana came and swapped code SOD conversation with self-flagellation....

Indiana Jones and the Wankers on the Internet

Zecc

What we need is an indie Indy movie.

Arantor

@Zecc I also wanted the indie fan game centered around the Fountain of Youth to emerge but alas it did not.

Carnage

@Arantor said in CodeSOD collection:

@Zerosquare are we pretending there isn’t a fifth film coming that may well be as trashy as the fourth?

If we're lucky, it'll be as trashy as the fourth. It's probably going to be a lot worse.

Tsaukpaetra

status: I smell a smell...

Tsaukpaetra

@Tsaukpaetra said in CodeSOD collection:

I smell a smell...

Top: A little-known site known for blue birds.
Bottom: JSON ALL THE THINGS!

filed under: Teams, why you do that?

topspin

@Tsaukpaetra said in CodeSOD collection:

Teams

BernieTheBernie

Browsing StackOverflow can be fun, too.
Ever considered that your Guid may be not so unique?
Well, look at this idea:

Is checking if a new GUID exists before insert a code smell?

Say I use GUIDs as keys. Considering the chance of a duplicate GUID being generated being what they are, is code like this considered a code smell? Guid newID = GenerateGuid(); while (dictionary.Ha...

BernieTheBernie

Let me start a new chapter: well-crafted, cleanly designed code.
We need some "patterns". As in: the Gang-of-Four-Patterns. But now: the Gang-of-WTF-Patterns.

I suggest to take a look at Hungarian notation. Actually, it has almost nowhere been used correctly. Because, instead of making wrong code look wrong, it was abused to indicate a variable's type. Like strA, intB, objC, etc.

We can use those pseudo-hungarian type indicators for causing confusion. What do you think of string dtX = "Blah!";? If you see that variable later on in the code, you may think it's some DateTime. But it isn't, it is a string (and also Intellisense could tell you).

Unfortunately, the declaration explicitly says that it is a string. C# offers an intersting feature: compiler inferred types. Still strictly typed, but you do not have tell the type explicitly: var dtX = "Blah!";

Generally, you can split the variable declaration from its assignment, like

string dtX;
// some code which does not use dtX
dtX = "Blah";

With compiler inferred types (var), that does not work. Anyway, some confusion has been achieved. Decide for each situation which version fits best.

OK, next step. When you print a DateTime, you ought to specify how you'd like it to be printed. Since the compiler knows that dtX is a string, you can not use the full amount of format specifiers. But some of them:

Console.WriteLine("{0}", dtX.ToString(CultureInfo.GetCultureInfo("en-ZA")));

That prints "Blah!".

And now you can perhaps understand following snippet:

var dtY = 42;
Console.WriteLine("{0}", dtY.ToString("Blah!"));

Have fun with X!

Bulb

@BernieTheBernie said in CodeSOD collection:

As in: the Gang-of-Four-Patterns. But now: the Gang-of-WTF-Patterns.

@BernieTheBernie said in CodeSOD collection:

Actually, it has almost nowhere been used correctly.

That's just as true for the Go patterns as for Hungarian notation and bunch of other programming stuff, really. Basically all of the “enterprise software patterns” is applying something completely inappropriately and out of context.

@BernieTheBernie said in CodeSOD collection:

If you see that variable later on in the code, you may think it's some DateTime. But it isn't, it is a string (and also Intellisense could tell you).

That's like Objections to Hungarian Notation 101: The type and prefix will go out of sync.

@BernieTheBernie said in CodeSOD collection:

C# offers an intersting feature: compiler inferred types. Still strictly typed, but you do not have tell the type explicitly: var dtX = "Blah!";

Most languages have that now. I don't even think C# was a particularly early adopter. It is especially useful when you have some non-trivial nested generics so methods like enumeration return types that would take whole line to spell out and made the code less readable if anything – in C# it happens most often with LINQ.

dkf

@BernieTheBernie said in CodeSOD collection:

And now you can perhaps understand following snippet:
var dtY = 42;
Console.WriteLine("{0}", dtY.ToString("Blah!"));

A method on Integer called ToString that takes a String argument? Why?

Bulb

@dkf Because C#, of course!

(I believe it's the format specification. Three times hooray for stringly typing!)

Bulb

@BernieTheBernie Also

https://youtu.be/FyCYva9DhsI

Filed under: I was wondering whether I can fish it out from history, but fortunately still had a tab with it open.

topspin

@Bulb what an extremely click-baity title. Not going to watch a 70 minute video if they can't bother to tell me what it's about.

Zecc

@BernieTheBernie

You had me at Console.WriteLine("{0}",

@dkf said in CodeSOD collection:

A method on Integer called ToString that takes a String argument? Why?

As @Bulb said, it's the format specifier. DateTime's have one such method as well.