Thank you for the feedback, @Asd. I’ve attempted to answer all of your questions below.
Why do you override the def, defp and @ functions? It makes this library inapplicable with other libraries (like decorators, for example). More traditional approach is to use @on_definition callback to collect data for each function and then @before_compile to replace implementations of the functions. This way it is compatible with other solutions.
As mentioned in the history section of the Bond docs the implementation of Bond is based on an older contracts library, ex_contract, from Dariusz Gawdzik. I carried over some of the code from ex_contract more or less directly.
However, you make a good point about compatibility with other libraries, so I will revisit the implementation and try to use @on_definition and @before_compile instead of overriding various Kernel macros.
Why do you use gen_statem? Module is always compiled in a single process.
I chose to use a finite state machine for keeping track of compile state. I chose gen_statem simply because it is already available with Erlang and I didn’t have to pull in an extra dependency for an FSM.
Bond.FunctionWithContract.function_id will fail when context atom in variable is not nil (for example code was generated by macro)
I will look into this and commit a fix.
It tries to insert assertions to every clause, which results in code failing to compile with cryptic error when assertion references a variable which is present only in one clause. I think that this is a problem with the design of the solution, not with the implementation. Is it expected?
This is an intentional design decision: to have preconditions and postconditions defined before the first clause of a function, and apply those preconditions and postconditions to every clause of that function.
Not everyone agrees with that design decision, though, as can be witnessed in the discussion on my unmerged pull request for ex_contract.
My opinion, informed by Bertrand Meyer’s writing on Design by Contract, and especially Object-Oriented Software Construction, is that contracts are part of the public interface for a module and its functions, and not simply an implementation aid to the developer of a module. To me this means that contracts should be declarative and specify the observable aspects of a function.
In the case of multi-clause functions, it is an implementation detail to use different names for parameters in the different clauses of the function, and that implementation detail is not even visible to callers of the function unless they look directly at the source code. I.e., it’s not in the docs and not part of the public interface for the function.
That said, I do think I could make improvements to Bond to make it easier to define contracts for multi-clause functions. I’m thinking of adding some sort of bind clause to assertions in Bond, which would allow for pattern matching on arguments and binding arguments to arbitrary names. (Incidentally, this might also help with defining invariants at the module level by providing a mechanism for invariants to reference function parameters and/or pattern match on function results, as discussed with @zachallaun above.)
In the meantime, the best approach is just to use consistent names for parameters in all clauses of a multi-clause function. If the generic name for a parameter is not sufficient in the context of the individual function clause, then an alias could be used. Something like the following, perhaps (untested):
@spec get(URI.t() | String.t()) :: term()
@pre uri_string_or_struct: is_binary(uri) or is_struct(uri, URI),
uri_string_is_parseable: is_binary(uri) ~> URI.parse(uri)
def get(%URI{} = uri) do
http_request(uri)
end
def get(uri = url_string) when is_binary(url_string) do
get(URI.parse(url_string)
end
Note the alias of the uri parameter as url_string in the second clause of the get/1 function, which allows the generic name uri to be used in the contract but still allows for a more meaningful name for that specific function clause.
Predicates work not only with booleans, but their spec states otherwise. Why?
This was simply an oversight on my part. I’ve updated the specs to look like this instead:
@spec xor(as_boolean(term()), as_boolean(term())) :: boolean()
@spec implies?(as_boolean(term()), as_boolean(term())) :: boolean()
This will be in the next release of Bond.
Its basically a runtime test. a contrived example because result in this case will be {:ok, result}
