Skip to content
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.

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

Fix order of elements in NameParser::parseVcfString #222

Open
wants to merge 1 commit into
base: 2.0.0-dev
Choose a base branch
from

Conversation

tups
Copy link

@tups tups commented Jan 9, 2024

Hello,

I noticed an issue in the parseVcfString method of the NameParser class, where the order of elements extracted from the VCF string does not match the expected fields for the Name object. In particular, the positions of the firstName and additional information seem to be reversed.

Problem observed:

In the current code, the VCF string is split into segments with explode, but the segments are assigned to variables in the wrong order. For example, for a VCF line like N:DUPONT;Thierry;;;, the first name and additional information are reversed.

Proposed solution :

Change the order of variables in the list assignment to correctly reflect the structure of the VCF string. Thus, $firstName receives the first name and $additional receives the additional information, according to the VCF specification.

Here is the corrected code:

final class NameParser extends PropertyParser implements NodeParserInterface
{
    public function parseVcfString(string $value, array $parameters = []): NodeInterface
    {
        @list($firstName, $lastName, $additional, $prefix, $suffix) = explode(';', $value);

        $this->convertEmptyStringToNull([
            $lastName,
            $firstName,
            $additional,
            $prefix,
            $suffix
        ]);

        return new Name($lastName, $firstName, $additional, $prefix, $suffix);
    }
}

Thank you for your time and consideration.

@tups tups force-pushed the debug-NameParser branch from f32758b to 4227d5f Compare January 9, 2024 17:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant