On 6 May 2013 18:53, Benedikt Ritter <brit...@apache.org> wrote: > 2013/5/5 sebb <seb...@gmail.com> > > > On 3 May 2013 14:37, Benedikt Ritter <brit...@apache.org> wrote: > > > > > (moving this to a new thread) > > > > > > Hi, > > > > > > I did a very small refactoring in Lexer just to realize that the > > isXXX(int) > > > methods (like boolean isDelimiter(int c)) really belong to CSVFormat > > rather > > > than to the Lexer. How do you feel about this? > > > > > > > > If I remember correctly, the methods were added to Lexer to improve > > performance. > > > > Oh, I never thought that calling a members method would have such an impact > on performance. Thanks for the info! > > I don't think it had a huge impact, but there would have been some benefit.
Note also that the CSVFormat fields are Character rather than char, so the checks would be more complicated if moved to CSVFormat as it is currently. I don't remember if this was the case at the time the methods were moved. > > > > > > > > Benedikt > > > > > > 2013/5/3 sebb <seb...@gmail.com> > > > > > > > On 3 May 2013 07:40, Benedikt Ritter <brit...@apache.org> wrote: > > > > > > > > > The format field in Lexer is now only used by CSVLexer1 in the test > > > > > directory. It may therefore be removed. > > > > > I wonder if the isXXX methods really belong to CSVFormat. > > > > > > > > > > WDYT? > > > > > > > > > > > > > > The CSVLexerN entries in the test directory are copies of the > > originals, > > > > for comparative performance tests, and should be left alone as far as > > > > possible. > > > > > > > > > > Benedikt > > > > > > > > > > > > > > > > > > > 2013/5/3 <brit...@apache.org> > > > > > > > > > > > Author: britter > > > > > > Date: Fri May 3 06:37:58 2013 > > > > > > New Revision: 1478655 > > > > > > > > > > > > URL: http://svn.apache.org/r1478655 > > > > > > Log: > > > > > > Use isDelimiter method instead of != check. > > > > > > > > > > > > Modified: > > > > > > > > > > > > > > > > > > commons/proper/csv/trunk/src/main/java/org/apache/commons/csv/Lexer.java > > > > > > > > > > > > Modified: > > > > > > > > > > > > commons/proper/csv/trunk/src/main/java/org/apache/commons/csv/Lexer.java > > > > > > URL: > > > > > > > > > > > > > > > > > > > > > http://svn.apache.org/viewvc/commons/proper/csv/trunk/src/main/java/org/apache/commons/csv/Lexer.java?rev=1478655&r1=1478654&r2=1478655&view=diff > > > > > > > > > > > > > > > > > > > > > > > > > > > ============================================================================== > > > > > > --- > > > > > > > > > > > > commons/proper/csv/trunk/src/main/java/org/apache/commons/csv/Lexer.java > > > > > > (original) > > > > > > +++ > > > > > > > > > > > > commons/proper/csv/trunk/src/main/java/org/apache/commons/csv/Lexer.java > > > > > > Fri May 3 06:37:58 2013 > > > > > > @@ -146,7 +146,7 @@ abstract class Lexer { > > > > > > * @return true if the given char is a whitespace character > > > > > > */ > > > > > > boolean isWhitespace(final int c) { > > > > > > - return c != format.getDelimiter() && > > > > > > Character.isWhitespace((char) c); > > > > > > + return !isDelimiter(c) && Character.isWhitespace((char) > > c); > > > > > > } > > > > > > > > > > > > /** > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > -- > > > > > http://people.apache.org/~britter/ > > > > > http://www.systemoutprintln.de/ > > > > > http://twitter.com/BenediktRitter > > > > > http://github.com/britter > > > > > > > > > > > > > > > > > > > > > -- > > > http://people.apache.org/~britter/ > > > http://www.systemoutprintln.de/ > > > http://twitter.com/BenediktRitter > > > http://github.com/britter > > > > > > > > > -- > http://people.apache.org/~britter/ > http://www.systemoutprintln.de/ > http://twitter.com/BenediktRitter > http://github.com/britter >
