2
\$\begingroup\$

I just would like to know if I am heading in the right direction with this. I am trying to learn and understand the concept of MVC, and I am doing that while making my own webpage, so I can learn a little bit more each time. I do not want to put too much code here, but I would simply like to ask, am I doing this right, and if not, what would you do different, and most importantly, why?

This little piece of code is in the controller for "Tracks".

public function viewForm()
{
    if($_SERVER['REQUEST_METHOD'] == 'POST') 
    {
        $artistID       = $_POST["artistID"];
        $artistName     = $_POST["artistName"];
        $trackName  = $_POST["trackName"];
        $trackLink  = $_POST["trackLink"];

        $tracks = $this->loadModel('TracksModel');
        $tracks->addTrack($artistID, $artistName, $trackName, $trackLink);

        header('location: ' . URL . 'tracks/viewform?submit=true');
    }
    require 'application/views/tracks/viewform.htm';
}

And this is in the model for it:

public function addTrack($artistID, $artistName, $trackName, $trackLink)
{
    $sql    = "INSERT INTO music_tracks (artistID, artistName, trackName, trackLink) VALUES (:artistID, :artistName, :trackName, :trackLink)";
    $query  = $this->db->prepare($sql);

    $query->bindParam(':artistID', $artistID, PDO::PARAM_INT);
    $query->bindParam(':artistName', $artistName, PDO::PARAM_STR);
    $query->bindParam(':trackName', $trackName, PDO::PARAM_STR);
    $query->bindParam(':trackLink', $trackLink, PDO::PARAM_STR);

    $query->execute(array('artistID' => $artistID, ':artistName' => $artistName, ':trackName' => $trackName, ':trackLink' => $trackLink));
}
\$\endgroup\$
2
\$\begingroup\$
  • You have some spacing issues. This may be because you are improperly mixing tabs and spaces in your indentation.
  • Your use of PDOStatement::bindParam() renders the array passed to PDOStatement::execute() redundant. You only need one or the other.
  • Typically, after issuing a header("Location:"), you use an immediate die or exit to prevent needlessly executing any other script.
\$\endgroup\$
1
  • \$\begingroup\$ Thanks, that second thing you said, I didn't know, makes sense, thanks! :) \$\endgroup\$
    – Hardist
    Aug 8 '14 at 6:46

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.