Skip to content

Added Unchecked Math Operations#426

Merged
heyLu merged 1 commit intopixie-lang:masterfrom
reutermj:master
Nov 6, 2015
Merged

Added Unchecked Math Operations#426
heyLu merged 1 commit intopixie-lang:masterfrom
reutermj:master

Conversation

@reutermj
Copy link
Copy Markdown
Contributor

@reutermj reutermj commented Nov 2, 2015

Related to issue #406. Added operations u+, u-, and u* for Integer, Float, BigInteger, and Ratio that follows the semantics of the checked operations except for cases of Integer and Integer. In these cases, the operations do not promote to BigInteger, but instead wrap like the equivalent C/C++ operations.

I also defined tests that mirror the tests for the checked operations, as well as added a test demonstrating the wrapping behaviour.

@heyLu
Copy link
Copy Markdown
Member

heyLu commented Nov 5, 2015

I'm a bit unsure about this. Here are a few comments:

  • I think the names should mirror the ones in Clojure, i.e. unchecked-add, ...
  • we may want to add them to a separate IUncheckedMath protocol
  • I think the operations should only work on integers and floats, not ratios

I may be wrong on these, feel free to tell me if that's the case!

@reutermj
Copy link
Copy Markdown
Contributor Author

reutermj commented Nov 5, 2015

I've made changes with respect to all of your comments. To be completely honest I only knew about clojure's unchecked-math and didn't know of the specific functions. I also added unchecked-inc and unchecked-dec to reflect the api.

@thomasmulvaney
Copy link
Copy Markdown
Member

Ace. If you can squash the commits I'll merge this shortly. Thanks!

@reutermj
Copy link
Copy Markdown
Contributor Author

reutermj commented Nov 5, 2015

Done. Let me know if you have any problems with it.

@heyLu
Copy link
Copy Markdown
Member

heyLu commented Nov 6, 2015

Thanks! Can you rebase this onto current master, so that the other commits disappear?

Added Tests for Unchecked Math Operations

Adjustments to unchecked math

Made changes to the implementation of unchecked math in regards to: implementing them under their own protocol, mirrored the api of clojure, reduced implementations of the protocol to just integer and float

Fixed errors in the tests that caused the build to fail
@reutermj
Copy link
Copy Markdown
Contributor Author

reutermj commented Nov 6, 2015

Sorry I thought I did that; git is still magic to me. It should be done now.

@heyLu
Copy link
Copy Markdown
Member

heyLu commented Nov 6, 2015

No worries, git tends to be confusing. :)

Thanks a lot for the pull request, will merge when the tests have run.

heyLu added a commit that referenced this pull request Nov 6, 2015
Added Unchecked Math Operations
@heyLu heyLu merged commit 4c9bf17 into pixie-lang:master Nov 6, 2015
@heyLu
Copy link
Copy Markdown
Member

heyLu commented Nov 6, 2015

✨ :)

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