17
Review
Don't use the main entry point to implement an algorithm. Create a method instead.
Think about how to allow this method to be usable for all kinds of types, not just integers. You have implemented the algorithm for integers, yet you show an example with strings.
When creating an algorithm, surely you have made some unit tests, at least for the happy ...
5
Small Bias
First a nit pick, Math.random() generates a number from 0 to < 1. It will never generate 1. Thus to get a statistical odds of 1/2 you must test either Math.random() < 0.5 or Math.random() >= 0.5. Testing Math.random() > 0.5 ? char : char.toUpperCase() will give a very (VERY) small bias in favor of upper case characters.
Dramatic Bias
...
5
All of dfhwze's points are spot-on, and the only thing I can think of to add to his type-signature is that if you're unfamiliar with all of the syntax he's using that's ok.
(If you want to learn the stuff in question, search for "inheritance" (or "interface"), "generics", "extension methods", and possibly "generators" or "yield".)
Regarding your actual ...
5
While your new solution does look somewhat better than the original, I would suggest that constantly removing elements from the list can be quite expensive. I would suggest shuffling or randomly sorting the list then simply divvying up the players to the teams.
Also you're still putting the implementation in Main, instead of calling a method.
Instead of ...
4
public static class Hashing
{
public static int SaltSize = 32;
This is public, static, and not const or readonly. That means:
"Anyone" can change it.
It's not thread-safe.
Hash validation is undermined.
I see no reason for it to be public, and every reason for it to be readonly or const. Personally I'd favour readonly and initialised from ...
2
Review
You should obfuscate the equality check return combinedHash.Equals(serverHash); using a SlowEquals implementation.
Khalid Abuhakmeh's post explains the vulnerability.
/// <summary>
/// Compares two byte arrays in length-constant time. This comparison
/// method is used so that password hashes cannot be extracted from
/// on-...
2
There's at least one problem with this approach.
What happens if Math.random() generates a number that can be represented with just a few digits, like 0.25? You've got only one random character in that case, not five. Instead of using the fractional part of a number, maybe it would make more sense to generate integers in the range 36^4 to 36^5-1, to ensure ...
1
Consider warmwaffle's code at GitHub (re: https://github.com/warmwaffles/Noise). The one difference I see between the two solutions is how the 'metaVector2f's are created. Your code is using non-repeatable random numbers; whereas, the other uses a mathematical calculation which includes prime numbers and bit shifting operations. The latter would produce a ...
1
Review
There is no added value in creating a new Random for every cycle in the loop. Create and reuse a single instance.
You have a convoluted way of generating the next random value. Why do r.nextInt((9 - 1) + 1) + 1 when r.nextInt(9) + 1 would yield the same? If it's a readability thing, the latter is better.
I agree with other answer that if you want ...
1
Enumerate once
I'm not sure whether you intended to write this to be iterable only once. Is this as designed?
[TestMethod]
public void CodeReview()
{
var bag = new ShuffleBag<string>(new[] { "a", "b", "c", "d" });
var e1 = bag.ToList(); // randomized: "b", "a", "c", "d"
var e2 = bag.ToList(); // last element: "d"
var e3 = bag....
Only top voted, non community-wiki answers of a minimum length are eligible
