[RFC] Constants for options and types

[igorsantos07]

I think we hardly write migrations every day, not even every week or maybe every month. Thus, we hardly know by heart how Phinx calls the database fields in a generic manner (is it string? text? and how do I call the two options to limit a float number?)

We got code completion for methods on IDEs, which is great - so we don't have to wonder if it's addColumn or createColumn or whatnot. But we still have to write strings for fixed arguments such as types and options. And then, every here and there when we got to write a new migration, we have to travel to the Phinx docs just to find the correct set of types that we can use.

I'm glad to write a PR that would implement something like this, if I get approval on the idea:

$this->table('user')
    ->addColumn('email', Column::TYPE_STRING, [Column::OPT_LENGTH => 30])
    ->addColumn('age', Column::TYPE_INT, [
        Column::OPT_LIMIT => 99,
        Column::OPT_SIGNED => false
    ])
    ->addColumn('balance', Column::TYPE_FLOAT, [
        Column::OPT_PRECISION => 2,
        Column::OPT_SCALE => 10,
        Column::OPT_DEFAULT => 0
    ])
    ->create();

PS: it seems we already have this type of constant for ForeignKey actions.

[rquadling]

I like this approach. Maybe further refinement ...

$this->table('user')
    ->addColumn('email', ColumnString::TYPE, [ColumnString::OPT_LENGTH => 30])
    ->addColumn('age', ColumnInt::TYPE, [
        ColumnInt::OPT_LIMIT => 99,
        ColumnInt::OPT_SIGNED => false
    ])
    ->addColumn('balance', ColumnFloat::TYPE, [
        ColumnFloat::OPT_PRECISION => 2,
        ColumnFloat::OPT_SCALE => 10,
        ColumnFloat::OPT_DEFAULT => 0
    ])
    ->create();

With this, attempting to use ColumnFloat::TYPE with ColumnString::OPT_LENGTH would look incorrect.

Namespacing the available options per type.

And, with appropriate docblocks, we could build from that information (or at least have a helper like we do for https://github.com/beberlei/assert).

In addition to the docblock, any specific restrictions by adapter can be documented, further helping the developer without the need to look elsewhere.

[igorsantos07]

I find it very odd to use full classes to namespace constants and nothing else. The type of issue that namespacing tries to solve can be tackled with proper docblocks only...

That said, it's still better than bare strings as we currently have. I'm open up for what's decided on the matter.

[rquadling]

I use enum type a lot. Essentially a class with just constants. DocBlocks aren't code. No way to enforce/validate at runtime without complex reflection. Unusual? Maybe, but once defined, significantly harder to break accidentally.

[Koriit]

This was the first thing that hit me when reading documentation. So many strings. I was going to propose the same thing if I didn't find this RFC. Using constants allows for IDE support, code completion, static validation, is less error prone and won't need us to look back to documentation for every little option. I'm strongly in favour of this change and even asking to implement it.

[rquadling]

@Koriit I'm happy to accept pull requests for this. I think it would make a great addition to Phinx.

[b1rdex]

I'd like to suggest https://github.com/antanas-arvasevicius/enumerable-type (or any other php enums implementation) if you want to enforce strong type checks.

The problem with addColumn method 3rd argument is that you can't enforce type for array keys in docblocks, so IDE will not ever help you with static analysis.

[mvrhov]

I find this one better as it also allows for sets: https://github.com/Elao/PhpEnums

[rquadling]

@mvrhov That enum lib looks very interesting. I use https://github.com/eloquent/enumeration.

Watch out for https://github.com/antanas-arvasevicius/enumerable-type/issues/3.

[Koriit]

I'm personally against using those enum-like structures here, firstly because we can't enforce a type for array keys anyway as @b1rdex has already noticed; secondly, because it adds additional complexity which is not outweighed by benefits it could provide(see point one); thirdly being this case not a valid application of enums, which even Elao/PhpEnums does notice it its README.

Thus, using simple class-namespaced constants would be good enough.

[igorsantos07]

@rquadling I'm with some available time and wanted to tackle this one. What's the final decision? Import some Enum library just for this, or simply go with class-namespaced constants?

If classes are the case, should I add those into the Column class we already have (just like the ForeignKey class does) or create some abstract classes just for that? Could those new classes get confusing in some way, or just documenting they're simply constant namespaces would do?

[rquadling]

I'd say go with our own classes. Correctly namespaced (Phinx\Enums perhaps - naming things is not my strong suit). Make sure everything is documented.

[dereuromark]

https://github.com/cakephp/phinx/pull/1739