Patch/#35 codacy issues #38
Conversation
…mann/opt4j.git into patch/#35-codacy-issues
reverted respective occurrences
| * | ||
| * @param value | ||
| * @param valueToSet | ||
| * the value to set | ||
| * @throws InvocationTargetException | ||
| * thrown if the value cannot be assigned | ||
| */ | ||
| public void setValue(String value) throws InvocationTargetException { |
There was a problem hiding this comment.
Not the same as what is mentioned in @param
There was a problem hiding this comment.
Corrected. Refactoring artifact.
| @@ -70,7 +70,7 @@ public void start(String[] args) throws ClassNotFoundException { | |||
| String filename = null; | |||
| Class<? extends Task> taskClass = null; | |||
| if (args.length < 1) { | |||
| throw new RuntimeException("Specify the task class"); | |||
| throw new ClassNotFoundException("Specify the task class"); | |||
There was a problem hiding this comment.
ClassNotFound is used for classpath Issues. Does this fit here? We should check the spec of CNF. Alternatively, IllegalArgumentExcpetion or IllegalStateException could also fit.
There was a problem hiding this comment.
Since it checks for at least one task class to be given as an argument, I think IllegalArgumentException is the best fit. Corrected.
| protected double getMinDistance(Spea2IndividualSet w0) { | ||
| double min = Double.MAX_VALUE; | ||
| for (Spea2IndividualSet w1 : individualSets) { | ||
| if (w0 != w1) { | ||
| if (w0 == w1) { |
There was a problem hiding this comment.
Has this been a logic error? Then, we should outline this explicitly in the release notes of the next release and also make this bug fix release quite soon.
There was a problem hiding this comment.
That was an important catch! Thanks a lot. I made that mistake reverting back from equals(). Corrected.
| @@ -32,8 +32,8 @@ | |||
| * <p> | |||
| * A Java implementation of the MT19937 (Mersenne Twister) pseudo random | |||
| * number generator algorithm based upon the original C code by Makoto | |||
| * Matsumoto and Takuji Nishimura (see <a | |||
| * href="http://www.math.sci.hiroshima-u.ac.jp/~m-mat/MT/emt.html"> | |||
| * Matsumoto and Takuji Nishimura (see | |||
There was a problem hiding this comment.
Shouldn't this be handled with the new Citation annotation?
There was a problem hiding this comment.
I suggest separation of concerns and fix this in another patch to restrict this to codacy styling.
There was a problem hiding this comment.
I did intentionally not use @citation here as the above references the code source, not a paper. Nonetheless, we can discuss if the corresponding paper should also be cited here, but this is SoC again.
| @@ -8,6 +8,9 @@ | |||
| import org.opt4j.core.genotype.PermutationGenotype; | |||
|
|
|||
| public class IndividualTest { | |||
|
|
|||
| private boolean stateChanged = false; | |||
There was a problem hiding this comment.
A proposition (not specifically targeted at this case): What do you think about making all class attributes protected instead of private. This makes writing tests a lot easier in many cases.
There was a problem hiding this comment.
SoC. Create different issue/patch.
| for (int i = 0; i < pop.size(); i++) { | ||
| id.put(pop.get(i), i); | ||
| List<Individual> population = new ArrayList<Individual>(individuals); | ||
| Map<Individual, Integer> individualID = new HashMap<Individual, Integer>(); |
There was a problem hiding this comment.
naming: better use plural for maps and sets. individualIds
| @@ -0,0 +1,5 @@ | |||
| --- | |||
| exclude_paths: | |||
| - 'opt4j-viewer/src/main/java/ptolemy/**/*' | |||
There was a problem hiding this comment.
What is your gut feeling. Should we also exclude all the unit tests or are the changes you did also senseful for them?
There was a problem hiding this comment.
I reworked the tests, so I would keep it currently. Let us discuss this as part of issue #39.
| } | ||
|
|
||
| /** | ||
| * Helper function for crossover() to fill and rotate. |
There was a problem hiding this comment.
Could you extend the description a little bit? What is done in this function, what is done in the one above.
There was a problem hiding this comment.
I mentioned the respective cut point as the difference between the functions. The concrete algorithm is already detailed in the comment of the complete class so I would not detail the comment for helper functions further than the new one I committed.
A: Patched the kind of obvious PMD issues and left out some that we have to discuss further and code marked as duplicate by codacy.
This is mostly:
B: Added .codacy.yml to exclude ptolemy plot and .js and .py files.