Expression is already of boolean type

[tshemsedinov]

https://github.com/diia-open-source/be-pkg-utils/blob/e969374fb769e59ab6bbdb709caeba0199e14a8c/src/applicationUtils.ts#L178-L184

static isItnFormatValid(itn: string): boolean {
  return !/^0{1,20}$/.test(itn) && itn !== 'null' && /^\d{10}$/.test(itn);
}

Also it will be better to remove null comparison and use string operations like .startsWith, .contains and so on in such simple cases at least we do not need regexp.

[vird]

Problem with this code is not only with "already boolean", it has redundant logic

According to tests this code can be rewritten in much simplier way https://github.com/diia-open-source/be-pkg-utils/blob/e969374fb769e59ab6bbdb709caeba0199e14a8c/tests/unit/applicationUtils.spec.ts#L813-L819

• should return false for an invalid ITN format if (!/^\d{10}$/.test(itn)) return false
• should return false for an ITN consisting of all zeroes Then if (!/^0*$/.test(itn)) return false This one also covers should return false for the string "null"

So

static isItnFormatValid(itn: string): boolean {
  // valid format should contain 10 digits, no letters allowed
  if (!/^\d{10}$/.test(itn)) return false
  // edge case. We don't allow all digits to be 0
  if (/^0*$/.test(itn)) return false
  return true
}

So check for null can be removed

[Deimos308]

@vird last condition can also be simplified to:

static isItnFormatValid(itn: string): boolean {
  // valid format should contain 10 digits, no letters allowed
  if (!/^\d{10}$/.test(itn)) return false
  // edge case. We don't allow all digits to be 0
  return +itn !== 0
}

No additional RegExp is required.

[vird]

Last condition should be not simplified Reason. If I want to remove part of logic I need remove 1 line In your case I can't remove last line

[Deimos308]

It sounds strange to me in several ways: 1) Using an extra if condition + a regular expression where it can be very easy to skip it. 2) "If I want..." - sounds like premature optimization in order to have a "universal solution", but what's the point? The function is as simple as possible... 3) "In your case I can't remove last line", do we need it?:

   ...
   // edge case. We don't allow all digits to be 0
   return +itn !== 0
}

  ...
  return true
}

Moreover, in this case I would personally prefer the format with a single line format provided by @tshemsedinov :

   ...
   // valid format should contain 10 digits, no letters allowed
   // edge case. We don't allow all digits to be 0
   return !/^\d{10}$/.test(itn) && +itn !== 0
}

Perhaps I don't fully understand your position... Can you explain with an example please?

[vird]

• v8 will optimize last condition
• my solution is more readable
• my solution is more editable