2013/5/6 sebb <seb...@gmail.com> > 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. >
Okay, this makes sense to me then. Thanks for the clarification. > > > > > > > > > > > > > > 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 > > > -- http://people.apache.org/~britter/ http://www.systemoutprintln.de/ http://twitter.com/BenediktRitter http://github.com/britter
