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

Use constant consistently #5837

Merged
merged 1 commit into from
Jun 19, 2016
Merged

Use constant consistently #5837

merged 1 commit into from
Jun 19, 2016

Conversation

foaly-nr1
Copy link
Contributor

@foaly-nr1 foaly-nr1 commented May 25, 2016

The type name returned by getName() must match the type name stored in the comment, otherwise the mapping will fail.

The example code should hence use the constant throughout, both for getName() and getSQLDeclaration():

class EnumVisibilityType extends Type
{
    const ENUM_VISIBILITY = 'enumvisibility';
    const STATUS_VISIBLE = 'visible';
    const STATUS_INVISIBLE = 'invisible';

    public function getSQLDeclaration(array $fieldDeclaration, AbstractPlatform $platform)
    {
        // TODO: Use constant
        return "ENUM('visible', 'invisible') COMMENT '(DC2Type:enumvisibility)'";
    }

    public function convertToPHPValue($value, AbstractPlatform $platform)
    {
        return $value;
    }

    public function convertToDatabaseValue($value, AbstractPlatform $platform)
    {
        if (!in_array($value, array(self::STATUS_VISIBLE, self::STATUS_INVISIBLE))) {
            throw new \InvalidArgumentException("Invalid status");
        }
        return $value;
    }

    public function getName()
    {
        return self::ENUM_VISIBILITY;
    }
}

@Ocramius Ocramius added this to the 2.6.0 milestone Jun 19, 2016
@Ocramius Ocramius self-assigned this Jun 19, 2016
@Ocramius
Copy link
Member

@foaly-nr1 thanks! \o/

@Ocramius Ocramius merged commit 1162440 into doctrine:master Jun 19, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants