4
\$\begingroup\$

I am working on a math library for an online game. I want to prevent as many errors as possible and I want to catch all errors as early as possible to simplify debugging in the long run. However, I feel like I drown in details. This is quite different from "let it crash" motto.

point has {number(), number()} type or better say {0, 0} is a point. I started with Dialyzer specs and guard statements which work as a poor man's assertions.

Here is my point class.

-module(point).
-author("nt").

-export([is_point/1, distance/2, translate/2, pointToMap/1]).

-type point() :: {number(), number()}.
-export_type([point/0]).

is_point({X, Y}) when is_number(X), is_number(Y) -> true;
is_point(_) -> false.

distance({X1, Y1} = A, {X2, Y2} = B) ->
  case is_point(A) of false -> error(badarg); _ -> ok end,
  case is_point(B) of false -> error(badarg); _ -> ok end,
  math:sqrt(math:pow((X2 - X1), 2) + math:pow(Y2 - Y1, 2)).

translate({X1, Y1} = A, {X2, Y2} = B) ->
  case is_point(A) of false -> error(badarg); _ -> ok end,
  case is_point(B) of false -> error(badarg); _ -> ok end,
  {X1 + X2, Y1 + Y2}.

pointToMap({X, Y} = A) ->
  case is_point(A) of false -> error(badarg); _ -> ok end,
  #{
    x => X,
    y => Y
  }.


%% Spec
-spec is_point(P :: point()) -> boolean().
-spec distance(A :: point(), B :: point()) -> float().
-spec translate(A :: point(), B :: point()) -> point().
-spec pointToMap(A :: point()) -> #{x:= number(), y := number()}.

{{0, 0}, {0, 0}} is a rect. rect "class" relies on point.

-module(rect).
-author("nt").

%% API
-export([is_rect/1, contains/2]).

-type rect() :: {point:point(), point:point()}.
-export_type([rect/0]).

is_rect({{OriginX, OriginY}, {W, H}}) when is_number(OriginX), is_number(OriginY), is_number(W), is_number(H) -> true;
is_rect(_) -> false.

-spec contains(Rect, Point) -> boolean() when Rect :: rect(), Point :: point:point().
contains({{OriginX, OriginY}, {W, H}} = R, {X, Y} = P) ->
  case is_rect(R) of false -> error(badarg); _ -> ok end,
  case point:is_point(P) of false -> error(badarg); _ -> ok end,
  ((OriginX < X) and (X < (OriginX + W))) and ((OriginY < Y) and (Y < (OriginY + H))).

Things are getting really massive and daunting. Given the fact there is almost no business logic I wonder how fast codebase will become a complete mess if I continue to check all the required preconditions in all functions.

Update:

I think about removing all the specs and "assertions" from the code. Functions like pointToMap will be also removed as I don't need to rely on concrete types anymore. Something like tupleToMap should be used instead.

point.erl:

-module(point).
-author("nt").

-export([distance/2, translate/2]).

distance({X1, Y1}, {X2, Y2}) ->
  math:sqrt(math:pow((X2 - X1), 2) + math:pow(Y2 - Y1, 2)).

translate({X1, Y1}, {X2, Y2}) ->
  {X1 + X2, Y1 + Y2}.

rect.erl

-module(rect).
-author("nt").

-export([contains/2]).

contains({{OriginX, OriginY}, {W, H}}, {X, Y}) ->
  ((OriginX < X) and (X < (OriginX + W))) and ((OriginY < Y) and (Y < (OriginY + H))).
\$\endgroup\$

1 Answer 1

2
\$\begingroup\$

I think the concerns about redundancy and verbosity are probably well placed.

andalso

The short circuiting logical AND is andalso. It can be used to simplify the code:

is_point({X, Y}) -> is_number(X) andalso is_number(Y).

Generally, the pattern:

// pseudo code
if f(x)
  return true
else
  return false

can be replaced with return f(x). Or in languages like Erlang where everything is an expression, f(x) alone is sufficient.

Atoms

In Erlang, it is idiomatic to use atoms as the first level of data validation:

is_point({point, X, Y}) -> is_number(X) andalso is_number(Y).

More importantly, it aids in readability and debugging. Atoms let points be distinguished from football scores:

1> point:is_point({point, 3, 2}).
true
2> point:is_point({football_score, 3, 2}).
** exception error: no function clause matching 
point:is_point({football_score,3,2})
(/erlang/point.erl line 4) 

Note that using atoms and good function names provides most of what we want to start debugging this type of problem. Football scores are not points and "letting it crash" provides better information than badarg.

Message passing

Labeling data with atoms lets us distinguish {rhombus, P1, P2} from {rectangle, P1, P2} and {circle, P1, P2}.

Try-catch

Taking error handling code out of point:is_point is consistent with the "unwritten code is bug free" heuristic. If crashes are problematic, the caller should wrap the call to point:is_point/1 in a try-catch-finally block. The caller can handle the exception at whatever granularity it needs. And we can avoid the close coupling that coordinated error handling requires whenever the defaults are good enough.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.