Class/model hierarchy for a school-management application

Question

Please find how I would model a system that had teachers, students, classes and parents. Instead of creating a model for each user type, I created only two models, User and Profile. Each user can have multiple profiles and each profile can have a unique profile_type.

I would love to get some feedback on my proposed architecture.

class User < ActiveRecord::Base
 has_many :profiles
 has_many :user_klasses
 has_many :klasses, through: :user_klasses
 def get_student_classes
 student_profile = profiles.select { |p| p.profile_type == 'student' }.first
 return [] if student_profile.nil?
 return klasses.select { |k| k.profile_id == student_profile.id }
 end
 def get_teacher_classes
 teacher_profile = profiles.select { |p| p.profile_type == 'teacher'}.first
 return [] if teacher_profile.nil?
 return klasses.select { |k| k.profile_id == teacher_profile.id }
 end
end
class Klass < ActiveRecord::Base
 has_many :user_klasses
 has_many :users, through: :user_klasses
 belongs_to :teacher, class_name: 'Profile', -> { where profile_type: 'teacher' },
 dependent: :destroy
end
class UserKlasses < ActiveRecord::Base
 belongs_to :user
 belongs_to :klass
end
class Profile < ActiveRecord::Base
 belongs_to :user
end

Answer 1

You mention that you would rather not create extra models but I would consider creating a model for Teacher, Student and for Parent. It is not really that much more work but will probably keep the separation between the concepts much cleaner in the long run (as per Damien's comments). I personally would also use names like TeacherRole and ParentRole or TeacherFacet.

I would also consider keeping the cardinality as has_one unless you need to deal with the possibility that a user needs more than one teacher profile (possibly because they teach in more than one places) or more than one parent profile (maybe they have multiple children).

You obviously haven't shown your complete data model but it looks wrong that each user is linked to classes through the user_klasses relation but then you query classes using a profile_id field on the Klass. Firstly you probably want to query the profile_id on the UserKlass model (the profile_id on Klass looks like it refer to the teacher's profile) but you should probably put the klasses relation on the Profile model (or whatever you call it).

One other thing I noticed here:

 belongs_to :teacher, class_name: 'Profile', -> { where profile_type: 'teacher' },
 dependent: :destroy

You probably don't want to remove the teacher record if a class is deleted, they may be teaching other classes after all. However you probably want to delete UserKlass if the Klass (or User for that matter) is deleted. Likewise it would make sense to delete the Profile record when a User is deleted.

Answer 2

Though there is nothing strictly wrong with your proposed design, I would say be wary of the additional data and behaviour that can be tied to students, teachers, and parents.

What you want to avoid is:

class Profile
 def general_method
 if profile_type == 'student'
 # do this
 elsif profile_type == 'parent'
 # do this
 elseif profile_type == 'teacher'
 # do this
 end
 end
end

One way of resolving this would be single table inheritance, but that would only resolve the behavioural pollution. The data would still be in the same table, which can develop into a messy schema as your models grow apart in structure and behaviour.

There is nothing wrong with jumping to something else AFTER that happens, but you could spend a little more time designing upfront to avoid it.

It comes down to do you want a messy database or a messy object model? Are a few redundant columns that bad? For example, having teacher's salary and student's allowance in the same table? The alternative is separating the table, but then you have duplicate columns on each table such as first/last name, address.

As you talk it through like this you begin to understand that it probably IS worth having a few potentially redundant columns in the table just to avoid those duplicate columns on multiple tables. Having said that, DB design is not my day-job, so keep looking around and see what works for you!

---

Aside from the design, let's talk a little about the code itself.

 def get_student_classes
 student_profile = profiles.select { |p| p.profile_type == 'student' }.first
 return [] if student_profile.nil?
 return klasses.select { |k| k.profile_id == student_profile.id }
 end

First problem is you have named your method get_student_classes. In Ruby we would simply call this student_classes. Forget getter and setter naming conventions. You get by calling the method itself, you set by assigning (student_classes = etc).

Second problem above is that you've used select. This will fetch all records from the database and then loop over them. Sure, it doesn't matter when there are only 10 or so records, but it becomes much less efficient as the data piles up. To prevent that, use a query. On top of that, use a scope to build that query.

The third problem is that you are performing two queries to get what you want (klasses.select).

What you want to do is run a single query to get the classes you want without the need to loop over and filter them in Ruby.

Hate to hit you with an anti-climax but I've run out of steam tonight :) I would advise you perhaps post it on SO. There is surely a better way of querying for that data.

• robskrob: Hey Damien, Thank you so much for your feedback. Regarding your comment about, get_student_classes, I believe only one DB query is used and that's self.klasses, which will query all of the user's classes. select is just ruby and does not fire off any additional queries.
• Damien: Hi @robertjewell - I was actually talking about using profiles.select to get the profile id you are using in your klasses.select block. You should be able to join profiles to klasses somehow and perform a single query.
• robskrob: Oh I see, what you mean about two queries. I guess I am struggling to understand how you propose getting a user's classes that he's teaching vs that he's learning. I've proposed the above modeling solution because the last thing I want to have are three different models for one user: Student, Teacher and Admin. It's only one user and so the backend modeling should not permit someone to create three different users when in reality it's one user with three different profiles.