8000 [Validator] added BIC (SWIFT-BIC) validation constraint by mvhirsch · Pull Request #15519 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

[Validator] added BIC (SWIFT-BIC) validation constraint #15519

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 9 commits into from
Closed

[Validator] added BIC (SWIFT-BIC) validation constraint #15519

Changes from 1 commit
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
2fd8573
[Validator] added BIC (SWIFT-BIC) validation constraint
mvhirsch Aug 12, 2015
99bc9f1
[Validator] fixed coding standards. refs #15519
mvhirsch Aug 12, 2015
a2b612e
[Validator] using non plural constraint message. refs #15519
mvhirsch Aug 12, 2015
ec839e7
[Validator] Using UUIDs instead of scalar values. refs #15519
mvhirsch Aug 12, 2015
398988d
[Validator] removed unnecessary comment. refs #15519
mvhirsch Aug 12, 2015
be41875
[Validator] fixed comment for clarity. refs #15519
mvhirsch Aug 12, 2015
f278cbb
[Validator] use just one strlen call. refs #15519
mvhirsch Aug 13, 2015
71e11d7
[Validator] added english and german translation messages. refs #15519
mvhirsch Sep 24, 2015
fc432e4
[Validator] fixed german translation. refs #15519
mvhirsch Sep 25, 2015
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
[Validator] fixed comment for clarity. refs #15519
  • Loading branch information
@mvhirsch
mvhirsch committed Sep 25, 2015
commit be41875b1137ad50ecd5e05a6827c28a5946382c
2 changes: 1 addition & 1 deletion src/Symfony/Component/Validator/Constraints/BicValidator.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ public function validate($value, Constraint $constraint)

$canonicalize = str_replace(' ', '', $value);

// the bic must have either 8 or 11 characters
// the bic must be either 8 or 11 characters long
if (8 !== strlen($canonicalize) && 11 !== strlen($canonicalize)) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this condition correct? Excuse me! The code is correct. I didn't understand the above comment correctly.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i would say yes, if it is not 8 AND not 11 characters long, then buildViolation

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about

if (in_array(strlen($canonicalize), array(8, 11))) {

?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Triiistan I suspect looping over an array is not faster than performing a &&

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@stof i think @Triiistan is talking about better readability

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was trying to save 1 call from strlen and I tried to find which style Symfony used most between

if (yyy !== xxxx && zzz !== xxxx) {

and

if (!in_array(xxxx, array(yyy, zzz)) {

and the latter was used a bit more.

With the first style, we can write

$canonicalizeLength = strlen($canonicalize);
if (8 !== $canonicalizeLength && 11 !== $canonicalizeLength) {

to avoid the duplication.

[Updated: I inverted both styles]

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

However, my concern may not be very pragmatic, so go with the version most readable to your eyes 😃

$this->context->buildViolation($constraint->message)
->setParameter('{{ value }}', $this->formatValue($value))
Expand Down
0