Skip to content

Add crypto-square exercise#5

Open
ebengt wants to merge 15 commits intolehoff:masterfrom
ebengt:crypto-square
Open

Add crypto-square exercise#5
ebengt wants to merge 15 commits intolehoff:masterfrom
ebengt:crypto-square

Conversation

@ebengt
Copy link

@ebengt ebengt commented Jun 27, 2014

No description provided.

kytrinyx and others added 15 commits June 1, 2014 19:45
* readme now explains the structure of the directories.
* config.json now lists the exercises in an order similar to Elixir.
Also ignore emacs ~ files.
Using escript to run tests is faster (1 second instead of 30).
To integreate better with eunit, tests are now suffxed with _tests.erl.
word_count needed a change to compile.
bank_account and binary_string needed renaming to fit the pattern.
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's hide all the number manipulation in the width function:

width(Len) ->
  Sqrt = trunc(math:sqrt(Len)),
  width1( Sqrt, Len).

width1( Sqrt, Len) when Sqrt*Sqrt < Len ->
  Sqrt + 1;
width1( Sqrt, _) ->
  Sqrt.

This makes it easier to read the flow of the code.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, you are right. The square root is better off hidden in width/1.
I still hold out for my variable names, though. I think Length is better than Len.

How come you are comparing "Sqrt*Sqrt < Len", instead of my "Square_root > Trunc"?
Will the latter give wrong answers?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your variable names are the best.

The two comparisons should do the same. I went for S*S<L because it seemed more natural in my head to focus on making sure that the square area covers the entire length. Either way is fine by me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants