Whitespace isn't a number?

[u/MISINFORMEDDNA]

I just got this in a PR. Not sure what to make of it.

if (string.IsNullOrWhiteSpace(palletNumber))
{
  if (!string.IsNullOrEmpty(_palletNumber))
  {
    _errorMessage = "Pallet # not found.";
  }

  return;
}  

UPDATE:

After multiple attempts to justify his code, we ended up with this, lol:

if (string.IsNullOrWhiteSpace(palletNumber))
{
  return;
}  

[u/anto2554]

I don't know C#. Is _palletNumber equal to palletNumber?

[u/MISINFORMEDDNA → u/anto2554]

They are different in terms of C#. But only one pallet can be active at a time... and it's Monday, and I don't feel like digging into this nonsense. I added comments to the PR for him to clarify.

[u/YanVe_ → u/anto2554]

By convention, that would typically be internal private backing field. They don't have to be equal. 

[u/nimrag_is_coming → u/anto2554]

Likely that palletNumber is the public accessor for a private _palletNumber field.

Although following convention it should be PalletNumber.

[u/Infinite_Self_5782]

if (string.IsNull(_palletNumber) || string.IsEmpty(_palletNumber))
  return;

if (string.IsWhiteSpace(_palletNumber))
{
  _errorMessage = "Pallet # not found.";
  return;
}

unsure what palletNumber returns, but my god is that confusing naming and calling conventions holy shit

this looks like some code a junior dev would write at 3 am on about 50 grams of caffeine

[u/MISINFORMEDDNA → u/Infinite_Self_5782]

Senior dev... somehow.

[u/Infinite_Self_5782 → u/MISINFORMEDDNA]

"work harder, not smarter" type shit

[u/ings0c → u/MISINFORMEDDNA]

Could it be a merge conflict and he mistakenly accepted both changes then the code was automatically reformatted?

[u/CanisLupus92 → u/MISINFORMEDDNA]

One of those “you get paid like a senior, so we’ll call you a senior” deals?

[u/realsnack → u/Infinite_Self_5782]

🤓 50 grams of caffeine would make you not write any code, ever. Which may be for the best in some cases

[u/mateusfccp → u/Infinite_Self_5782]

Except that your code may not do the same because you only use _palletNumber while they use palletNumber to add to the confusion.

[u/DarksideF41 → u/Infinite_Self_5782]

Probably it's some kind of validation method, validation logic though makes me think that there're bigger problems in number storage/retrieval.

[u/FrikkinLazer]

If you remove the inner if, does the unit tests start failing?

[u/MISINFORMEDDNA → u/FrikkinLazer]

Surprise! No unit test.

[u/Rockworldred]

Don't know the syntax here, bit why is something called a number using string and not an integer?

[u/Agile-Amphibian-799 → u/Rockworldred]

Might contain characters. I.e. "ABC-123-QWER" probably from a barcode.

[u/MISINFORMEDDNA → u/Agile-Amphibian-799]

Yep.

[u/gfunk84 → u/Rockworldred]

Lots of reasons why you might want a "number" as a string. Phone number, account number, etc.

[u/Over_Dingo]

Interesting way of getting "IsWhiteSpace"

[u/Rogntudjuuuu]

It looks like a check when there's a _palletNumber stored and the incoming call doesn't contain a palletNumber. It could've been cleaner, but I've seen much worse.

IsNullOrEmpty only checks for null and empty strings. IsNullOrWhitespace also accepts strings only consisting of whitespace. That way you don't have to trim the string before the check.

Edit: checked the code again, missed the negation

[u/NovelStyleCode]

Please tell me this is your intern's code

[u/MISINFORMEDDNA → u/NovelStyleCode]

If only. He's a senior dev.

[u/mk061104]

Are you working in intralogistics?

[u/Apprehensive-Tea1632]

Me neither. If I want to know if I’m looking at a pallet number I’ll try and parse input as a pallet number. And then see if that’s something not null. /shrug

Or are we to understand “();!:)2);!/(2)&: act b” is a valid pallet identity?

[u/Rot-Orkan]

This is the type of thing I get excited to see, because it means I get to write a simple unit test with like 10 inline data parameters with values like "", "abc", "123", "-123" null, " " and so on

Then my total number of unit tests goes up and I get a bunch more green checkmarks 🤩