Skip to content

Provide a mechanism to create a new delta from an existing delta without caring about its type #75

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

Closed
cowwoc opened this issue Apr 10, 2020 · 6 comments

Comments

@cowwoc
Copy link
Contributor

cowwoc commented Apr 10, 2020

I am writing a method that needs to break a delta into pieces (e.g. two deltas that result in the same patch as the original delta). The current design is problematic because:

  1. I don't want my method to know/care what the delta type is.
  2. The AbstractDelta class is abstract, so I cannot simply invoke its constructor.
  3. The AbstractDelta subtypes do not provide with() methods.
  4. Some of your code (e.g. DiffRowGenerator) check the class type instance (e.g. if (delta instanceof InsertDelta) instead of using AbstractDelta.getType() so I cannot subclass AbstractDelta myself.

Probably the easiest way to resolve this is to add an abstract withChunks(Chunk<T> original, Chunk<T> revised) method to AbstractDelta.

@cowwoc cowwoc changed the title Provide a mechanism to create a new delta from an existing delta Provide a mechanism to create a new delta from an existing delta without carrying about its type Apr 10, 2020
@cowwoc cowwoc changed the title Provide a mechanism to create a new delta from an existing delta without carrying about its type Provide a mechanism to create a new delta from an existing delta without caring about its type Apr 10, 2020
@wumpz wumpz pinned this issue Apr 10, 2020
@wumpz
Copy link
Collaborator

wumpz commented Apr 18, 2020

I agree, that different implementations of Deltas could be useful. I will replace all instance checks with getType checks.

@wumpz
Copy link
Collaborator

wumpz commented Apr 18, 2020

So I replaced all instanceof checks with its corresponding getType checks.

I not happy with an withChunks method. A delta in itself should be final. Do you need the specific apply / restore functionality with your new Delta? If not and with this getType change you could simply build your own DeltaClass that could hold all data with its corresponding type.

@wumpz wumpz unpinned this issue Apr 18, 2020
wumpz added a commit that referenced this issue Apr 18, 2020
@cowwoc
Copy link
Contributor Author

cowwoc commented Apr 19, 2020

@wumpz I think you misunderstood what I meant by withChunks. The delta is final as you said. Invoking withChunks returns an new delta having the same type of the original, but having different source, target chunks. Are you okay with that?

@cowwoc
Copy link
Contributor Author

cowwoc commented Apr 22, 2020

@wumpz Great! Let me know when the SNAPSHOT is ready for testing.

@cowwoc
Copy link
Contributor Author

cowwoc commented Apr 22, 2020

@wumpz I just noticed the SNAPSHOT has been updated. I tested this new feature and it works great. Thank you! Looking forward to an official (non-SNAPSHOT) release.

@wumpz
Copy link
Collaborator

wumpz commented Apr 23, 2020

The release Issue is already there (#78).

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

No branches or pull requests

2 participants