Keep me from developing bad OOP habits!

[NotionCommotion]

I have an application which takes input from GET or POST, creates a controller, and evokes the appropriate task.  While not exactly how it is done, it is pretty close to:
 

$controller=new childController();
$task=array_merge(['task'=>'default'],$_GET,$_POST)['task'];
$controller->$task();

Now, I have two tasks which are almost identical, and the only difference is one passes "foo" to documents::removeDocument(), and the other passes "bar".  To implement, I did the following:
 

class childController extends parentController
{
    public function deleteFooDocument(){$this->deleteDocumentHelper('foo');}
    public function deleteBarDocument(){$this->deleteDocumentHelper('bar');}
}

<?php
class parentController
{
    protected function deleteDocumentHelper($type){
        if(isset($_POST['id'],$_POST['doc_id'])){
            if(documents::removeDocument($type,$_POST['doc_id'],$_POST['id']))
            {
                $success=1;
                //Ability to replace the following line with one or more lines
                $this->getModel()->updateParentAudit($this->audit_table,$_POST['id']);
            }
            else {$success=0;}
            header('Content-Type: application/json;');
            $this->dontCache();
            echo(json_encode(array('success'=>$success)));
        }
        else {exit($this->missingPage());}        
    }
}  
?>

Now, I realize one of the methods should do $this->getModel()->updateParentAudit($this->audit_table,$_POST['id']); upon successfully deleting a document, but the other should do something else, and am struggling on how to deal with it.

 

I think I am going down a slippery slope.  Did a little research on "helper methods", and some say they are evil.  Should I be doing this totally differently?

[kicken]

Based on the example you gave, I see no reason to create a helper method on the parent class. The deleteFoo and deleteBar methods are part of the same child class, so just make the helper a private method there.

 

As far as being able to add arbitrary code at the sccess branch, one thing you can do is use a callback method.

<?php
class childController extends parentController {
    public function deleteFooDocument(){$this->deleteDocumentHelper('foo');}
    public function deleteBarDocument(){
        $this->deleteDocumentHelper('bar', function($doc_id, $id){
            $this->getModel()->updateParentAudit($this->audit_table, $id);
        });
    }

    private function deleteDocumentHelper($type, $successCallback=null){
        if(isset($_POST['id'],$_POST['doc_id'])){
            if(documents::removeDocument($type,$_POST['doc_id'],$_POST['id']))
            {
                $success=1;
                //Ability to replace the following line with one or more lines
                if ($successCallback){
                    $successCallback($_POST['doc_id'], $_POST['id']);
                }
            }
            else {$success=0;}
            header('Content-Type: application/json;');
            $this->dontCache();
            echo(json_encode(array('success'=>$success)));
        }
        else {exit($this->missingPage());} 
    }
}
?>

Alternativly you could use a couple separate helper methods, but keep the if branch for the delete in the controller.

<?php
class childController extends parentController {
    public function deleteFooDocument(){
        $this->checkExists();
        $success = documents::removeDocument('foo',$_POST['doc_id'],$_POST['id']);
        $this->outputResponse($success);
    }

    public function deleteBarDocument(){
        $this->checkExists();
        $success = documents::removeDocument('bar',$_POST['doc_id'],$_POST['id']);
        if ($success){
            $this->getModel()->updateParentAudit($this->audit_table, $_POST['id']);
        }
        $this->outputResponse($success);
    }

    private function checkExists(){
        if (!isset($_POST['id'], $_POST['doc_id'])){
            exit($this->missingPage());
        }
    }

    private function outputResponse($success){
        header('Content-Type: application/json;');
        $this->dontCache();
        echo(json_encode(array('success'=>$success)));
    }
}
?>

[NotionCommotion]

Based on the example you gave, I see no reason to create a helper method on the parent class.

 

My bad, I didn't give you all the info.  These methods are used with other child classes as well.

 

 

As far as being able to add arbitrary code at the sccess branch, one thing you can do is use a callback method.

 

Yea, I've been messing around with doing so.  Is this considered a "right" way of doing this?

 

 

 

Alternativly you could use a couple separate helper methods, but keep the if branch for the delete in the controller.

Or is this considered the correct way, or should I be doing things totally differently?