Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve Racket code #21

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Improve Racket code #21

wants to merge 6 commits into from

Conversation

elibarzilay
Copy link

Roughly same speed, but much better looking code. A bunch of smaller commits with explanations.

The thing that tends to trip people is that the `integer?` predicate is
true for both exact integers (like 123) *and* inexact ones (like 123.0),
while Typed Racket's `Integer` corresponds only with the exact ones.  So
usually the easy way out of this is to use `exact-integer?` instead.

As a side note, "The `if n` is like a form of pattern matching" --
that's not really true.  There's no pattern matching happenning.
Instead, once you use a predicate for a specific type with something in
an `if`, Typed Racket knows that in the then-branch, something has that
type.  In the `if n` case, that means that it knows that it cannot be
`#f`, and if outside the `if` it was `(U #f Whatever)` then in the then
branch it is left as `Whatever`.  This is called "occurrence typing".
* Use a plain `for` loop over the input instead of an indirect counter

* Use `(cons x y)` to add things on the left side instead of
  `(append l (list x))` (if you really need to preserve the order, then
  it's better to `reverse` the lists when you're done reading)

* Using plain `'` quote is generally preferred when you don't need the
  extra features of quasi-quotes

* `str->int` is used only in `read-places`, so put it there
…lobal vector.

Makes it three times slower, but much more readable code.
(Functional in the proper sense, not in the we-don't-mutate sense.)
Makes it roughly as fast as the global-vector version.
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.

1 participant