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

Tests for StringManipulationHelper #945

Merged

Conversation

richarda23
Copy link
Contributor

@richarda23 richarda23 commented Jul 23, 2021

  • identified and fixed a bug in equalsToIgnoreEndline if number of lines was different
  • identified equalsToXml as not really mplemented or documented and not used
  • re-implemented join() in simpler way using tests to validate unchanged behaviour and widened to accept a Collection rather than AbstractCollection
  • some updated javadoc
  • work towards Improve test coverage - currently 49.5% in bio-java core #944

Copy link
Contributor

@josemduarte josemduarte left a comment

Choose a reason for hiding this comment

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

LGTM, thank you! Thanks for the javadocs too

return String.format("%1$-" + n + "s", s);
}

private static void validatePadding(int n) {
if (n <=0 ) {
Copy link
Member

Choose a reason for hiding this comment

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

Why can't the padding be == 0?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If I set padding to 0, a java.util.MissingFormatWidthException is thrown from within String.format method when padRight is called, and a FormatFlagsConversionMismatchException when padLeft is called.

Copy link
Member

Choose a reason for hiding this comment

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

Thank you for clarification, @richarda23. I doublechecked the Javadoc and it is inhibited from their side.
Please add your name in the @author tag.

@aalhossary aalhossary merged commit 600763e into biojava:master Jul 31, 2021
@richarda23 richarda23 deleted the tests-for-stringManipulationHelper branch July 31, 2021 16:02
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

4 participants