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

attribute name not equal to parent name issue #7

Closed
Rafeethu opened this issue Mar 29, 2012 · 8 comments
Closed

attribute name not equal to parent name issue #7

Rafeethu opened this issue Mar 29, 2012 · 8 comments

Comments

@Rafeethu
Copy link
Contributor

hey guys,

when i create an object which has two attributes with different names which refer the same parent type, one attribute name has to be different than the parent name. this is creating an issue when we call object->GetAttribut2 function.

is there a workaround to this?

Rgds
Rifky

@Rafeethu
Copy link
Contributor Author

yes. a simple example is

http://www.phpobjectgenerator.com/?language=php5.1&wrapper=pdo&pdoDriver=mysql&objectName=Invoice&attributeList=array+%28%0A++0+%3D%3E+%27customer%27%2C%0A++1+%3D%3E+%27address%27%2C%0A++2+%3D%3E+%27customer_address%27%2C%0A++3+%3D%3E+%27shipping_address%27%2C%0A%29&typeList=array%2B%2528%250A%2B%2B0%2B%253D%253E%2B%2527VARCHAR%2528255%2529%2527%252C%250A%2B%2B1%2B%253D%253E%2B%2527BELONGSTO%2527%252C%250A%2B%2B2%2B%253D%253E%2B%2527BELONGSTO%2527%252C%250A%2B%2B3%2B%253D%253E%2B%2527BELONGSTO%2527%252C%250A%2529

here address, customer_address, shipping_address are all child of address class

$inv = new Invoice();
[...]
$inv_address = $inv->GetAddress(); <-------- this will work
$inv_shippingaddress = $inv->GetShipping_address(); <-------- this is not working...

i think if we can mention the parent or child original class name and then generate objects will solve the issue.

is the same error occuring for you?

@Loksly
Copy link
Collaborator

Loksly commented Mar 29, 2012

You are right it won't work as the generator generates code like these:

/**

  • Associates the shipping_address object to this one
    • @return boolean
      /
      function GetShipping_address()
      {
      $shipping_address = new shipping_address();
      return $shipping_address->Get($this->shipping_addressId);
      }
      /
      *
    • Associates the shipping_address object to this one
  • @return
    */
    function SetShipping_address(&$shipping_address)
    {
    $this->shipping_addressId = $shipping_address->shipping_addressId;
    }

which should be :

/**

  • Associates the shipping_address object to this one
    • @return boolean
      /
      function GetShipping_address()
      {
      $shipping_address = new address();
      return $shipping_address->Get($this->shipping_addressId);
      }
      /
      *
    • Associates the shipping_address object to this one
  • @return
    */
    function SetShipping_address(&$shipping_address)
    {
    $this->shipping_addressId = $shipping_address->addressId;
    }

There should be a way to choose the class for these fields.
The generator doesn't have enough information.
I'll see if I can add that funcionality as soon as possible.
Change the code of the class by yourself meanwhile.

Have a nice day!

@Rafeethu
Copy link
Contributor Author

i have changed the coding on my local server and it's working now.. i'll try to upload it to git asap so that you can review it.
i mean the pog code.

@Rafeethu
Copy link
Contributor Author

Loksly,

i have changed the coding to accept parent object name. can you revise and see whether it's ok.

mean time any idea of incorporating prepared statements in to pog? is using prepared statements rather than direct script will cause performance issue in pog?

rgds
Rifky

@Loksly
Copy link
Collaborator

Loksly commented Apr 27, 2012

I've read the code right now. I need more time for a further review.
I've seen you've change the wsdl for adding a new parameter (classlist),
that breaks backwards compatibility and I think joel should be the one
who has to be agree with that before accepting the commit. I agree with you
with the use of jquery. It works fine and is expected to working fine in the future.
About using prepared statements there are some issues. There should be a way
to choose if you want the generated code to use them or not. It's easy to generate
code for a Get, Insert and Delete functions and harder for a GetList or for a DeleteList functions.
I think is a mistake to break compatibility with code already generated. Imaging a code like this:

$restrictions = array();
$restrictions[] = array( 'year','<',date('Y') );
$restrictions[] = array( ' and exists (select * invoices where  invoices.`deleted`=0 and invoices.project=project.projectId )' );
$projects = $project->GetList($restrictions,'title');

What kind of code for GetList do you suggest?
This kind?

(GetList function)
...
$counter = 0;
$params = array();

for ($i=0, $c=sizeof($fcv_array); $i<$c; $i++)
{
    if (sizeof($fcv_array[$i]) == 1)
    {
        $this->pog_query .= " ".$fcv_array[$i][0]." ";
        continue;
    }
    else
    {
        if ($i > 0 && sizeof($fcv_array[$i-1]) != 1)
        {
            $this->pog_query .= " AND ";
        }
        if (isset($this->pog_attribute_type[$fcv_array[$i][0]]['db_attributes']) && $this->pog_attribute_type[$fcv_array[$i][0]]['db_attributes'][0] != 'NUMERIC' && $this->pog_attribute_type[$fcv_array[$i][0]]['db_attributes'][0] != 'SET')
        {
            if ($GLOBALS['configuration']['db_encoding'] == 1)
            {
                $value = POG_Base::IsColumn($fcv_array[$i][2]) ? "BASE64_DECODE(".$fcv_array[$i][2].")" : "'".$fcv_array[$i][2]."'";
                $this->pog_query .= "BASE64_DECODE(`".$fcv_array[$i][0]."`) ".$fcv_array[$i][1]." ".$value;
            }
            else
            {
                if (POG_Base::IsColumn($fcv_array[$i][2]))
                {
                    $value =  POG_Base::IsColumn($fcv_array[$i][2]) ? $fcv_array[$i][2] : "'".$this->Escape($fcv_array[$i][2])."'";
                    $this->pog_query .= "`".$fcv_array[$i][0]."` ".$fcv_array[$i][1]." ".$value;
                }else{
                    $value =  POG_Base::IsColumn($fcv_array[$i][2]) ? $fcv_array[$i][2] : "'".$this->Escape($fcv_array[$i][2])."'";
                    $this->pog_query .= "`".$fcv_array[$i][0]."` ".$fcv_array[$i][1]." :param".$counter;
                    $params[':param'.$counter] = $value;

                    $counter++;
                }
            }
        }
        else
        {
            $value = POG_Base::IsColumn($fcv_array[$i][2]) ? $fcv_array[$i][2] : "'".$fcv_array[$i][2]."'";
            $this->pog_query .= "`".$fcv_array[$i][0]."` ".$fcv_array[$i][1]." ".$value;
        }
    }
}

if (counter>0)
{
    $preparedStatement = Database::PreparedStatement($this->pog_query);
    foreach($params as $name => $val)
        $preparedStatement->bindParam($name,$val);
    $preparedStatement->execute()
}else{
//current code
}

//Note: I've written this code here (& without any test)

As I have said before, we have to see if the new code works properly with the one users of pog might have written.
About performance I've read is not very appreciable. Better table/database design increases performance more significantly. I guess security (SQL injection prevention) is a better reason for using prepared statements.

@joelwan
Copy link
Owner

joelwan commented May 14, 2012

I think running the existing unit tests as well as creating new ones to validate backward compatibility of existing plugins should be a pre-requisite before code can be merged. Thoughts?

@Loksly
Copy link
Collaborator

Loksly commented May 14, 2012

You are right, I'm sorry.
I did the commit/merge using the mobile phone.
I'll check the code tonight (in about 5 hours).
I already read it and it looks like it works.
I confess I haven't run the tests as I should.

So sorry.

@joelwan joelwan closed this as completed Apr 17, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants