ali-ghanbari commented on pull request #233: URL: https://github.com/apache/commons-text/pull/233#issuecomment-888817931
> Hi @ali-ghanbari > > I was going to start reviewing algorithm C, but I realized you had a few more differences in the pull request from the last time I read your code. > > There are problems with the current implementation, namely the @SInCE version that is not correct, one unnecessary blank space (I can fix that later if needed when merging), and the most important that was the parameters for the function that - I think - you moved to members of the class so that Checkstyle would not complain. > > It would be better to avoid maintaining state in the object if that's not necessary (someone reading the code will immediately know it's thread-safe, since it has no dependency to other classes, and no local state, among other benefits). > > Your previous version was really good, very close to being ready to be merged. Could you undo these changes I mentioned above, keeping whatever you had about algo B and C, please? > > Thanks! > Bruno Certainly, @kinow :) I will address all of your comments soon. By the way, JMH Maven profile and the test code are ready; I can push that also. Thanks! -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@commons.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
