DatabaseMap::getTableByPhpName($phpName) with no namespaces #622 + test #629

Merged
merged 3 commits into from Apr 8, 2013

4 participants

@jaugustin
Propel member

Hi,

I add tests for @ciromattia 's PR #622

But I think there is something wrong with the DatabaseMap and namespaces

We set Name ( table name foo1 ), phpName ( Foo1 ), classname ( namespaced\Foo1 ), but we only use classname to get the right TableMap.

The point is that the method getTableByPhpName never use phpName but only classname which is equals only for non namespaced classes.

@willdurand , @marcj , @staabm what should we do with this ?

@staabm
Propel member

In my opinion it doesn't make sense to have a phpName and a classname... when we have a classname property it should be a calculated field based on the phpName but not a property which can be set separately...?

@ciromattia

@zeuben confirmed it working, so I assume it's good to merge :)

@willdurand
Propel member

this looks good to me but yes I think we don't need both the phpName and the className. As @staabm said, we could calculate the className based on the phpName where this value would be the FQCN.

@ciromattia

Agreed. This deserves an issue on its own IMVHO, though.

@willdurand
Propel member

ping @jaugustin

@jaugustin
Propel member

@willdurand I think you can merge it like this and create a new issue to fix the duplicate method

@willdurand willdurand merged commit c4765f5 into propelorm:master Apr 8, 2013

1 check passed

Details default The Travis build passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment