Rounding x, y, and angle to 1 decimal place

[austinschwartz]

Fixing issue #28 by updating the pr-str-turtle function. Using reader conditionals to work in Clojure and ClojureScript. I don't have much experience in this language, so I'm not sure how spacing is supposed to work, or if theres a better of doing any of this. However, Logo has a special place in my heart as my first language so I'd love to contribute anything I can :)

Clojure:

#object[clojure.lang.Atom 0x5eb3e420 {:status :ready, :val ([:x "-20.8"] [:y "381.0"] [:angle "93.7"] [:pen true] [:color [0 0 0]] [:fill false])}]

ClojureScript:

#object [cljs.core.Atom {:val ([:x "-8291.5"] [:y "129048.5"] [:angle "93.7"] [:pen true] [:color [0 0 0]] [:fill false])}]

[googlebot]

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

:memo: Please visit https://cla.developers.google.com/ to sign.

Once you've signed, please reply here (e.g. I signed it!) and we'll verify. Thanks.

---

• If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
• If your company signed a CLA, they designated a Point of Contact who decides which employees are authorized to participate. You may need to contact the Point of Contact for your company and ask to be added to the group of authorized contributors. If you don't know who your Point of Contact is, direct the project maintainer to go/cla#troubleshoot.
• In order to pass this check, please resolve this problem and have the pull request author add another comment and the bot will run again.

[austinschwartz]

I signed it!

[googlebot]

CLAs look good, thanks!

[echeran]

The Clojure code formatting looks good! And good job with using format.

The only thing that I would say is use merge instead of concat, and make the (select-keys...) form the first argument to merge. That keeps the return value of the expression as a map, so the output will look like #object[clojure.lang.Atom 0x5eb3e420 {:status :ready, :val {:x "-20.8", :y "381.0", :angle "93.7", :pen true, :color [0 0 0], :fill false}}] instead of #object[clojure.lang.Atom 0x5eb3e420 {:status :ready, :val ([:x "-20.8"] [:y "381.0"] [:angle "93.7"] [:pen true] [:color [0 0 0]] [:fill false])}]

(Reasons being: concat is part of the core seq library, so it converts data structure collections into their seq-ified form before operating. A seq-ified map is a seq of the entries as 2-elem vectors. Also, even though putting the (select-keys...) form first in the call to merge isn't necessary given your change to the key vector arg for select-keys, it still may be good to keep the "base" map as the first arg to merge given the order in which merge works.)

Can you make that change and update the PR?

[austinschwartz]

Sure thing! I'll take a look at it later tonight. Thanks for the comments!

[austinschwartz]

I didn't realize that concat was turning it into a vector! Thanks for the help :) I tested it again, and I think it should be good.

Clojure:

#object[clojure.lang.Atom 0x9c3475 {:status :ready, :val {:pen true, :color [0 0 0], :fill false, :x "-540.8", :y "12168.9", :angle "92.6"}}]

ClojureScript:

#object [cljs.core.Atom {:val {:pen true, :color [0 0 0], :fill false, :x "-226.0", :y "283.3", :angle "347.8"}}]

[echeran]

There's one last finishing touch. I should have noticed this earlier, but it's small. format returns a string, and because we're using pr/prn/pr-str/prn-str, we see the quotation marks get printed. (In contrast, there's no difference with print/println for 3.1 and "3.1").

Given a floating point x, what do you think about replacing the format code with: (float (/ (int (* x 100)) 100)) ?

My rationale of tradeoffs: Although I don't want to encourage "magic numbers" and not reusing prior impls from somewhere else, It doesn't seem worth taking on extra deps and the precision is unlikely to change. The above code works both in Java and JS, even if JS has a built-in way to do this. Even though the relevant floating point spec IEEE 754 would only store binary fractions perfectly, I think CLJ/CLJS are clever enough to print the truncated decimal fraction most if not all of the time.

[austinschwartz]

With that int cast, you won't be able to have arbitrarily long x/y values, which previously worked. Do you want to be able to support this? I don't think its super important, but it should be documented if it isn't supported.

With format:

user=> (back 89178913189231231)
#object[clojure.lang.Atom 0x5ff66a87 {:status :ready, :val {:pen true, :color [0 0 0], :fill false, :x "7796268508.7", :y "-178357826378462208.0", :angle "90.0"}}]

With int cast to truncate

user=> (back 191212112)
IllegalArgumentException Value out of range for int: -17396781800  clojure.lang.RT.intCast (RT.java:1191)

However, using your code with bigint seems to work. Also, do you want 1 decimal place, or two?

[echeran]

Oh, interesting. Yeah, it's good to support arbitrary values if possible rather than documenting exceptions. And yeah, Clojure uses longs and doubles by default for literals (I think), but doubles have a wider range than longs. So yes, bigint sounds good for CLJ. If that's not supported in CLJS, then one of these solutions ought to work. (And yes, big numbers when measuring in pixels probably won't come up in practice.)

One decimal place is probably enough. The point is just to make a cosmetic presentation that's user-friendly (less jarring) for beginners. The actual turtle value is unchanged, as one would see from (str @clojure-turtle.core/turtle).

[austinschwartz]

Updated to use round now on CLJS. One strange difference now, is that if the value has nothing after the decimal, it'll print it as an integer without the decimal.

Clojure:

user=> (back 1231231231.23232)
#object[clojure.lang.Atom 0x17e842f8 {:status :ready, :val {:pen true, :color [0 0 0], :x 53.8, :y -1.23123123E9, :angle 90.0}}]

ClojureScript:

#object [cljs.core.Atom {:val {:pen true, :color [0 0 0], :x 0, :y -123456789, :angle 91.2}}]
clojure-turtle.core=> (forward 1.23)
#object [cljs.core.Atom {:val {:pen true, :color [0 0 0], :x 0, :y -123456787.8, :angle 91.2}}]

[echeran]

The change looks great. Thanks for all the work! There's something about just 1 decimal place that reminds me of the old Logo system I used way back when.

Side note on JS numbers - there's apparently only one number type - double-length floating point. Numbers get printed like this as if integral types exist when they actually don't, leading to these inconsistencies. JSON is a big practical improvement over XML, but it inherits these JS quirks by definition. People probably transmit data with numerical ids in JSON often, so you can guess the tradeoff that was made. That's one of the ways EDN cleans up JSON's shortcomings.

[austinschwartz]

No problem! I'm glad to contribute! I might take a look through some of the other issues to see what else I can help with.