Comments
I think your huge comment block about the "Requirement" of your class can be better placed as a class-level Javadoc comment:
/**
* Return common characters joined in a String, preserving the order in...
*/
public class CommonCharacters5 {
// ...
}
Utility classes and methods
For utility classes and methods, the convention is to make the class final
and the constructor private
so that it's clear that they should not be instantiated:
public final class CommonCharacters5 {
private CommonCharacters5() {
// empty
}
public static String commonCharactersOf(String string1, String string2) {
// ...
}
}
CharSequence
vs String
Since codePoints()
is actually a method from the CharSequence
interface, you may want to consider making your method accept a pair of CharSequence
s so that they are even less restrictive to work with.
Unit testing
I lean more towards TestNG myself, and I think its @DataProvider
annotation usage is more expressive to construct a set of tests to assert iteratively. I happen to have another answer here here that touches on parameterized testing with TestNG. :)
Regardless of the testing framework though, you may want to consider whether to opt for a better modeling approach for your tests, to make them more expressive. For example, if you can encapsulate them as an Enum
:
enum MyTestCase {
EMPTY_STRINGS("", "", ""),
A_AND_EMPTY("a", "", ""),
A_AND_B("a", "b", ""),
// ...
ROCKET_EMOJI("\uD83D\uDE80", "\uD83D\uDE80", "\uD83D\uDE80"),
// ...
private final String one;
private final String other;
private final String expected;
private MyTestCase(String one, String other, String expected) {
this.one = one;
this.other = other;
this.expected = expected;
}
}
In your case, the slight benefit I can see is that you can freely swap the inputs to test that your method works regardless of which argument is the longer one:
@org.junit.Test
public void test() {
// assuming current is an instance of MyTestCase
Assert.assertEquals(current.expected, commonCharactersOf(current.one, current.other));
Assert.assertEquals(current.expected, commonCharactersOf(current.other, current.one));
}
Comments
I think your huge comment block about the "Requirement" of your class can be better placed as a class-level Javadoc comment:
/**
* Return common characters joined in a String, preserving the order in...
*/
public class CommonCharacters5 {
// ...
}
Utility classes and methods
For utility classes and methods, the convention is to make the class final
and the constructor private
so that it's clear that they should not be instantiated:
public final class CommonCharacters5 {
private CommonCharacters5() {
// empty
}
public static String commonCharactersOf(String string1, String string2) {
// ...
}
}
CharSequence
vs String
Since codePoints()
is actually a method from the CharSequence
interface, you may want to consider making your method accept a pair of CharSequence
s so that they are even less restrictive to work with.
Unit testing
I lean more towards TestNG myself, and I think its @DataProvider
annotation usage is more expressive to construct a set of tests to assert iteratively. I happen to have another answer here that touches on parameterized testing with TestNG. :)
Regardless of the testing framework though, you may want to consider whether to opt for a better modeling approach for your tests, to make them more expressive. For example, if you can encapsulate them as an Enum
:
enum MyTestCase {
EMPTY_STRINGS("", "", ""),
A_AND_EMPTY("a", "", ""),
A_AND_B("a", "b", ""),
// ...
ROCKET_EMOJI("\uD83D\uDE80", "\uD83D\uDE80", "\uD83D\uDE80"),
// ...
private final String one;
private final String other;
private final String expected;
private MyTestCase(String one, String other, String expected) {
this.one = one;
this.other = other;
this.expected = expected;
}
}
In your case, the slight benefit I can see is that you can freely swap the inputs to test that your method works regardless of which argument is the longer one:
@org.junit.Test
public void test() {
// assuming current is an instance of MyTestCase
Assert.assertEquals(current.expected, commonCharactersOf(current.one, current.other));
Assert.assertEquals(current.expected, commonCharactersOf(current.other, current.one));
}
Comments
I think your huge comment block about the "Requirement" of your class can be better placed as a class-level Javadoc comment:
/**
* Return common characters joined in a String, preserving the order in...
*/
public class CommonCharacters5 {
// ...
}
Utility classes and methods
For utility classes and methods, the convention is to make the class final
and the constructor private
so that it's clear that they should not be instantiated:
public final class CommonCharacters5 {
private CommonCharacters5() {
// empty
}
public static String commonCharactersOf(String string1, String string2) {
// ...
}
}
CharSequence
vs String
Since codePoints()
is actually a method from the CharSequence
interface, you may want to consider making your method accept a pair of CharSequence
s so that they are even less restrictive to work with.
Unit testing
I lean more towards TestNG myself, and I think its @DataProvider
annotation usage is more expressive to construct a set of tests to assert iteratively. I happen to have another answer here that touches on parameterized testing with TestNG. :)
Regardless of the testing framework though, you may want to consider whether to opt for a better modeling approach for your tests, to make them more expressive. For example, if you can encapsulate them as an Enum
:
enum MyTestCase {
EMPTY_STRINGS("", "", ""),
A_AND_EMPTY("a", "", ""),
A_AND_B("a", "b", ""),
// ...
ROCKET_EMOJI("\uD83D\uDE80", "\uD83D\uDE80", "\uD83D\uDE80"),
// ...
private final String one;
private final String other;
private final String expected;
private MyTestCase(String one, String other, String expected) {
this.one = one;
this.other = other;
this.expected = expected;
}
}
In your case, the slight benefit I can see is that you can freely swap the inputs to test that your method works regardless of which argument is the longer one:
@org.junit.Test
public void test() {
// assuming current is an instance of MyTestCase
Assert.assertEquals(current.expected, commonCharactersOf(current.one, current.other));
Assert.assertEquals(current.expected, commonCharactersOf(current.other, current.one));
}
Comments
I think your huge comment block about the "Requirement" of your class can be better placed as a class-level Javadoc comment:
/**
* Return common characters joined in a String, preserving the order in...
*/
public class CommonCharacters5 {
// ...
}
Utility classes and methods
For utility classes and methods, the convention is to make the class final
and the constructor private
so that it's clear that they should not be instantiated:
public final class CommonCharacters5 {
private CommonCharacters5() {
// empty
}
public static String commonCharactersOf(String string1, String string2) {
// ...
}
}
CharSequence
vs String
Since codePoints()
is actually a method from the CharSequence
interface, you may want to consider making your method accept a pair of CharSequence
s so that they are even less restrictive to work with.
Unit testing
I lean more towards TestNG myself, and I think its @DataProvider
annotation usage is more expressive to construct a set of tests to assert iteratively. I happen to have another answer here that touches on parameterized testing with TestNG. :)
Regardless of the testing framework though, you may want to consider whether to opt for a better modeling approach for your tests, to make them more expressive. For example, if you can encapsulate them as an Enum
:
enum MyTestCase {
EMPTY_STRINGS("", "", ""),
A_AND_EMPTY("a", "", ""),
A_AND_B("a", "b", ""),
// ...
ROCKET_EMOJI("\uD83D\uDE80", "\uD83D\uDE80", "\uD83D\uDE80"),
// ...
private final String one;
private final String other;
private final String expected;
private MyTestCase(String one, String other, String expected) {
this.one = one;
this.other = other;
this.expected = expected;
}
}
In your case, the slight benefit I can see is that you can freely swap the inputs to test that your method works regardless of which argument is the longer one:
@org.junit.Test
public void test() {
// assuming current is an instance of MyTestCase
Assert.assertEquals(current.expected, commonCharactersOf(current.one, current.other));
Assert.assertEquals(current.expected, commonCharactersOf(current.other, current.one));
}