Hacker News new | ask | show | jobs
by addictedcs 2451 days ago
First example I've checked is wrong [1]. Shuffle a list, with a static Random object initialization. Using a static random object is not thread-safe in .NET.

"One common solution is to use a static field to store a single instance of Random and reuse it. That’s okay in Java (where Random is thread-safe) but it’s not so good in .NET – if you use the same instance repeatedly from .NET, you can corrupt the internal data structures." [2]

Very often the very same resources which claim to be the source of truth provide bad code examples.

[1] https://www.programming-idioms.org/cheatsheet/Csharp

[2] https://codeblog.jonskeet.uk/2009/11/04/revisiting-randomnes...

4 comments

Quite frankly, this site's examples are pretty horrible.

Case in point, Idiom #46 in C[0] uses strncpy and predictably fails to correctly terminate the buffer.

Idiom #55[1] converts a integer to a string with itoa, never bothering to mention that it isn't part of the C standard or POSIX, while for some reason using a 4096 byte big output buffer.

Idiom #39[2] first example uses clrscr(), which I assume is some sort of old DOS function? The entire example looks unrelated to the problem.

The idea is certainly good, but you need some serious vetting system to make this usable.

[0] https://www.programming-idioms.org/idiom/46/impl/477

[1] https://www.programming-idioms.org/idiom/55/impl/438

[2] https://www.programming-idioms.org/idiom/39/impl/2042

%d for strlen is cute as well.

That’s the fate of all such sites. They start with a nice idea and a good intent, but the crowd fills it with crap, nobody cares/is able to screen it, and it instantly becomes an unreliable source of nonsense.

Ed: btw, #1 is terminated by calloc(6), but it is indeed a trip mine looking for prey. It is unknown if strncpy was used intentionally or out of ignorance and if the next person will be aware.

> #1 is terminated by calloc(6)

Yet this seems to be the perfect place and time to remind the kind audience that sizeof measures the number of char-sized chunks, thus sizeof(char) is always the constant 1, no need to spell it out.

> Case in point, Idiom #46 in C[0] uses strncpy and predictably fails to correctly terminate the buffer.

calloc returns zeroed memory.

Plus #46 casts calloc, and uses the unnecessary sizeof (char).
There are also issues with the JS examples.

For example:

"""

Remove all occurrences of string w from string s1, and store the result in s2.

var s2 = s1.replace(w, '');

"""

Since w is a string, this will only replace the first occurrence, not all occurrences.

I may have been using JavaScript's replace method wrong my entire life...
I changed it to:

    var regex = RegExp(w, 'g');
    var s2 = s1.replace(regex, '');
Now it doesn’t work if the string you want to replace has any regex special characters. One JS idiom for replace-all is

  str.split(before).join(after)
That's also problematic. If w includes characters that have special meaning in a regex (e.g. a period), this will cause issues.

You could try to escape those special characters but that is potentially error prone.

For an arbitrary w, one approach is something kind of awful like:

  let s2 = s1;
  while (s2.includes(w)) {
    s2 = s2.replace(w, '')
  }
Edit: minitech's solution is better.
You are aware of how a community driven site works? If its wrong, then go ahead and put your "right" implementation there. Beyond that, you're inferring threading into the problem. There is no mention of thread-safety in that example, nor does it claim to be. If you want to over-analyze the problem statement, that's your prerogative- that doesn't make that answer wrong however.
> You are aware of how a community driven site works? If its wrong, then go ahead and put your "right" implementation there.

It doesn't provide any indication of whether an entry has been vetted. At least with stackoverflow you can see discussion in the comments and guess whether an answer has been critiqued.

If you have sufficient expertise in an area and poke around SO long enough, you'll spot the problem with an egalitarian system soon enough: the blind leading the blind.

Unfortunately, the site doesn't inspire participation. There's no way for good answers to rise up above bad answers. And there are a lot of bad answers.
> Beyond that, you're inferring threading into the problem.

That's what I was thinking. There are no threads in the example.

But it's not at all unreasonable to think some newbie may come along, copy & paste that solution, and then call Shuffle from multiple threads.

Thread safety is something that needs to be touched on constantly, and it's smart to write your code such that it would be thread safe. Because otherwise, it just looks like a normal function call. And it could take someone ages to figure out why everything is breaking if they were to actually use that example.

The newbie just needs to try to use that code in any webapp. It will break. The answer is bad and needs to be downvoted.
The example did not have threaded code so I find your nitpick unfair. The usage of random in their examples is perfectly fine.
If the site is providing toy examples that are not useful in production, then it should state that.

If "idioms" mean code that adheres to best practices, given that C# code is frequently threaded, their examples should mitigate common threading issues.

>then it should state that.

It will be ignored. StackOverflow guys and all participants have put an enormous effort into making it at least partially trusted source of snippets, and I hope it will never be outranked by sites like this.

(to creator: sorry for this, but in a long run it only harms everyone, unless you “die” for it)

If the language runtime supports threading, in the absence of explicit documentation, code that assumes single thread access is broken.