egor.pasko commented on revision r45 in project apache-rat-pd.
Details are at http://code.google.com/p/apache-rat-pd/source/detail?r=45
Score: Positive
Line-by-line comments:
File: /trunk/src/main/java/org/apache/rat/pd/core/PlagiarismDetector.java
(r45)
===============================================================================
Line 102: pdCommandLine, out);
-------------------------------------------------------------------------------
>this probably looks better if wrapped after '= 'like this:
>final List<IHeuristicChecker> algorithmsForChecking =
> configureHeuristicCheckers(pdCommandLine, out);
I totally agree. :)
thanks for fixing this in some next commit
I use default Eclipse built-in formatter. If it is better to use any
different formatter, lets do it!
This was surprising to me. I never really used default formatting in
Eclipse, so I did not know. Here is how to fix:
Window -> Preferences -> Java -> Code Style -> Formatter -> Select a
Profile (Eclipse [built-in]) Show ... -> Indentation -> Tab policy ->
Spaces only -> Apply
name it something like "Eclipse [spaces]"
Then use Source -> Format to reformat individual files. Worked for me :)
Do you like that?
To reformat everything a quick replacement script can be run. A usual
request in such cases is not to intermix formatting changes with other work
in a single commit.
Line 130: * @param pdCommandLine
-------------------------------------------------------------------------------
It is nice that you notice that. It is already done.
thanks for doing this! it makes the code cleaner
Line 135: List<ISearchEngine> toret = new
ArrayList<ISearchEngine>();
-------------------------------------------------------------------------------
>Comment by egor.pasko, Aug 05 (5 days ago):
>hm, 'ret' is more natural to me than 'toret', 'toRet' is comprehensible
too .. java people > just love >CamelCase
This is correct. I like CamelCase, too! :)
hey :) you could give a link on the revision where you fix this. I checked
the last revision. Sounds to be fixed. Thanks for that!
Line 154: PdCommandLine pdCommandLine, PrintStream out) throws
IOException {
-------------------------------------------------------------------------------
Actually, if we want to use logging this way, PrintStream can be passed by
[1]setter method
[2]through class constructor
[3]like public static class member
I prefer first two approach . I think that situation in this class will
be much better if all
methods are not static anymore. PrintStream will be then just a class
member. main
function will be begin with:
PlagiarismDetector pd = new PlagiarismDetector();
.................
final PrintStream out = pd.getProperPrintStream(pdCommandLine);
I am OK with this approach.
However, to make code shorter you could make a public setter with a
protected getter in a superclass of all heuristics. And then you do not
need to define this thing in every heuristic. And cleaning up is easier.
Just a thought.
Line 336: new ByteArrayOutputStream());
-------------------------------------------------------------------------------
This is nice hack. :) Thanks.
I am happy to help you, and this is not a hack :)
Respond to these comments at
http://code.google.com/p/apache-rat-pd/source/detail?r=45
--
You received this message because you starred this review, or because
your project has directed all notifications to a mailing list that you
subscribe to.
You may adjust your review notification preferences at:
http://code.google.com/hosting/settings
