views:

245

answers:

6

I have a web form that manipulates records in a MySQL database. I have a method for displaying an edit interface for both creating new records and editing them

if ($_POST['new_page']) {
        print "<h2>Create new page</h2>\n";
        $isNew=1;
        $this->EditForm();
    } else if($_POST['edit']){
        print "<h2>Edit page</h2>\n";
        $isNew=0;
        $this->EditForm();
    }

I am trying to use the global variable $isNew to determine where a record is to be added or updated. However, whenever my SaveChanges() function is run, $isNew is always 0. $isNew is declared immediately after the class declaration, outside all of the functions.

class Editor{
    public $isNew;

Full code sample (from http://pastebin.com/40TQFEd5):

When the object is created in index.php, the method HTMLEditorHandler() is called

<?php 

class HTMLEditor{

    var $isNew;

    function SaveChanges($author, $company, $title, $content, $new){
        // Get AuthorID
        // Search database for ID 
        $sql="SELECT ID";
        $sql.=" FROM authors";
        $sql.=" WHERE Name = '$author'";
        $author_id=$this->db->getOne($sql);
        // If author not found, add to database
        if(!$author_id){
            $sql="INSERT INTO authors(Name)";
            $sql.="VALUES ('{$author}')";
            $this->db->query($sql);
            $author_id=mysql_insert_id();
        }
        print "isNew: ".$this->isNew;
        /*if($this->isNew==1){
            $sql="INSERT INTO pages(CompanyID, AuthorID, Title, Content, DateCreated, DateUpdated)";
            $sql.=" VALUES ('{$company}', '{$author_id}', '{$title}', '{$content}', NOW(), NOW())";
            $this->db->query($sql);
        } else if($this->isNew==0){
            print "Not new";
        }*/
    }

    function EditForm($isNew){
        if(isset($_POST['pageID'])){
            $sql="SELECT Name, Title, Content, CompanyID";
            $sql.=" FROM pages, authors\n";
            $sql.=" WHERE pages.AuthorID = authors.ID";
            $sql.=" AND pages.ID = '".$_POST['pageID']."'";

            $result=$this->db->query($sql);
            $row=$result->fetchRow();
            $company=$row['CompanyID'];
        }
        print "<form action=\"{$_SERVER['PHP_SELF']}\" method=\"post\">\n";
            print "<table width=\"100%\"summary=\"New Page\"\n>";
                print "<tr>\n";
                    print "<th>Author: </th>\n";
                    print "<td><input type=\"text\" name=\"author\"";
                        if(isset($row['Name'])){
                            print "value=\"".$row['Name']."\"";
                        }
                    print "/></td>\n";
                print "</tr>\n";
                print "<tr>\n";
                    print "<th>Company: </th>\n";
                    print "<td>\n";
                        $this->ShowCompanies($company);
                    print "</td>\n";
                print "</tr>\n";
                print "<tr>\n";
                    print "<th>Title: </th>\n";
                    print "<td><input type=\"text\" name=\"title\"";
                        if(isset($row['Title'])){
                            print "value=\"".$row['Title']."\"";
                        }
                    print "/></td>\n";
                print "</tr>\n";
                print "<tr>\n";
                    print "<th>Content: </th>\n";
                    print "<td>\n";
                        print $this->myToolBar->EditableArea("content", htmlspecialchars($row['Content']), "100%", 400, "NoSave");
                    print "</td>\n";
                print "</tr>\n";
            print "</table>\n";
            print "<input type=\"submit\" name=\"save\" value=\"Save\"/>\n";
            print "<input type=\"submit\" name=\"\" value=\"Cancel\"/>\n";
        print "</form>\n";
    }

    function DefaultForm(){
        print "<form action=\"{$_SERVER['PHP_SELF']}\" method=\"post\">\n";
            print "<input type=\"submit\" name=\"new_page\" value=\"Create a new page\"/>";
            print "<h2>Edit an existing page</h2>\n";
            print "<table summary=\"Edit Page\">\n";
                print "<tr><th>Year</th><td>";
                    print "<select name=\"year\" onchange=\"showPages()\" id=\"year_select\">\n";
                    for ($year=date('Y'), $max_year=date('Y')-10; $year > $max_year; $year--) { 
                            print "<option value=\"".$year."\">".$year."</option>\n";
                        }
                    print "</select>\n";
                print "</td></tr>";
                print "<tr><th>Company: </th><td>";
                    $sql="SELECT organisations.OrgID, companynames.CompanyName";
                    $sql.=" FROM qsvision.organisations";
                    $sql.=" LEFT JOIN qsvision.companynames";
                    $sql.=" ON qsvision.organisations.CompanyID=qsvision.companynames.CompanyID";
                    $sql.=" WHERE CompanyName!=''";
                    $sql.=" GROUP BY companynames.CompanyID";
                    $sql.=" ORDER BY companynames.CompanyName ASC";
                    $organisations=$this->db->getAll($sql);

                    print "<select name=\"org_id\" onchange=\"showPages()\" id=\"org_id\">\n";
                        print "<option value=\"\">[Select...]</option>\n";
                        for($i=0, $max_i=count($organisations); $i<$max_i; $i++){
                            print "<option value=\"{$organisations[$i]['OrgID']}\"";
                            if($site['OrgID']==$organisations[$i]['OrgID']){
                                print " selected=\"selected\"";
                            }
                            print ">".htmlspecialchars($organisations[$i]['CompanyName'])."</option>\n";
                        }
                    print "</select>\n";
                print "</td></tr>\n";
                print "</table>";
                print "<div id=\"results_table\"></div>";
        print "</form>";
    }

    function HTMLEditorHandler(){
        if ($_POST['new_page']) {
            print "<h2>Create new page</h2>\n";
            $this->EditForm(true);
        } else if($_POST['edit']){
            print "<h2>Edit page</h2>\n";
            $this->EditForm(false);
        } else if($_POST['delete']){
            $this->DeletePage();
            $this->DefaultForm();
        } else if($_POST['save']){
            $this->SaveChanges($_POST['author'], $_POST['org_id'], $_POST['title'], $_POST['content'],$this->isNew);
            $this->DefaultForm();
        } else {
            $this->DefaultForm();
        }
    }
}

?>
+1  A: 

access it with this:

$this->isNew = 1;
webbiedave
+2  A: 

You have to use $this, when referencing to instance properties inside a class method:

$this->isNew = 1;
WishCow
It's not making a difference
Robert
Then we'll need to see more of your code.
webbiedave
I have a button for creating new records print "<input type=\"submit\" name=\"new_page\" value=\"Create a new page\"/>";and a button for editing a page print "<input type=\"submit\" name=\"edit\" value=\"Edit"/>";they both call the same method for inputting/editing a record which has a button for saving print "<input type=\"submit\" name=\"save\" value=\"Save\"/>\n";which calls a save method if($_POST['save']){ $this->SaveChanges(//data to be saved);but print statement show that as soon as this method is called, $isNew is set to nothing
Robert
I meant more of your class code.
webbiedave
Dont get much space in commentshttp://pastebin.com/40TQFEd5
Robert
Put $this->isNew = 1 to line 121.
WishCow
A: 

By defining public $isNew, you are creating a class property, not a global. Class properties are accessed using the $this keyword, just as you have done with your method call. You meant to do this:

class Editor {
    public $isNew;

    function whatever() {
        if ($_POST['new_page']) {
            print "<h2>Create new page</h2>\n";
            $this->isNew=1;
            $this->EditForm();
        } else if($_POST['edit']){
            print "<h2>Edit page</h2>\n";
            $this->isNew=0;
            $this->EditForm();
        }
    }

    function EditForm() {
        echo $this->isNew;
    }
}

Is there any reason your're not just passing the "new" flag to EditForm() as an argument?

Adam Backstrom
The function for saving records is only called once because creating and editing records both use the same edit function. I cant see how I can pass it a variable
Robert
Definition: `function EditForm( $is_new ) { echo $is_new; }`. Calling: `$this->EditForm(1);`.
Adam Backstrom
EditForm() has a submit button for running the save method on $_POST['save']. It doesnt matter what I pass to EditForm() because EditForm() doesnt call SaveChanges()
Robert
+2  A: 

Those 2 variables have two completely distinct values. A member variable does not override one in an outer scope, it is there for an instance of that class only. So if you want to access the global value, you need to use the keyword global:

class Editor {
    public function foo() {
        global $isNew;
        if ($isNew) {
            # ...
        }
    }
}

Note that using globals this way is not good practice, the idea behind OOP is that you put everything you need within the class into the class. OTOH if this value controls the behaviour of just one function, you should rather pass it as a parameter to that function instead of accessing the global.

EDIT after code update: You're not setting your variable ($isNew) anywhere. Just a guess, but did you want to set it at the start of EditForm? That line would be $this->isNew = $isNew;.

soulmerge
I thought it was in the class? Also, the function for saving records is only called once because creating and editing records both use the same edit function. I cant see how I can pass it a variable
Robert
@Robert: I think we need to see the complete function. Could you edit your question to add the missing info?
soulmerge
A: 

I would recommend passing the 'new' flag into the class method. ie.

class Editor {
  public function edit($new = false) {
    if ($new) {
      print "<h2>Create new page</h2>\n";
      $this->edit_form();
    } else {
      print "<h2>Edit page</h2>\n";
      $this->edit_form();
    }
  }

  public function edit_form() {
    // form stuff
  }
}

You could just call edit_form() and pass the flag into there as well. That way you could do conditional stuff in your edit_form method as well.

RDL
A: 

Looking at your full code from http://pastebin.com/40TQFEd5, it's clear you don't understand how the PHP process flow works. In short, every time a page is loaded (either by GET or POST), it's like your program is starting from scratch. The only way data is preserved between separate page loads is if you explicitly store it somewhere to be preserved - such as in a server-side SESSION variable, or client side: * output it in a link so it can be picked up in a GET variable * output a form field (eg, hidden field) so it can be picked up in a GET or POST variable (depending on form submission method) * call SetCookie() or output javascript that sets a cookie so it can be picked up in a COOKIE variable

The relevant bit of code:

    if ($_POST['new_page']) {
        print "<h2>Create new page</h2>\n";
        $this->EditForm(true); 
    } else if($_POST['edit']){
        print "<h2>Edit page</h2>\n";
        $this->EditForm(false);
    } else if($_POST['save']){
        $this->SaveChanges($_POST['author'], $_POST['org_id'], $_POST['title'], $_POST['content'],$this->isNew);
        $this->DefaultForm();

Aside from the problems that you're not even setting the $isNew variable in your code example, the real problem is that the flow works like this:

  • Page is loaded, with the POST value 'edit' or 'new_page'. A new instance of HTMLEditor class is created, and (though your code doesn't actually do this now) $isNew is set appropriately based on the POST value. Form is output to page, and sent to client
  • User fills out form in browser, and hits submit
  • Page is loaded, with the POST value 'save'. A new instance of HTMLEditor class is created. isSet is unknown, since it was not preserved and sent to the server again.

So simple solution: in your EditForm() method, output a hidden field that includes the isSet value, or even better, the post ID value.


As an aside, your code can use some work. There is at least one SQL injection vulnerability:

$sql.=" AND pages.ID = '".$_POST['pageID']."'";

Indenting your print statements based on HTML makes it hard to read the code:

        print "<table width=\"100%\"summary=\"New Page\"\n>";
            print "<tr>\n";
                print "<th>Author: </th>\n";
                print "<td><input type=\"text\" name=\"author\"";
                    if(isset($row['Name'])){
                        print "value=\"".$row['Name']."\"";
                    }
                print "/></td>\n";

and indeed, having that much form output shown as print statements is hard to read and maintain. I'd suggest you look into a template engine: See http://stackoverflow.com/questions/62617/whats-the-best-way-to-separate-php-code-and-html or http://stackoverflow.com/questions/62605/php-as-a-template-language-or-some-other-php-templating-script

gregmac
Thanks very much. I hadn't realized what I was doing was wrong and your suggestion has solved my problem. I have pointed out the SQL injection vulnerability to my superior and I don't think it'll be an issue because this will only be used in-house and we have magic quotes enabled. As for the print statements, I don't like them either but they're the company style guidelines =(. I greatly appreciate you helping further my knowledge and understanding of PHP =)
Robert