bad code generation

Bug #284702 reported by Adam Janosek on 2008-10-16
2
Affects Status Importance Assigned to Milestone
CoughPHP
High
Anthony Bush

Bug Description

hello, i have this (two) sql tables

CREATE TABLE `network` (
  `id` int(11) NOT NULL auto_increment COMMENT 'PK',
  `interfaceId` int(11) default NULL COMMENT 'FK_interface_id',
  `ipAdresa` varchar(16) collate utf8_czech_ci NOT NULL COMMENT 'ip adresa site',
  `maska` int(16) unsigned NOT NULL COMMENT 'maska site',
  `ipRouter` varchar(16) collate utf8_czech_ci default NULL COMMENT 'ip adresa routeru',
  `verejna` int(1) unsigned NOT NULL default '0' COMMENT 'boolean 1=je verejna / 0=neni verejna',
  `isp` enum('sumnet','poda','sloane') collate utf8_czech_ci NOT NULL default 'sumnet' COMMENT 'urceni ktery isp vlastni tuto sit , poda a sloane maji verejne',
  PRIMARY KEY (`id`),
  KEY `interfaceId` (`interfaceId`),
  KEY `verejna` (`verejna`),
  KEY `isp` (`isp`)
) ENGINE=InnoDB DEFAULT CHARSET=utf8 COLLATE=utf8_czech_ci COMMENT='ip site verejne/neverejne' AUTO_INCREMENT=7 ;

CREATE TABLE `custPc` (
  `id` int(11) NOT NULL auto_increment COMMENT 'PK',
  `customerId` int(11) default NULL COMMENT 'FK customer id',
  `popis` varchar(50) collate utf8_czech_ci NOT NULL COMMENT 'textovy popis pocitace',
  `macAdresa` varchar(12) collate utf8_czech_ci default NULL COMMENT 'mac adresa stroje',
  `networkId` int(11) default NULL COMMENT 'FK network id',
  `ipAdresa` varchar(16) collate utf8_czech_ci default NULL COMMENT 'ip adresa stroje',
  `networkIdVerejna` int(11) default NULL COMMENT 'FK network id pro verejnou ip adresu',
  `ipAdresaVerejna` varchar(16) collate utf8_czech_ci default NULL COMMENT 'verejna ip adresa stroje',
  `down` int(11) default NULL COMMENT 'rychlost downloadu',
  `up` int(11) default NULL COMMENT 'rychlost uploadu',
  PRIMARY KEY (`id`),
  KEY `customerId` (`customerId`),
  KEY `networkId` (`networkId`),
  KEY `networkIdVerejna` (`networkIdVerejna`)
) ENGINE=InnoDB DEFAULT CHARSET=utf8 COLLATE=utf8_czech_ci COMMENT='stroje zakazniku' AUTO_INCREMENT=8 ;

ALTER TABLE `custPc`
  ADD CONSTRAINT `custPc_ibfk_2` FOREIGN KEY (`customerId`) REFERENCES `customer` (`usr_id`) ON DELETE CASCADE ON UPDATE CASCADE,
  ADD CONSTRAINT `custPc_ibfk_3` FOREIGN KEY (`networkId`) REFERENCES `network` (`id`) ON DELETE CASCADE ON UPDATE CASCADE,
  ADD CONSTRAINT `custPc_ibfk_4` FOREIGN KEY (`networkIdVerejna`) REFERENCES `network` (`id`) ON DELETE CASCADE ON UPDATE CASCADE;

so at brief, there are two relation between this two tables (networkId and networkIdVerejna(in eng.networkIdPublic) ), and when i generate classes in /models/generated/CustPc_Generated.class.php is like this:

<?php

/**
 * This is the base class for CustPc.
 *
 * @see CustPc, CoughObject
 **/
abstract class CustPc_Generated extends CoughObject {

...omited

 public function loadNetwork_Object() {
  $this->setNetwork_Object(Network::constructByKey($this->getNetworkId()));
 }

 public function getNetwork_Object() {
  if (!isset($this->objects['Network_Object'])) {
   $this->loadNetwork_Object();
  }
  return $this->objects['Network_Object'];
 }

 public function setNetwork_Object($network) {
  $this->objects['Network_Object'] = $network;
 }

 public function loadNetwork_Object() {
  $this->setNetwork_Object(Network::constructByKey($this->getNetworkIdVerejna()));
 }

 public function getNetwork_Object() {
  if (!isset($this->objects['Network_Object'])) {
   $this->loadNetwork_Object();
  }
  return $this->objects['Network_Object'];
 }

 public function setNetwork_Object($network) {
  $this->objects['Network_Object'] = $network;
 }

 // Generated one-to-many collection loaders, getters, setters, adders, and removers
...omited

so load/get/set/Network_Object is twice time in new generated code. I think that's bad behaviour.
Isn't it ?

i'm testing it on CoughPHP 1.3.2 (2008-09-22)

  Adam Janosek

description: updated
Anthony Bush (awbush) wrote :

Thank you for bug report, we will look into this soon.

Changed in coughphp:
assignee: nobody → awbush
importance: Undecided → High
Anthony Bush (awbush) wrote :

This can be resolved a number of ways:

1. Follow the out-of-box convention and use "_id" for ID columns, e.g.:

 * Change custPc.networkId to network_id
 * Change custPc.networkIdVerejna to verejna_network_id

2. Customize the "field_settings/id_regex" config value:

 * Add a line to the `database_schema_generator.inc.php` file:

  $config['databases']['db_name']['custPc']['field_settings']['id_regex'] = '/^(.*)Id$/';

 * Make sure all ID columns end in "Id" (the custPc.networkIdVerejna column would need to be changed to networkVerejnaId or verejnaNetworkId).

3. Release a change to CoughPHP's one-to-one entity name detection. Out-of-box the generator will try to strip off "_id" and use the result as for method names. In the event it is unable to do this (e.g. column names not following convention), it will use the related table name. This works fine until there is more than one column linking to the same table, as this bug demonstrates. The CoughGeneratorConfig::getForeignTableAliasName() method should updated to check for this case and instead just fail over to the column name instead of using the table name.

I've committed the fix for #3 and will release it on the web site today. I still recommend a schema change to address points #1 or #2, or just make alias methods that don't include the "Id" in the method name, e.g.:

 public function getNetwork_Object() {
  return $this->getNetworkId_Object();
 }
 public function getNetworkVerejna_Object() {
  return $this->getNetworkIdVerejna_Object();
 }

Changed in coughphp:
status: New → Fix Committed
Anthony Bush (awbush) wrote :
Changed in coughphp:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers