Using CakePHP models/controllers properly

Question

I'd like to know if I'm doing this properly. Basically, I have 3 models: Student, Course, StudentCourse. I'm running into problems when I'm trying to sign a student up for a course (I've removed a lot of the non-important code - auth component, view, etc).

// Courses Controller (ie: /courses/signup/6)
class CoursesController extends AppController {
        public function signup($id = null) {
            $this->Course->id = $id;
            if (!$this->Course->exists()) {
                throw new NotFoundException(__('Invalid course'));
            }

            // Let's check to see if the user can signup for the course
            if($this->Course->canSignupForCourse($this->Auth->user('id'))) {

            } else {
                $this->Session->setFlash(__('You cannot signup for that course.'), 'flash_error');      
                $this->redirect(array('controller' => 'pages', 'action' => 'calendar'));
            }
    }   
}

// Course Model:
class Course extends AppModel {
    // Class 'hasMany' StudentCourse
    // Class 'habtm' Student

    public function isOwnedBy($course, $user) {
        return $this->field('id', array('id' => $course, 'student_id' => $user)) === $course;
    }

    public function isApproved() {
        $course = $this->read(null);

        if($course['Course']['approved_by'] != 0) {
            return true;
        }

        return false;
    }

    public function isComplete() {
        $course = $this->read(null);

        if($course['Course']['completed'] != 0) {
            return true;
        }

        return false;
    }   

    public function canSignupForCourse($studentId) {    

        if(! $this->StudentCourse->isSignedUpForCourse($studentId) && $this->isApproved() && $this->isComplete()) {
            return true;
        }       

        return false;
    }

}

// Student Model
class Student extends AppModel {
    // Student 'hasMany' Course
    // Student 'hasMany' StudentCourse
}

// StudentCourse Model
class StudentCourse extends AppModel {
    // StudentCourse 'belongsTo' Student
    // StudentCourse 'belongsTo' Course
    public function isSignedUpForCourse($studentId = null, $courseId = null) {
        return $this->find('count', array('conditions' => array('StudentCourse.course_id' => $courseId, 'StudentCourse.student_id' => $studentId))) != 0 ? TRUE : FALSE;
    }   
}

Basically, am I going about this the right way, are my relationships correct, or, is there a better/easier way to figure out if a student can signup for a course.

Answer 1

To me your code and relations looks good! You removed the redundancy in your database by splitting the tables up. Your cake really looks cakes way to me!

Just make sure you actually do something with the exception thrown and it should be all good.

---

Edit : I know this is a little late, but I wanted to complete my answer for the sake of completeness.

Your code style looks pretty good to me. The functions names are well chose. What I mean by that is by putting verb first you make the function a lot easier to find and understand.

Also, putting the logic in the model is the good choice. Continue to keep thin controllers.

Answer 2

Redundant bool returning:

public function isApproved() {
    $course = $this->read(null);

    return $course['Course']['approved_by'] != 0;
    /* if($course['Course']['approved_by'] != 0) {
        return true;
    }

    return false; */
}

Answer 3

I agree with Hugo, looks good.

I'd just like to add a little bit of best practice; Controllers and Models should have public $name="ControllersName"; and public $name="ModelName"; at the top respectively.