Open johnnieskywalker opened 7 years ago
Hi Johnnie, Thank you for respect.
Your thoughts, in general, look reasonable, but what and why I did Java implementation this way is covered by the following rules:
nlp-unconstrained-core/hooke-jeeves/__orig/src/hooke.c
file. It is coded in C. All other implementations are designed to reflect this code as close as possible in other languages. So I decided to keep all identifier naming throughout the project as they appear in the reference implementation. Keep all programming units as close as possible to that. All original code comments are also kept untouched and no additional comments were added. The exception is only Javadoc / Doxygen comments were added.Hope, now it is a little bit more clear how this code is organized.
Hi Radislav,
I see your point and that's understandable. You wrote the same algorithm in few languages using the same pattern. That's good I can just import it as Java project and see how it works instead of trying to understand C code which I am not as good at as in Java.
Although Java and other OO languages were invented for some reason. They offer some feautures which make writing reuseable code much easier. So don't you bother using them. Your code may still work the same way it worked in C and being much more readable.
As a proof I made a fork of your repo and did some small refactor. I made interface ObjectiveFunction which is implemented by Woods and Rosenbrock classes. It has method objectiveFunctionValue which is used instead all those messy long if else blocks. Moreover if you want just to add new function you just write new class implementing ObjectiveFunction interface. You dont have to change Hooke class anymore so now it respects SINGLE RESPONSIBILITY PRINCIPLE. I have also changed names of some variables and added some comments like "what jj variable name stands for". Do you believe if you take a look at your code in few years time you will remember that it was number of iterations? Other comment is about that you were returning 0.0 function value when Class passed to method was else than Rosenbrock or Woods. It may be achieved with default interface method returning 0.0. You can see my changes in merge request :
I'd be glad if you take a look at it :)
I beg your pardon if you find my comments impolite. Though I believe we all use GitHub to learn from each other right 😃 ? I learned from you implementsations of algorithms, and share my thoughts about some improvements that may be done. That makes us both more open minded programmers 😉
In this comment I talk about changes which you can see under Commits on Nov 5, 2016 .
In Commits on Nov 6, 2016 I made even further changes which still keep results the same but deleted few variables like "iAdj" which I found useless, but I know it had some meaning I don't understand. I have also added default implementation of findValueForArguments which returns 0.0 as it were in original code that's why I also changed java version in pom.xml to 8.
Hi, Respect for amount of work done on this programs. I try to make something similar in Java, and I got shocked by few things I found in your code.
For first what for do you pass argument "final int n" for functions f in Rosenbrock and Woods as it is not used anywhere.
Secondly why are you passing Rosenbrock and Woods as .class and then checking it in if then else. What if you had 1000 functions you would make 1000 if else blocks ?
You can make abstract class for example "MathFunction" which would have everything that every function that is optimized have in common and then just call its methof "f" or I'd better call it "functionValue" all this if else blocks would be unnnecesary, you will call function you need depending of what MathFunction extending class you will pass...
Best regards,