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

Use an interface instead of method reflection to call the API #523

Open
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
4 participants
@SquidDev
Copy link
Contributor

SquidDev commented Feb 24, 2018

This makes it easier to verify at compile time that API methods are valid. I also think it makes the API code substantially easier to read, but that may just be me.

Use an interface instead of method reflection to call the API
This makes it easier to verify at compile time that API methods are
valid.
@Lignum

Lignum approved these changes Feb 24, 2018

Copy link
Contributor

Lignum left a comment

I also think it makes the API code substantially easier to read, but that may just be me.

Not just you, this is beautiful. Looks good to me.

@theoriginalbit

This comment has been minimized.

Copy link

theoriginalbit commented Mar 25, 2018

I also think it makes the API code substantially easier to read, but that may just be me.

Definitely a much better way to do it! Though if ComputerCraft is using Java 8 then it could be made a little cleaner with Optionals.

@dmarcuse

This comment has been minimized.

Copy link
Contributor

dmarcuse commented Mar 25, 2018

Though if ComputerCraft is using Java 8 then it could be made a little cleaner with Optionals.

It could be made even cleaner on top of that using Guava's Suppliers.memoize (Minecraft/forge already has Guava as a dependency, right?)

e.g. write a supplier to get an optional containing the API impl, memoize it with Guava, and then just use apiSupplier.get().ifPresent(...)

SquidDev added a commit to SquidDev-CC/CC-Tweaked that referenced this pull request Dec 10, 2018

Use an interface instead of method reflection to call the API
This is originally based on dan200/ComputerCraft#523, but being more
liberal in how it breaks backwards compatibility. 1.14 and all that
after all.

There's several reasons for this change:
 - Make the API code cleaner and less error prone by removing
   reflection.
 - Try to make ComputerCraft.java less monolithic by moving
   functionality into separate module-specific classes.
 - Hopefully make the core class less Minecraft dependent, meaning
   emulators are a little less dependent on anything outside of /core.

SquidDev added a commit to SquidDev-CC/CC-Tweaked that referenced this pull request Dec 17, 2018

Use an interface instead of method reflection to call the API
This is originally based on dan200/ComputerCraft#523, but being more
liberal in how it breaks backwards compatibility. 1.14 and all that
after all.

There's several reasons for this change:
 - Make the API code cleaner and less error prone by removing
   reflection.
 - Try to make ComputerCraft.java less monolithic by moving
   functionality into separate module-specific classes.
 - Hopefully make the core class less Minecraft dependent, meaning
   emulators are a little less dependent on anything outside of /core.

SquidDev added a commit to SquidDev-CC/CC-Tweaked that referenced this pull request Dec 17, 2018

Use an interface instead of method reflection to call the API
This is originally based on dan200/ComputerCraft#523, but being more
liberal in how it breaks backwards compatibility. 1.14 and all that
after all.

There's several reasons for this change:
 - Make the API code cleaner and less error prone by removing
   reflection.
 - Try to make ComputerCraft.java less monolithic by moving
   functionality into separate module-specific classes.
 - Hopefully make the core class less Minecraft dependent, meaning
   emulators are a little less dependent on anything outside of /core.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.