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

Document query-related interfaces #326

Merged
merged 4 commits into from Dec 3, 2016

Conversation

JordanMartinez
Copy link
Contributor

I believe PointQuery should extend BoundsQuery (since that provides the initial query method) and should better clarifies what its query method method does. There is no PointQueryImpl because BoundsQueryImpl is the actual implementation used for the PointQuery interface.

Why then does the BoundsQuery interface exist? I have no idea. Perhaps to prevent any changes to the implementation's position or offset values.

@JordanMartinez
Copy link
Contributor Author

Note: I did not write javadoc for PointQuery's query method due to the above concern/issue.

@JordanMartinez
Copy link
Contributor Author

I'm going to look into the locator package to see if that might give more insight as to the various ways that PointQuery can work.

@brcolow
Copy link
Collaborator

brcolow commented Dec 3, 2016

To be honest I am not a fan of the interface Thing, class ThingImpl implements Thing pattern. Ideally I think interfaces should almost always be adjectives (think Comparable, Sortable, Identifiable, etc.). That is, we would have a Queryable<T> interface in this case. However, I do not think tearing apart the working base we have would be fruitful (not saying you are suggesting to do that, but to enforce the adjectives idea would take a lot of re-writing).

In any case, can you show me with code what your suggestion is for the inheritance of PointQuery and BoundsQuery?

@JordanMartinez
Copy link
Contributor Author

How can PointQuery extend BoundsQuery though? BoundsQuery's query method returns Bounds and PointQuery's query returns Point2D. What am I missing?

Ah! Haha... I didn't realize they had 2 different return types 😄

@JordanMartinez
Copy link
Contributor Author

Why doesn't BoundsQuery have a @FunctionalInterface annotation? That's essentially all it is.

@JordanMartinez
Copy link
Contributor Author

interface Thing, class ThingImpl implements Thing pattern.

Neither am I. I think I read somewhere that this is an anti-pattern, but I can't recall all that it said.

@brcolow
Copy link
Collaborator

brcolow commented Dec 3, 2016

The @FunationalInterface annotation is optional, but it is nice to use. I saw throw it on 😄 ! Going to bed for the night.

@brcolow brcolow merged commit 0dc231b into TestFX:master Dec 3, 2016
@brcolow
Copy link
Collaborator

brcolow commented Dec 3, 2016

Merged. Thanks a lot.

@JordanMartinez JordanMartinez deleted the documentQuery branch December 3, 2016 21:23
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.

None yet

2 participants