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

Adding an OOP analogue for StringUtils.splitPreserveAllTokens ISSUE #1722 #1743

Merged
merged 8 commits into from
Nov 30, 2024

Conversation

tjann7
Copy link
Contributor

@tjann7 tjann7 commented Nov 1, 2024

Rationale of some decisions made:

  1. Functional Interface allows use of custom String splitter function avoiding the static method calls
  2. A new class created as modifying already existing Split class would not be shorter : if we add some checking for flags would create just as many constructors as with 2 distinct classes
  3. Tests only compare lengths of obtained Collections

@tjann7 tjann7 changed the title Add an OOP analogue of StringUtils.splitPreserveAllTokens. ISSUE #1722 Try correcting maven checkstype violations Nov 1, 2024
@tjann7 tjann7 changed the title Try correcting maven checkstype violations Adding an OOP analogue for StringUtils.splitPreserveAllTokens #1722 Nov 1, 2024
@tjann7 tjann7 changed the title Adding an OOP analogue for StringUtils.splitPreserveAllTokens #1722 Adding an OOP analogue for StringUtils.splitPreserveAllTokens ISSUE #1722 Nov 1, 2024
tjann and others added 2 commits November 1, 2024 17:18
@tjann7
Copy link
Contributor Author

tjann7 commented Nov 1, 2024

@yegor256 @pnatashap do these commits satisfy requirements #1722?
Apologies for unnecessary dirt in pull request.

import org.cactoos.list.ListOf;

/**
* Used for avoiding static method calls.
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tjann7 maybe you can provide a more detailed explanation here, with an example? Currently, it's hard to understand what this class is for, exactly.

@tjann7
Copy link
Contributor Author

tjann7 commented Nov 2, 2024

@yegor256 please review
Also, is it okay to tag people always asking for a review or you will reach eventually on your own?

@yegor256
Copy link
Owner

@rultor merge

@rultor
Copy link
Collaborator

rultor commented Nov 30, 2024

@rultor merge

@yegor256 OK, I'll try to merge now. You can check the progress of the merge here.

@rultor rultor merged commit 51e76ef into yegor256:master Nov 30, 2024
9 checks passed
@rultor
Copy link
Collaborator

rultor commented Nov 30, 2024

@rultor merge

@yegor256 Done! FYI, the full log is here (took me 7min).

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.

3 participants