Commons CSV
  1. Commons CSV
  2. CSV-78

Use Character instead of char for char fields except delimiter

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 1.0
    • Component/s: None
    • Labels:
      None

      Description

      Apart from the delimiter - which must be specified (obviously) - the other char fields are optional.

      At present it's not possible to create a new format from an existing format and remove (say) the encapsulation character.

      If the parameters were changed to Character instead of char, then it would be possible to pass null.

        Issue Links

          Activity

          Hide
          Sebb added a comment -

          Two alternatives:

          1) provide methods to disable the setting, e.g. withNoEscape(), withNoEncapsulator(), withNoCommentStart()

          2) provide enable disable methods, e.g. setUseEscape(boolean).

          In both cases it would be useful to use separate fields to store the enabled/disabled setting.
          This would allow the tricky DISABLED char to be dropped.

          Show
          Sebb added a comment - Two alternatives: 1) provide methods to disable the setting, e.g. withNoEscape(), withNoEncapsulator(), withNoCommentStart() 2) provide enable disable methods, e.g. setUseEscape(boolean). In both cases it would be useful to use separate fields to store the enabled/disabled setting. This would allow the tricky DISABLED char to be dropped.
          Hide
          Sebb added a comment -

          Also, it would speed up the isEscaping() etc methods; the Lexer would not then need to cache the values.

          Show
          Sebb added a comment - Also, it would speed up the isEscaping() etc methods; the Lexer would not then need to cache the values.
          Hide
          Emmanuel Bourg added a comment -

          I'm fine with Characters instead of primitives if there is no impact on the parser performance. This will certainly mean that the primitive values have to be cached in the parser.

          Show
          Emmanuel Bourg added a comment - I'm fine with Characters instead of primitives if there is no impact on the parser performance. This will certainly mean that the primitive values have to be cached in the parser.
          Hide
          Sebb added a comment -

          I no longer like the idea of using a null Character to disable the setting, as it entails implicit or explicit boxing for the user code.

          Option 2) above was intended to allow the existing char (if any) to be preserved.
          However, using setEscape(true) would need to check if an escape had been set, so I think it was not a good idea either.

          Another possibility would be to provide no-arg versions of the with() methods.

          1) withNoEscape()     withNoEncapsulator()     withNoCommentStart()
          3) withEscape()       withEncapsulator()       withCommentStart()
          4) withEscapeNone()   withEncapsulatorNone()   withCommentStartNone()
          

          Option4 above is just option 1 renamed to look more like the existing methods.

          Other renames are possible.

          Show
          Sebb added a comment - I no longer like the idea of using a null Character to disable the setting, as it entails implicit or explicit boxing for the user code. Option 2) above was intended to allow the existing char (if any) to be preserved. However, using setEscape(true) would need to check if an escape had been set, so I think it was not a good idea either. Another possibility would be to provide no-arg versions of the with() methods. 1) withNoEscape() withNoEncapsulator() withNoCommentStart() 3) withEscape() withEncapsulator() withCommentStart() 4) withEscapeNone() withEncapsulatorNone() withCommentStartNone() Option4 above is just option 1 renamed to look more like the existing methods. Other renames are possible.
          Hide
          Benedikt Ritter added a comment -

          We should wait with this until we have implemented CSV-99.

          Show
          Benedikt Ritter added a comment - We should wait with this until we have implemented CSV-99 .
          Hide
          Benedikt Ritter added a comment -

          CSV-99 has been fixed. How do we want to fix this? I like withoutXxx().

          Show
          Benedikt Ritter added a comment - CSV-99 has been fixed. How do we want to fix this? I like withoutXxx() .
          Hide
          Emmanuel Bourg added a comment -

          I think I prefer a null value for disabling.

          Show
          Emmanuel Bourg added a comment - I think I prefer a null value for disabling.
          Hide
          Gary Gregory added a comment -

          Right now we have:

              public static final CSVFormat MYSQL =
                      DEFAULT
                      .withDelimiter(TAB)
                      .withEscape(BACKSLASH)
                      .withIgnoreEmptyLines(false)
                      .withQuoteChar(null)
                      .withRecordSeparator(LF);
          

          which looks pretty clear to me, with null as the quote char. We a have other API that take both Character and char.

          I thought this issue was done... hm...

          Show
          Gary Gregory added a comment - Right now we have: public static final CSVFormat MYSQL = DEFAULT .withDelimiter(TAB) .withEscape(BACKSLASH) .withIgnoreEmptyLines( false ) .withQuoteChar( null ) .withRecordSeparator(LF); which looks pretty clear to me, with null as the quote char. We a have other API that take both Character and char. I thought this issue was done... hm...
          Hide
          Sebb added a comment -

          URL: http://svn.apache.org/r1509239
          Log:
          CSV-78 Use Character instead of char for char fields except delimiter
          Already done as part of CSV-99 etc.
          Updated Javadoc to clarify that null can be used to disable settings

          Modified:
          commons/proper/csv/trunk/src/main/java/org/apache/commons/csv/CSVFormat.java

          Show
          Sebb added a comment - URL: http://svn.apache.org/r1509239 Log: CSV-78 Use Character instead of char for char fields except delimiter Already done as part of CSV-99 etc. Updated Javadoc to clarify that null can be used to disable settings Modified: commons/proper/csv/trunk/src/main/java/org/apache/commons/csv/CSVFormat.java

            People

            • Assignee:
              Unassigned
              Reporter:
              Sebb
            • Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development