Designing Elixir Systems with OTP (Pragprog)

To be honest I’m not really asking for this, as I said I’ve not even completed the book and found It has added a lot of value day to day, so I am certainly a happy customer anyway. It was only something that popped into my head when reading the thread and might not even be a good idea. Personally I think the book did the right thing in order to keep focus on the topics at hand in terms of the main content. I’m somewhat inclined to think that types and dialyzer would be best served overall elsewhere, as far as I am aware there seems to be a bit of a lack of a easy to digest tutorial text for people just moving beyond the elixir basics who are looking to add static analysis which might be part of why it has been brought up?

1 Like

It could be that, sure.

Sadly, Dializer is by no means a home run of static analysis wins. It’s slow and cryptic, which makes it a hassle to work with. The information it gives is helpful, but it definitely makes you work for it.

5 Likes

True. But I still think that having some form of analysis is better than having none. Or perhaps you disagree with me?

I have the impression from the book that you use Dialyzer in your daily work - please correct me if I am wrong, I would love to know why you don’t use it (if you don’t) or what alternatives you think are better.


I have a few more questions regarding the book (please keep in mind I ask as I read, I have not yet finished the whole thing).

On page 109 we create our first GenServer and we add the following callback to it:

  def init(quizzes) when is_map(quizzes), do: {:ok, quizzes}
  def init(_quizzes), do: {:error, "quizzes must be a map"}

This translates to the following spec:

@spec init(any) :: {:error, String.t} | {:ok, map}

Which according to the init specification (and dialyzer) is incorrect:

The return type 
          {'error', binary()} in the specification of init/1 is not a subtype of 
          'ignore' |
          {'ok', _} |
          {'stop', _} |
          {'ok', _,
           'hibernate' | 'infinity' |
           non_neg_integer() |
           {'continue', _}}, which is the expected return type for the callback of the 'Elixir.GenServer' behaviour

I know I am likely missing something here. But perhaps it should be def init(_quizzes), do: {:stop, "quizzes must be a map"} instead ?

Another thing, is that I noticed you are using quiz.title as key for the quizzes map. Given that we will have many quizzes at the same time, won’t this be dangerous because it will allow key clashes?

What would the be the downsides of using reference() as an id for a quiz when creating one?

I tend to use it when building libraries, to provide even more assurance that I’m building something robust.

I tend not to bother when I’m building applications, because of the previously mentioned hassle. This is total laziness and my part and it’s bad. Still, it’s kind of telling that the tool pushes me to this.

You are 100% correct. Great catch!

It’s a fair point. Some tradeoffs are definitely made to make the book code easier to follow.

You could use references, as you suggest, but then you need to give user code some means of finding the reference they are currently interested in.

2 Likes

I am glad I am somehow contributing to the great work you guys have been doing. Even though my approach may not be the best sometimes, that is always my end goal ! :smiley:

That is correct. PragDave introduced a concept in one of his courses called a Tally. A tally is a special representation of the state we give to the user. In this case, the state would be the quizzes map, and a tally could be a part of the map plus some extra information the user needs. This is a pattern used in FP I was not aware of before trying his courses, but I have had difficulty applying it in other places.

  1. Have you ever heard of giving tally’s to the client?
  2. If so, what are your thoughts on this pattern? How would it (hypothetically) fit the mastery example (if at all)?

On page 111 you present the user with this code snippet:

def quiz_fields(), do:
  %{ mastery: 2, title: :simple_addition}

Here you create this function that helps us create a quiz by setting some fields. As you can see in this map, the title field is an atom, yet on page 49 you specify the quiz.title is of type String.t. In a discussion we had previously, you mentioned using Strings for scenarios where the input comes from users is better because we don’t fill up the atom’s table (and avoid a crash).

Am I missing something here? Could you please elaborate on your decision to use an atom for the title instead of a String ?


On page 118 you append errors to the list of errors instead of prepending. In the first chapters you specifically recommend prepending to lists rather than appending to them. Could you help me understand the benefits of taking a different approach here?

Sorry, but I’m not familiar with this definition of a Tally. I tried to Google it, but I don’t think I’m finding the right concept.

I think this is just a symptom of having two authors. I intended for the title to be a string for the reasons mentioned. My co-author wrote the code using atoms, which to be fair does seem just fine for the tests he uses them in. I guess we should say either is allowed.

It felt important to me to keep the errors in the order they are encountered and I really doubt we’re going to be dealing with large enough lists of errors that performance would suffer. I made a calculated exception here.

3 Likes

Thank you for your detailed replies!
It is truly inspiring to have someone answer all my questions. I know I can be a bit heavy sometimes, but I want you to know I really appreciate all the time you are spending in answering them!

I will keep on reading and for sure keep on posting!

1 Like

On page 122 you have this code snippet for a file called mastery.ex

def build_quiz(fields) do
  with :ok <- QuizValidator.errors(fields),
       :ok <- GenServer.call(QuizManager, {:build_quiz, fields}), 
  do:  :ok, else: (error-> error)
end

What caught my attention here is the call GenServer.call(QuizManager, {:build_quiz, fields}).

Why have the authors decided to make a GenServer.call instead of using the Public API QuizManager already offers, namely the build_quiz function previously defined?

Could someone help me understand the technical reasons behind this decision? (performance issues, usage issues, etc).


In page 122 ins the function add_template, since we are not validating the title, what would keep me from setting the title as a function or some other thing? Did the authors decided to not validate the title because such a validation was not needed?

def add_template(title, fields), do:
    with  :ok <- TemplateValidator.errors(fields),
          :ok <- GenServer.call(QuizManager, {:add_template, title, fields}),
    do: :ok

I think this is just a simple error on our part. Thanks for catching it.

Titles are really just for the creator. It’s the name you give a thing so you can refer back to it later. The system doesn’t really care what it is.

If this is the case I would like to point out that the add_template, select_question and answer_question functions of the mastery.ex file all have the same issue.


On a separate note, why doesn’t the public API of QuizManager have a function to start it?
Currently if you want to spin up a QuizManager you need the following code:

def start_quiz_manager, do:
    GenServer.start_link(QuizManager, %{}, name: QuizManager)

My question is, why not have a public function in the API for that as well?

def start_quiz_manager, do:
    QuizManager.start() #inside it calls GenServer.start_link(QuizManager, %{}, name: QuizManager)

If the purpose of the public API of QuizManager is to hide the gritty details and the low levels calls to GenServer, why wasn’t a start function also added for the public API?

Would love to have your opinion !

OK, I actually took the time to look this time and I was wrong about us being wrong. :slight_smile: (It’s hard for me to check exact pages, because my local book is in a different state than your beta book.)

We start off showing these bare calls and save the API wrapping for the next chapter so we can use it to address other concerns like dynamic supervision. Keep reading. We get there!

Same answer. It’s in the next chapter.

1 Like

@JEG2 Hi. First of all, thank you for the book. I learned a lot.

I have a question about data validation. In the book you are doing it in the boundary layer. What if my validator has a bug and and invalid data gets through to the core? Is it OK if my core raises an error then? Or better to build the core that never breaks?

Example: A question can only accept answers “yes” and “no”. Every time you answer a question, it gets added to the list of responses in the quiz. But if the user somehow manages to answer “maybe”, which is invalid and the validator doesn’t catch it, we have options to: don’t add response to the list and return quiz unchanged, crash, return an error tuple.

Error tuple is not good for composition, not doing anything and returning the quiz is probably not what the user expected, and raising sounds quite good to me.

I hope I described the problem. What are your thoughts?

It composes quite well with with and various other constructs. :slight_smile:

It really just depends on what ‘kind’ of error it should be though. Like should it be handleable first of all?

It’s a good question if it should be handleable. I guess sometimes yes, sometimes no. This is the code I wrote today:

def answer_question(pitch, question_id, answer) do
  response = Response.new(pitch, question_id, answer)
  
  add_response(pitch, response)
end
  
defp add_response(pitch, %Response{valid: false}), do: pitch
defp add_response(pitch, %Response{valid: true} = response) do
  %Pitch{pitch | responses: Map.put(pitch.responses, response.question_id, response)}
end

Here only a valid response gets added to the map. We were debating with a co-worker if raising an error for the invalid response is a better approach than silently returning the pitch untouched. His argument was that the validation in the boundary layer can be buggy/incomplete and invalid data can leak into the core. He would then prefer to know about it, instead of silently returning the pitch.

In our case with questions and answers it is actually better like it is, because there is no handling of errors for this use-case. But I can also see his point of view, that if this was a more critical use-case, then it would be better to deal with it somehow.

My preference would be to return a tagged tuple and not to swallow it. I would expect the frontend to turn it into a helpful notification for the user, even if that’s just “Something has gone wrong. We have been notified…”

I try to reserve raising errors for the cases where there’s no viable action left to take.

1 Like

But in the book you were never returning tuples from functional core, even when you were expecting user submitted data. Instead you validated in the boundary layer, but if it would somehow fail to validate correctly, then the core would have invalid state. Just trying to unterstand what strategies are better, not attacking your solution :slight_smile:

I think I’m missing something in this discussion. Why would validation code at the boundaries be buggy while code deep in the core won’t be?

We almost surely don’t want to validate in both places, right? The two sources of truth would eventually drift apart.

Finally, there’s no reason a functional core can’t return errors. There’s a section, in the Boundaries chapter no less, about the value of treating errors as data. Consider how Phoenix controllers interact with contexts, for example.

Anyway, I feel like I’m not really grasping your query. What am I missing?

1 Like

That’s a good point about not wanting to validate in both places. I guess we were trying too hard to protect the system. And core is like a last chance to save the state from getting invalid. Wrong way to think about it. Totally makes sense that the validations will drift apart.

Now after this discussion, I’m planning to remove this function:

defp add_response(pitch, %Response{valid: false}), do: pitch

Also don’t check in add_response if the response in valid or not, but just add it. And the Validator in the boundary will take care of checking if we should call add_response in the first place.

1 Like

On page 154, in the section describing concurrency:

Concurrency

In the simplest terms, concurrency means doing more than one thing at the same time. In our venacular, it’s a micro concern.

I believe you mean vernacular instead :smiley:

A few lines later:

A client request can farm out six different database

Don’t you mean fan out? I never heard the term farm out but in case this is not a typo, I would really like to learn more about it!

1 Like

It’s as colloquialism

Specifically:

to turn over for performance by another usually under contract
farm out a job

So it’s often used to mean “outsourcing work”.

1 Like