ydz (3) [Avatar] Offline
#1
Hello,

I'm working through this excellent book and have been thoroughly enjoying it so far. I've been following along implementing my version of the code as the author introduces it and I've hit a snag in chapter 7, section 7.3.2, in particular when applying the change in listing 7.9.

defmodule Todo.Server do
  ...
  def handle_cast({:add_entry, new_entry}, {name, todo_list}) do
    new_list = Todo.List.add_entry(todo_list, new_entry)
    Todo.Database.store(name, new_list)
    {:noreply, {name, new_list}}
  end
  ...
end



In this snippet Saša introduces a modification to the code to read persisted Todo.Lists using the database from the Todo.Server, however up until that moment Todo.Database was started and managed by the Todo.Cache as described in listing 7.8:

defmodule Todo.Cache do
  ...

  def init(_) do
    Todo.Database.start()
    {:ok, %{}}
  end

  ...
end


The change does work since the entry point to the system is the Todo.Cache, but I find this confusing since now the Server is now coupled with an initialized Database and cannot be tested on its own.

I realize this is not a huge problem but it did confused me when reading the rest of the chapter from that point on, particularly when looking at figure 7.2 which includes "Starts" link from the Cache to the Server but not to the Database.

I wonder what would be the appropriate way of solving this. My first instinct was to have the Todo.Servers attempt to initialize the Database and that surprisingly works even though I expected it to throw an error since we'd be attempting to reregister the Database server process under the same name.

Again, I realize it's not a big problem but I thought I'd share my experience.

Thanks again for an excellent book.

Yeray
ydz (3) [Avatar] Offline
#2
I forgot to mention I'm reading the 2nd edition MEAP V03

Best,
Yeray
sjuric (106) [Avatar] Offline
#3
Hi,

I agree that from the testing standpoint it's somewhat unfortunate that the server is coupled to the initialized database. As it's mentioned in the note at the end of 7.2.2, "It’s worth noting that the example projects in this book aren’t test-driven or particularly well tested." and "some improvisations have been used to ensure basic correctness".

In this case, I'm removing the persist folder before the tests start, to make sure that tests start with an empty database. This is still not perfect, as we could have multiple tests using the same entities, and so one test might negatively affect the outcome of another.

In a real project, you'd probably use an external database, such as PostgreSQL. If you work with such database via the Ecto library, that library provides the support for sandboxed testing, where each test runs in its own transaction which is rolled back at the end, so db changes are only visible inside the test, and can't affect the outcome of other tests (which can even run concurrently).

If you don't use Ecto, then you'd either need to implement some manual sandboxing yourself, or make sure that different tests work on different entities. The former approach might be quite tricky, while the latter one is fairly trivial, and I've used it myself on different occasions.


when looking at figure 7.2 which includes "Starts" link from the Cache to the Server but not to the Database


Yeah, the fact that the database is started from the cache is a quick crutch to keep things simple at this point. It's even mentioned in the book: "To start the server, you’ll plug into the Todo.Cache.init/1 function. This is a quick hack, but it’s sufficient for the moment." Later on, in chapter 9, the code is reworked so the database is started outside of the cache.


My first instinct was to have the Todo.Servers attempt to initialize the Database and that surprisingly works even though I expected it to throw an error since we'd be attempting to reregister the Database server process under the same name.


When you try to start another instance of the process which tries to register under a name which is already taken, the start function won't throw, but it will return an error in the shape of {:error, {:already_started, pid_of_the_registered_process}}. This will be explained (and used) in chapter 9.

Therefore, the fact that you're starting the database from the server doesn't really improve anything here, but it also doesn't crash anything. Instead, you'd probably want to empty the database before each test, and make sure to use a different cache instance in each test.


I forgot to mention I'm reading the 2nd edition MEAP V03


It would probably be better if this post was included in the forum for the 2nd. edition, though your questions apply to the 1st edition as well.
ydz (3) [Avatar] Offline
#4
Hi Saša, thanks for your quick reply.

I understand the testability of the code is second, I'm happy to know this addressed in later chapters, looking forward to it.

I was also cleaning up the persist folder in my tests, albeit through ExUnit callbacks. I didn't know you could do it in the test helper, I try not to look at your repo and implement my own solutions but I definitely missed that.

I'm looking forward to start using libraries like Ecto, but I like you bare bones approach first.

I didn't realize there was 2nd edition forum, maybe an admin can move the thread there.

Thanks,
Yeray
sjuric (106) [Avatar] Offline
#5
ydz wrote:
I was also cleaning up the persist folder in my tests, albeit through ExUnit callbacks. I didn't know you could do it in the test helper, I try not to look at your repo and implement my own solutions but I definitely missed that.


Using ExUnit callbacks for that is usually a better approach as it is more obvious, and less global (because callback only apply in the test case where they are specified). Here I opted for the helper script to make sure that the database is empty before all tests. But normally, I first try with callbacks, and only resort to modifying the test helper if I want to execute something before all the tests.