Tharos / LeanMapper

Lean Mapper is a tiny ORM based on powerful Dibi database abstraction library for PHP.
MIT License
87 stars 35 forks source link

Mapper::getRelationshipColumn() posielat dalsi parameter $field #27

Closed achtan closed 10 years ago

achtan commented 10 years ago

bolo by super kedy si do Mapper::getRelationshipColumn() posielal ako 3. paremeter $field

lebo mam:

@property \Tralandia\Language\Language $defaultLanguage m:hasOne

a nazov stlpca v DB mam defaultLanguage_id cize {$field}_id a preto by sa mi tam hodila ta premenna field

viem ze to mozem dopisat do m:hasOne(defaultLanguage_id:) ale ja to pouzivam ako konvenciu tak aby som to nemusel vsade dopisovat a vedel nastavit len na jednom mieste

co ty na to?

pmachan commented 10 years ago

Tak si udělej base entitu, kterou budeš používat tam, kde je potřeba defaultLanguage a nemusíš to pak všude dopisovat

achtan commented 10 years ago

mne nejde len o tento jeden konkretny pripad, ale mam ich mnoho... a vsade je to rovnaka konvencia {$field}_id

pmachan commented 10 years ago

Tak pokud je to všude stejná konvence tak proč si nenapíšeš svůj mapper, který ji bude využívat (nic tě nenutí používat konvenci kterou má DefaultMapper).

JInak k té base entite:

/**
* @property \Tralandia\Language\Language $defaultLanguage m:hasOne(defaultLanguage_id:)
*/
class BaseEntity extends \LeanMapper\Entity { }

// Text bude obsahovat $defaultLanguage od BaseEntity 
class Text extends BaseEntity  { }
achtan commented 10 years ago

no offence, ale zda sa mi ze si nepochopil co tu riesim, skusim pockat na to co povie @Tharos

PS: vlastny mapper samozrejme pouzivam a prave preto by som rad v Mapper::getRelationshipColumn() mal parameter $field ako je to aj v Mapper::getColumn()

Tharos commented 10 years ago

Vysvětlím všechny problémy, které s tímhle mám…

1) Rozhraní IMapper je už teď poměrně košaté a pro nově příchozí ne úplně intuitivní. Každý další parametr by jej ještě zesložitil. Všichni, kdo si implementují vlastní mapper nebo aktivně používají defaultní, by museli řešit, co ten parametr znamená a jestli se jich nějak týká. Z tohoto důvodu velmi nerad IMapper nějak upravuji (či dokonce rozšiřuji)…

2) Tebou navržený parametr by byl nekonzistentní s těmi, které už ta metoda má. $sourceTable i $targetTable jsou „nízkoúrovňové“ (databázové) hodnoty a Ty bys mezi to přimíchal „vysokoúrovňový“ (objektový) $field. Podobná nekonzistence v mapperu nikde jinde není. :)

3) Tvá konvence mi přijde taková zrádná. :) De facto si děláš databázi závislou na entitách a podle mě je to zbytečná komplikace. Sice provázání databáze s entitami je v Lean Mapperu těsné (viz getColumn a getEntityField), ale tohle je ještě jiný level. :) No a ten navržený třetí parametr by k něčemu takovému v podstatě nabádal a to by mi asi docela vadilo…

Z výše uvedených důvodů se k této změně nepřikláním.

achtan commented 10 years ago

k tomu 3. bodu: hej mas pravdu, mam entity usko prepojene s DB lebo projekt povodne bezal na Doctrine 2, a tam pouzivali takuto konvenciu... to je inak argument na zvazenie, ze na kolko zjednodusit prechod z Doctrine 2 na LM :)

ale ok, chapem a respektujem tvoje rozhodnutie