• (nodebb)

    return secnod; //yes i know, don't frist me

  • Victor (unregistered)

    Aren't the offset and numCharToRead arguments mixed up?

  • (nodebb)

    First one is 100% offshore "debeloper" code. Second one is fairly amusing for the name and comment alone. The code equivalent of "Don't @ me" kek

  • I dunno LOL ¯\(°_o)/¯ (unregistered)

    Why is s2, an obvious temporary, declared as a class variable? My concurrency spidey-sense is tingling; if this gets used by two threads at the same time, Bad Stuff will happen. Not just any Bad Stuff, but the worst kind, a Heisenbug.

  • Alistair (unregistered) in reply to I dunno LOL ¯\(°_o)/¯

    It won't cause concurrency bugs because it is never modified.

  • van Dartel (unregistered)

    It looks like the result of an inside job by somebody disguising themselves as an imbecile while introducing a kind of buffer overflow (in the sense of being able to access previously stored data). The level of self-deprecation in both the code as well as comments makes them rather smart.

  • Anonymous') OR 1=1; DROP TABLE wtf; -- (unregistered)

    I misread numcharstoread as NumCharStoreAd for whatever reason. That might even be a better name since it's not as completely misleading.

  • Brian (unregistered)

    It's not exactly a substring function. If I'm reading this correctly, it's extracting each individual character and returning it as a separate string in an array of strings. I can't even fathom why someone might want to do this. But, as you say, if this method were ever called it would throw a null reference exception, so maybe it's just a floating bit of dead code left in there for your (and our) entertainment.

  • (nodebb) in reply to Victor

    The parameters aren't exactly mixed up. Yes, numcharstoread really is an offset, but the reverse isn't true. The offset isn't the length to read, but rather the absolute stopping point (one past the end).

    Assuming aInstance is an instance of a that has s2 correctly initialized, string[] result = aInstance.mid(11, 7, "Hello, world! Goodbye!"); would return 4 characters (worl), not 11.

  • Jay (unregistered)

    So if offset were less than numcharstored, it would crash with an "index out of range" error. You could fix it by adding a comparison with the length of the string or try-catch the error.

  • WTFGuy (unregistered)

    No. If offset were less than numcharestoread the loop wouldn't execute even once and the function would return the untouched s2. Likewise if offset was equal to numcharestoread.

    IndexOutOfRangeException will be thrown if offset (or numcharstoread) is greater than s.Length - 1

    On top of everything else, this will also fail if s contains any of the Unicode characters that need more than one char to represent them.

    Offshore "work" at it's finest. (misplaced apostrophe intentional).

  • NoLand (unregistered)

    Everyday, I wake up in the afternoon and go along until it's time to rise. By this time, I'm usually done with my work.

    It's night owl code!

  • (nodebb)

    The second one could be worse:

    // I don't know why, what are you going to do about it?
  • Bruce W (unregistered)

    And some people say the quality of WTFs here are going down. This is wow. The WTFs have layers.

  • Tom (unregistered)

    OMG this code is a WTF on so many levels.

    • the unitialized s2 field
    • the fact that the implementation is stateful, which makes it intrinsically not thread-safe
    • the offset and numcharstoread parameters whose purpose seem to have been inverted
    • the fact that it returns the result as an array of strings
    • the comments in broken English that further confuse things

    I'm amazed to see so many WTFs in such a small piece of code. I have a coworker who is a terrible coder, and I don't think even he could have made such a horrible mess.

  • Barf4Eva (unregistered) in reply to Tom

    I can understand the argument against stateful implementation for something like someone's wonky reinvention of not-quite substring. Or in microservices. Or web apps. But I really don't see the merit in arguing against stateful implementation in a general sense in any ol class.. I saw that a couple of times in this thread. Is that the intention of the argument in those threads, or have I misunderstood? I figured perhaps I misunderstood how generalized the dread was. There are advantages and disadvantages, as always.

  • Best Of 2021 (unregistered)

    This is top drawer. A real WTF, in that you would actually say those three words when you find it, and it has you scratching your head for a while to even understand what the author was trying to achieve.

    If it actually worked (i.e. it initialised s2 to a string of the right length, and you were allowed to assign by index in C# strings) it does

    mid("Test string", 9, 6) -> [ "r", "i", "n" ]

    What is that ever useful for?

    And then of course there's the 'not working' - NullReferenceException, assignment to immutable string index. And then the bonus WTFs: no bounds checking, Engrish comment, unnecessary instance state (which makes it non-thread-safe for no good reason).

  • Prime Mover (unregistered)

    Can't actually see much wrong with this.

    Ahaa, I think I've got it. The comment lines are introduced with 3 forward-slash characters when in reality only 2 are needed? Yes indeed, what an appalling waste of computer resources.

  • NoLand (unregistered)

    Now, to be fair, what seems to have happened here: this was originally a regular implementation of substr() (meaning, return n chars starting at given offset), and later the specs changed (e.g., some other party implemented code, which provided parameters in this weird format) and the code was quickly fixed to adjust to the new requirements, but comments and parameter names stayed the same. (Probably, "Make this work and don't invest too much, just make it work. — And no grumpy comments!")

  • (nodebb)

    I love docstrings, that just repeat the signature(or method name).

    I don't need to know that String[] is an array, I ALREADY know that. I want to know what it is supposed to CONTAIN, and I won't take "Strings" for an answer.

    Writing a proper docstring often uncovers issues with the design before writing any code. Or just outright demonstrates that the idea is nonsense. Here it more looks like "documentation" was a style guideline they didn't agree with.

  • (nodebb)

    By the way, is there some possibility to subscribe to further comments? So far I just use Pushbullet to put the article from iPad to PC and later take another look, but that's quite the crutch. Plus, Pushbullet isn't ven officially on the AppStore anymore, AND it lacks a plausible monetization scheme. The premium plan is too expensive for what it brings to the table, and they generate zero revenue from free users instead of using lightweight ads. Heck, even showing an ad before showing pushed files would be acceptable.

  • (nodebb) in reply to R3D3

    Offtopic rambling, sorry.

  • Airdrik (unregistered) in reply to Barf4Eva

    The problem with stateful classes is that they are prone to thread safety problems - primarily race conditions. Thus in order to use them in any multi-threaded application (which nowadays is practically any application) you need to use them with caution: either never share instances between threads or ensure that any invocations are carefully guarded and synchronized in order to ensure that it always works the way it was "designed" or "intended" regardless of the order that the various threads end up walking through the code (which can also lead to bottlenecks which can hurt the performance of the application).

    Because of the problems you run into in multi-threaded applications, stateful implementations have become a bad practice and are generally discouraged. With few exceptions, the benefits are marginal while the drawbacks are significant.

  • Duke of New York (unregistered)

    I propose a consciousness experiment called the Stack Overflow Room. It's like John Searle's Chinese Room, except that the subject is always a human, and the problem is whether he can be said to "learn" to code by pasting from Stack Overflow without understanding any of the English words involved.

  • Duke of New York (unregistered)

    There's a reason that the Java SimpleDateFormat documentation has had a callout about thread-safety since 1.4. Every so often, someone who needs to process date strings inside a method gets the bright idea to save some resources by reusing a single format instance, because surely those smart guys at Sun made it thread-safe internally...

  • ooOOooGa (unregistered)

    brb. Adding a function to my php god utility file that aliases 'explode' as 'beSplited'.

  • (nodebb)

    HP's version of dtksh let you write GUI procedures with the shell. I wrote a simple taskbar that kept abending. But it didn't fail when I put in debug echos. The following line fixed the bug (dtksh was built on an old buggy version of ksh):

      : # don't remove this line that theoretically shouldn't do anything.
  • WTFGuy (unregistered)

    @Best of 2021 ref:

    ... and you were allowed to assign by index in C# strings) it does

    and

    ... assignment to immutable string index.

    Not sure what you mean. The code is

    char ch = s[i];
this.s2[j] = Convert.ToString(ch);

    Indexing into a System.String and retrieving the Char from that position is normal C#. See https://docs.microsoft.com/en-us/dotnet/api/system.string.chars?view=net-5.0 . So the first line is just fine.

    SInce s2 is an array of strings there's also nothing wrong with the second line. If the array had been initialized to an array of the correct length rather than left as scalar null.

    What are you seeing?

  • Chris (unregistered)

    All these worries about thread safety is assuming the calling code is keeping one instance of class "a". I fully expect a new instance to be initialised every time someone wants to call "mid", even from the same block of code, given the quality of this code.

  • CoyneTheDup (unregistered)

    @van Dartel I vote with you. That first one is a concealed memory sniffer.

  • CoyneTheDup (unregistered)

    @van Dartel I vote with you. That first one is a concealed memory sniffer.

  • löchleindeluxe (unregistered) in reply to Anonymous') OR 1=1; DROP TABLE wtf; --

    O, penis sues!

  • WTFGuy (unregistered)

    @van Dartel ref:

    It looks like the result of an inside job by somebody disguising themselves as an imbecile while introducing a kind of buffer overflow (in the sense of being able to access previously stored data). The level of self-deprecation in both the code as well as comments makes them rather smart.

    and @CoyneTheDup ref:

    @van Dartel I vote with you. That first one is a concealed memory sniffer.

    Not in C# it's not. It's a concealed IndexOutOfRangeException generator.

    I'm not 100% certain we can conclude this is C# versus some other C-like language. But it sure looks like it to me.

  • Best Of 2021 (unregistered) in reply to WTFGuy

    SInce s2 is an array of strings there's also nothing wrong with the second line.

    Yeah you're right I was blanking out the part where the result is an array of strings, because WTF why would you do that.

  • (nodebb)

    I will admit it... this past week in the midst of frustrating debugging (25 year old codebase) I did use variable "Voo" and "Doo".... might not be so bad, bu tI did commit them to SCCM.

  • Barf4Eva (unregistered) in reply to Airdrik

    I feel the concern is geared more towards objects that are not immutable rather than objects that are not stateless, in the general sense regarding threading. I'd find myself hard-pressed to have entirely stateless class design throughout an entire application, I guess? But objects whose state doesn't change, that I can sign up for more often, but still not entirely. Maybe I'm a lost cause :D I appreciate your feedback, though.

Leave a comment on “Two Knowing Comments”

Log In or post as a guest

Replying to comment #:

Log Out

« Return to Article