Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Use interface for RowKeyDistributors #12

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
4 participants
Contributor

snopoke commented May 28, 2012

No description provided.

Collaborator

abaranau commented Jun 9, 2012

Patch looks good. Before I commit it, could you please add some reasoning for adding interface in description of the task?

The idea with the abstract class on the top of the hierarchy was to preserve ability to extend it (add more methods, etc.) without breaking existing code. With interface, if one implements it, there's no way to change it without breaking. This is especially important when usage of the lib is growing & lib is still changing a lot.

Collaborator

abaranau commented Jun 9, 2012

btw, +1 for moving addInfo() method

Owner

otisg commented Jun 11, 2012

+1 for abstract class reasoning, so I, too, would be curious to know why the interface.

Contributor

snopoke commented Jun 11, 2012

I'm not sure I agree with your argument for the abstract class. If you remove or change methods you do break existing code.

Classes that implement the interface are still free to extend the abstract class if they wish but aren't forced to.

The reason for using an interface is twofold:

  1. Separate the implementation from the contract
  2. Create a more explicit contract

Having looked at this again I might actually remove getAllDistributedKeys and getDistributedIntervals from the interface since they are internal methods.

I'm not stuck on the idea of an interface. I just thought it would be a good idea.

andre77 commented Jul 3, 2012

+1 for interface
the reasons wre given by snopoke
you can still have an abstract class as a parent of your whole class hierarchy to implement some stuff

@snopoke snopoke closed this May 27, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment