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

added long index based access methods with default int cast to all ac… #159

Closed
wants to merge 1 commit into from

Conversation

axtimwalde
Copy link
Member

…cesses

This reduces code duplication for imglib/imglib2-unsafe#1 (comment)

@hanslovsky
Copy link
Member

I don't see any issues with this and I think this can be merged.

@hanslovsky
Copy link
Member

Actually, I thought about it a little more and I think this is not beneficial over an additional interface that extends the existing access interfaces with long index get and set methods and defaults the int index get and set methods to delegate to the long index get and set methods.

With this PR, one would need to implement 4 methods per access for long index accessing (including overwriting the default methods), whereas the interface hierarchy as it is right now in imglib2-unsafe only requires the implementaiton of 2 methods for that.

@axtimwalde
Copy link
Member Author

... plus the interfaces. We can count characters :). How about using int-based accesses in long-assuming code? Is that something that we want? Sorry for the premature pull request.

@ctrueden
Copy link
Member

Why not do things the other way, with the int-based methods being default and delegating to the long ones?

@hanslovsky
Copy link
Member

This seems like the natural choice to me as well. There is one problem with this, however: All existing accesses would need to be updated. That is why I would prefer to introduce new interfaces that extend the existing accesses and add the long index get and set methods and default the int to delegate to the long ones.

@tpietzsch
Copy link
Member

It doesn't look like this is supposed to be merged. Can it be closed?

@axtimwalde axtimwalde closed this Jun 2, 2017
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.

4 participants