CodeSOD collection

dkf

@BernieTheBernie said in CodeSOD collection:

Send a ping, and if that fails, do not try to get a TCP connection.

Just watch out for all the firewalls out there that by default drop pings (ICMP) though the TCP port is open just fine.

Particularly a problem if one or other end is in Azure. That's always configured to drop ICMP ECHO messages.

Bulb

@dkf Good to know. Azure is all the rage here. But I mostly switched to tcptraceroute for that very reason anyway.

BernieTheBernie

New challenge. There is an unsigned 16 bit integer number which tells us about 16 boolean on/off states of some device. Now Fritz requires that some of those states are to be tretaed "inverted". That is, he provides a list with the numbers of the items, and when it is in on, it must report off, and the other way round. Of course, also when setting items.

So, with this number being known to represent bits, I could just create an equivalent unsigned 16 bit number from the list of the "inverted" numbers, and do an XOR. That's it (ok, some casting around necessary, because in C#, XOR cannot be applied to ushort).

But ...

You remember enums? They are great. Let's combine them with the Flagsattribute:

[Flags]
private enum B : ushort
{
    b00 = 1,
    b01 = 2,
    b02 = 4,
    ....
    b15 = 32768
}

So, the original number can be represented with this enum (and that does actually fit the meaning much better, ).

Hence, following snippet looks "better":

IEnumerable<byte> bs; // list of bits to be inverted
B tmp = (B)inputValue;
B r = 0;
B[] Bs = ((IEnumerable<B>)Enum.GetValues(typeof(B))).ToArray();
foreach (var value in Bs)
{
    if (tmp.HasFlag((B)value))
    {
        if (!bs.Contains((byte)Bs.IndexOf(value)))
        {
            r |= (B)value;
        }

    }
    else
    {
        if (bs.Contains((byte)Bs.IndexOf(value)))
        {
            r |= (B)value;
        }
    }
}

return (ushort)r;

Bernie wants to be Bad Boy. What about adding some Linq with ternaries?

IEnumerable<byte> bs; // list of bits to be inverted
B tmp = (B)inputValue;
B[] Bs = ((IEnumerable<B>)Enum.GetValues(typeof(B))).ToArray();
var m = ((IEnumerable<B>)Enum.GetValues(typeof(B))).ToList().Aggregate((B)0, (d, s) =>
    tmp.HasFlag((B)s)
        ? !bs.Contains((byte)Bs.IndexOf(s))
            ? s | (B)d
            : d
        : bs.Contains((byte)Bs.IndexOf(s))
            ? s | (B)d
            : d
);
return (ushort)m;

OK. Fine. And now distribute the ternary operators in a well-readable way.
Enjoy your fun, dear cow-orkers!
I'll enjoy mine knowing how you will enjoy this snippet.

dkf

@BernieTheBernie I'd use the << operator instead.

Or just burn the whole lot to the ground.

BernieTheBernie

@dkf That can be combined. In one section of the ternary, use |, in the other use <<.

dkf

@BernieTheBernie Combine the bits with + instead (since as long as you don't have carry it's just like XOR) but now the less-than-wise will have to study things much harder to work out what you're doing.

// You could also do something like this
var m = bs.Select(b => 1 << b).Sum()
                       ^ inputValue;

As you can see, you've "helpfully" stated what the input value really is, right there in the code!

Gribnit

@dkf now that's idiomatic!

BernieTheBernie

Some more Kevin gems, that is ♭♯♮ (accidental wtf).
What's the difference in the meaning of string and integer? Seems to be context dependent...

[System.Xml.Serialization.XmlElementAttribute(Form = System.Xml.Schema.XmlSchemaForm.Unqualified, DataType = "integer")]
public string X
{
    get => m_X;
    set => m_X = value;
}

At least, the Unqualified keyword in the attribute is somehow adequate...

BernieTheBernie

Let's reduce an int to 2 different values instead of 2^32:

private int GetTextRotation() => ImageRotation == 180 ? ImageRotation : 0;

So, if ImageRotation is 180, the method will return 180. For all other 2^32-1 possible values of ImageRotation, it will retrun 0.
Why? But why? What's that meant to accomplish?
It's likely another ♭♯♮ .

BernieTheBernie

When looking up an error message found in the log files, Bernie stumbled upon an ♭♯♮ by Jim:

public virtual void StartActionQueue(ICannonAction _cannonAction, IAlarmInfo _alarmInfo, bool _interruptible, IActionQueueResultParameters _actionQueueResultParameters, IActionReportClient _actionReportClient)
{
    Logger.LogInfo(Name, "Running ActionQueue cannot start. Most likely the cannon controlling is in error state.");
    _actionQueueResultParameters.ActionQueueTerminatedWithErrorsEvent.Set();
}

is a method in an abstract class.
So, if a class inheriting from this abstract class does not override this method, that message will be logged. Unfortunately, Name is a private readonly property, initialized to the name of the abstract class. Hence, it is not possible to determine which derived class wrote the message (3 of 5 derived classes do not override), and in case there were several instances of that derived class, which one of the instances did (well, that was the concept behind that Name property...).

The method is called at two places in one class. But ... there it is called on a public writable property of the type of that abstract class (MyProperty.StartActionQueue(...), with SomeType MyProperty {get; set; }).. And now have additional fun with finding out where that setteris called from, and under what circumstances, etc,

Applied Mediocrity

@BernieTheBernie said in CodeSOD collection:

the cannon

Zecc

@BernieTheBernie said in CodeSOD collection:

it is called on a public writable property

BernieTheBernie

@Applied-Mediocrity said in CodeSOD collection:

@BernieTheBernie said in CodeSOD collection:

the cannon

Your fear is appropriate. Though it does not fire bullets thru the air, it is a dangerous item.
But would you prefer Canon(i.e. the printer manufacturer, and its products)? We have another thread for that...

BernieTheBernie

Fortunately, the complete Settingsfolder is under source control:

How else would we be able to keep such many more variants of the file under source control?

PleegWat

@Applied-Mediocrity said in CodeSOD collection:

@BernieTheBernie said in CodeSOD collection:

the cannon

That's what that code should be fired out of.

dkf

@BernieTheBernie said in CodeSOD collection:

Let's reduce an int to 2 different values instead of 2^32:

FWIW, text rotations probably ought to be a float. As soon as you're rotating words, the steps between single degrees are a bit too coarse.

HardwareGeek

@BernieTheBernie said in CodeSOD collection:

Most likely the cannon controlling is in error state.

I recommend aiming the cannon at Kevin. Presumably whether it fires or not depends on the nature of the error state, but that can add some entertaining uncertainty to what would otherwise be a routine and well-deserved execution.

HardwareGeek

@dkf said in CodeSOD collection:

@BernieTheBernie said in CodeSOD collection:

Let's reduce an int to 2 different values instead of 2^32:

FWIW, text rotations probably ought to be a float. As soon as you're rotating words, the steps between single degrees are a bit too coarse.

Specify the rotation in radians. For any desired rotation, there should be some integer value n such that n - mπ is a sufficiently close approximation to the desired result.

BernieTheBernie

@HardwareGeek said in CodeSOD collection:

I recommend aiming the cannon at Kevin.

But this code is the achievement of Jim.
Anyway, I like the idea.

BernieTheBernie

Well, Jim, I enjoy your mode of thinking:

string text1;
if (IsJustForNotification)
{
    text1 = string.Format(LocalizedTexts.ExtinguishRequestText1, AlarmInfo.AlarmLevel, ExtinguishCannonName, AlarmInfo.Source);
    if (string.IsNullOrWhiteSpace(PushButtonConfirmationText))
    {
        text1 += " " + LocalizedTexts.DefaultPushButtonConfirmationText;
    }
    else
    {
        text1 += " " + PushButtonConfirmationText;
    }
}
else
{
    text1 = string.Format(LocalizedTexts.ExtinguishRequestText1, AlarmInfo.AlarmLevel, ExtinguishCannonName, AlarmInfo.Source);
}

Just for notification, dear Jim!, you could write that code a little shorter and clearer...

BernieTheBernie

C# offers string interpolation. Kevin is a real fan of it:

AlarmInfo = new DemoAlarm("TestAlarmLevel", $"PsAlias"),

But why, Kevin, why do you apply it for the second parameter only? You could also use it for the first parameter!
There's room for improvement.
Right where you left it.

Gribnit

@BernieTheBernie said in CodeSOD collection:

C# offers string interpolation. Kevin is a real fan of it

Because C# fucked up and does not warn on degenerate interpolations.

Tsaukpaetra

@Bulb said in CodeSOD collection:

Yes, provided pagination isn't getting in the way. I've seen a couple systems that load the first page of data only, then re-sort or filter client side. Cue puzzled user looks.

ServiceHow: "20 records were removed from list due to security"

topspin

@BernieTheBernie wondering how you extinguish a cannon.

Applied Mediocrity

@topspin said in CodeSOD collection:

@BernieTheBernie wondering how you extinguish a cannon.

With a bigger cannon

HardwareGeek

@topspin said in CodeSOD collection:

@BernieTheBernie wondering how you extinguish a cannon.

After firing, a wet swab was run down the barrel to ensure any residue was extinguished before the powder for the next shot was loaded. Because bad things might happen if you didn't.

BernieTheBernie

#if DEBUG - an easy to misuse pragma.
Just another small Kevin gem:

#if DEBUG
    Logger.LogInfo(nameof(UpdateAlarmStates), $"Adding Alarm for {X}!");
#endif

What about:

    Logger.LogDebug(nameof(UpdateAlarmStates), $"Adding Alarm for {X}!");

Bulb

@BernieTheBernie I don't remember whether C# can have this method be magic, so … the second will still build the string even if debug logging is off, won't it?

Of course in C++, where the statement usually is a macro and therefore behaves like if it was wrapped in the #if DEBUG, there is sometimes the opposite problem: things work in debug and stop working in release, because they depended on a side-effect of the log statement that was not compiled in.

Zecc

I don't expect the string formatting and method call to be omitted, no.

I vaguely remembered this ⇓ being a thing, so I went to check: https://learn.microsoft.com/en-us/dotnet/api/system.diagnostics.conditionalattribute

Indicates to compilers that a method call or attribute should be ignored unless a specified conditional compilation symbol is defined.

But turns out (emphasis mine):

You can apply the ConditionalAttribute attribute to methods and classes. However, its use on classes is valid only for types that are derived from Attribute. ConditionalAttribute either will be ignored or will produce a compiler warning or error message if you apply it to any other type.

I suspect this requirement is for making the compiler implementation simpler and more efficient.

Edit: I misread that part. The requirement for classes to derive from Attribute is only when you apply ConditionalAttribute to the class itself. You can apply the attribute to any method in any class as long as it returns void.
So yes, it is possible that Logger.LogDebug() has a [Conditional("DEBUG")] applied to it. But.. I doubt it.

Edit 2: I have confirmed there will be no side-effects if the call is compiled out, as is the sensible thing.

BernieTheBernie

@Bulb no. Kevin seemed to have wanted to get the log output only for debug purposes (but how could I be sure? Does he actually know what he wants?). That's what the LogDebug method is for.
(Of course, you have to switch the log level to debug when you want to see it - so not perfectly equivalent)

Bulb

@BernieTheBernie said in CodeSOD collection:

(Of course, you have to switch the log level to debug when you want to see it - so not perfectly equivalent)

Well, I'd expect the log level to default to debug when testing from the IDE and overridden to something less verbose in the staging and production environments only.

izzion

@Bulb said in CodeSOD collection:

@BernieTheBernie said in CodeSOD collection:

(Of course, you have to switch the log level to debug when you want to see it - so not perfectly equivalent)

Well, I'd expect the log level to default to debug when testing from the IDE and overridden to something less verbose in the staging and production environments only.

Woah there, this is way too professional of a solution. I'm going to have to ding your WTDWTF license for 2 points.

BernieTheBernie

A different chapter of , and this time
We build some devices, and they contain some parts, let's call them for brevity Part1 and Part2.
During many years, things have been developed further, and there are a few generations of our devices, and of course, of those parts. And there comes an issue: it is not possible to combine Part1 and Part2 wildly, they have to fit to each other.
Say, Part1 version 1.23 must not be combined with Part2 version 1.18. Actually, if you do not take too close a look, things seem to work also in that combination. The software checks for the validity of the combination only in 1 place: when it has to decide if it sends a specific command to Part1 or to Part2 (there are only some 3 commands which changed their place during evolution).

Now, Fritz called me: some machine out at a customer site just showed him the finger during startup after an update. Because of incompartible versions of Part1 and Part2.

What has changed?

Oh, the version combination check has not changed at all for those old numbers since times immemorial.

But...
Jim added a line of code to a different component such that it calls such a moved command during its start. And there it crashes.

Actually, the device should have reported a failure for some other task it is supposed to perform once a day, which also uses such a command. But perhaps that task was not configured at all...

Wunderful!
It means: there is a device with an "illegal" combination of Part1 and Part2 at a customer site, we never knew that, and now it goes .

Bulb

@BernieTheBernie So I suppose you were tasked to make it somewhat kinda sorta work again because nobody wants to replace that device and development time magically costs nothing¹.

¹ I don't know how expensive those devices are, so depending on other factors it might be worth the time to get it to work good enough again, but I've often seen companies being very reluctant to buy something for a few bucks, requiring extra work to get the cheaper option to work that costed many times more in the end but development work does not need specific approval, so nobody sees it. For that reason I've been insisting on quoting prices in man-hours at the usual consultation rate lately.

Gribnit

@izzion said in CodeSOD collection:

@Bulb said in CodeSOD collection:

@BernieTheBernie said in CodeSOD collection:

(Of course, you have to switch the log level to debug when you want to see it - so not perfectly equivalent)

Well, I'd expect the log level to default to debug when testing from the IDE and overridden to something less verbose in the staging and production environments only.

Woah there, this is way too professional of a solution. I'm going to have to ding your WTDWTF license for 2 points.

But shouldn’t we charitably assume

public static synchronized void setLogLevel(final String logLevel) {
    LOG_DB_REF.set(logDbForLevel(logLevel));
}

? Wouldn't we want them to assume the same of us?

Bulb

I am currently trying to rework deployment of some micro-service (itself collection of nano-services)¹ that assembles some config data. The entries it writes to the database all look like:

{
    "type": "json",
    "data": {
        …
    }
}

As if it wasn't a bit too late to learn it's json at that point.

Zecc

@Bulb It's saying the data property contains JSON. So you can be reassured you don't have XML-in-JSON.

Not in the first level of nesting anyway.

Arantor

@Zecc but what if it were JSON that contained JSONx as a string insi—

I’ve gone cross eyed.

Gribnit

@Arantor said in CodeSOD collection:

@Zecc but what if it were JSON that contained JSONx as a string insi—

I’ve gone cross eꙮd.

Well that's what the type field is for.

BernieTheBernie

@Bulb said in CodeSOD collection:

@BernieTheBernie So I suppose you were tasked to make it somewhat kinda sorta work again because nobody wants to replace that device and development time magically costs nothing¹.

¹ I don't know how expensive those devices are, so depending on other factors it might be worth the time to get it to work good enough again, but I've often seen companies being very reluctant to buy something for a few bucks, requiring extra work to get the cheaper option to work that costed many times more in the end but development work does not need specific approval, so nobody sees it. For that reason I've been insisting on quoting prices in man-hours at the usual consultation rate lately.

Fortunately, Fritz understands that there's an issue. It just has not shown up during the time before - and it would be a rather critical situation when that code gets invoked which will show the issue. Or: that situation may have happened, but nobody understood, and the service guy - after failing to rescue the device by re-starting the software only - did it the hard way: switch the device electrically off, wait a little, switch it on again. That would likely help...

No, to me the real question is: how many devices with such an invalid hardware combination are out there in the wild?

BernieTheBernie

Dear Jim, or my dear Jim!
How do you extract a parameter from a URL?
Jim's original code looked like:

    var idx = window.location.href.indexOf("?Id=");
    var id = window.location.href.substring(idx + 4, idx + 5);

And I guess, our web guys here can spot the issue immediately.
Excatly: when idgets longer than 1 single character, the thingy is fucked up.

So Jim had to replace that code.

And, believe it or not, Bernie counted 21 occurences of this snippet being replaced...
:
: :thats_fine:

Zecc

@BernieTheBernie Makes you wish it would be possible to do something like

new URLSearchParams(window.location.search).get('id')

doesn't it?

BernieTheBernie

@Zecc That was the solution. At 21 occurences...

dcon

@BernieTheBernie said in CodeSOD collection:

@Zecc That was the solution. At 21 occurences...

Too bad there 24 original instances, amiright???

Zecc

@dcon The number of instances is irrelevant, because they all go through the same helper method, right?

Gribnit

@Zecc said in CodeSOD collection:

@dcon The number of instances is irrelevant, because they all go through the same helper method, right?

Oh, yeah, every file that does this declares that method.

Bulb

@BernieTheBernie said in CodeSOD collection:

    var idx = window.location.href.indexOf("?Id=");
    var id = window.location.href.substring(idx + 4, idx + 5);
And I guess, our web guys here can spot the issue immediately.
Excatly: when idgets longer than 1 single character, the thingy is fucked up.

The first thing I noticed was he is assuming the parameter will be the first one.

BernieTheBernie

@Bulb Only one. Imagine there were 2 parameters...
Or ... oh I do not want to imagine ... POST

Gribnit

@BernieTheBernie said in CodeSOD collection:

@Bulb Only one. Imagine there were 2 parameters...
Or ... oh I do not want to imagine ... POST

But if the ID isn't on the URL, how will the intermediate servers cache it?

izzion

@BernieTheBernie said in CodeSOD collection:

@Zecc That was the solution. At 21 occurences...

So Jim's "Practical Functions 101" course is scheduled for next semester?