Proper structuring of testable code in context

[csadewa]

@josefrichter

So my questions are

1. should I split my function so that it does the checks and then calls the plain vanilla create function?
2. should I test these two separately? and does it make sense to test each of them?
3. do I need to rename the second one to insert_enrollment? because they have the same arguments, co calling create_enrollment in the first one would basically result in recursive call and infinite loop, right?

hmmm, your question is more of code structuring, and there’s room for opinion & preference on it, so i would answer just like how i would prefer to do it:

1. Yes, i generally like to split module which deals with low level data (Enrollment, Classes) with module that govern business logic. in this case you mix actual DB creation logic of enrollment with business logic,
2. yes, you should test the business logic code part, but testing insert_enrollment is a bit dubious (for me anyway) as it’s really standard code from ecto, i usually trust ecto to do it’s job.
3. if you do split the module, having create_enrollment would be better as it’s more standard.

just for example, here’s how business logic code might be:

defmodule Enrol.EnrolService do
  
  def enroll_student(student, class) do
    Repo.transaction(fn ->
      if Classes.class_have_existing_capacity?(class) do
        Enrollment.create_enrollment(attrs)
      else
        Repo.rollback(:class_full)
      end
  end)
end