Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.
Sign upAdded CombSortTest.java #825
Conversation
copied over CombSort.java from master branch to Development branch. Added CombSortTest.java to Development branch
|
This should fix some naming and formatting errors to match with Java's coding style. |
|
|
||
| class CombSortTest { | ||
|
|
||
| //One global combSort instance variable |
| //One global combSort instance variable | ||
| CombSort combSort = new CombSort(); | ||
|
|
||
| //Integer Sort Test |
SirWindfield
Sep 15, 2019
Same goes here. The naming scheme of your methods should be testSomething. Like testIntegerSort and testStringSort. Methods in Java start with a lowercase letter. Make sure to correct that everywhere else too.
SirWindfield
Sep 15, 2019
•
No, just change it to testStringSort and testIntegerSort. Your class is named CombSortTest already, no need to add it to the method names. Alongside that, all test methods from the other classes are named using the testXXX scheme. And to keep consistency it would be better if you could change them too :)
jayyusi
Sep 15, 2019
Author
CycleSortTest class does not follow the consistency you mentioned ?!
it follows what I suggested :)
class CycleSortTest {
@Test
void cycleSortIntegerTest() {
CycleSort cycleSort = new CycleSort();
SirWindfield
Sep 15, 2019
•
If you take a look at all the other tests, they follow the convention testXXX where XXX gives some details of what is being tested. In your case, it would be testStringSort or something similar.
Edit:
Well, looks like some tests need a rename then :) They shouldn't be named like that though. Even the JUnit site lists the testXXX naming scheme as a go-to. And in all Java based open source projects I worked on they mostly used that scheme.
Well, I am in no position to force you to use it, I'm no member of this organisation. At least make the method names lowercase please. Because THAT is Java convention.
jayyusi
Sep 15, 2019
Author
haha, yes now you see what I mean, and now I see what you mean, I just noticed in the /search folder, the tests follow the naming convention of testXXX.
I did the changes, did you want me to also remove all the comments like this one:
//Integer Sort Test
I'm almost done, and thank you for the review and the edit suggestions. Its always good to have another pair of eyes. I admit I totally overlooked the method names.
SirWindfield
Sep 15, 2019
Would be nice if you could remove any comments that don't really add value, yes. If they are needed to explain why a certain thing is done, let them in. But something like // Integer Sort test is not really needed.
No problem :)
jayyusi
Sep 15, 2019
Author
alright, done.
I made a commit to the same pull request.
Do you know when will pull requests be merged with the development branch ?
|
|
||
| //compare the two results, our CombSort vs. Java's sort | ||
| Assertions.assertArrayEquals(array1,array2); | ||
|
|
|
|
||
| //Integer Sort Test | ||
| @Test | ||
| void CombSortIntegerTest(){ |
|
|
||
| //String Sort Test | ||
| @Test | ||
| void CombSortStringTest(){ |
removed unnecessary comments removed blank lines at end of method modified method names
copied over CombSort.java from master branch to Development branch.
Added CombSortTest.java to Development branch