epam/parso

Move TIME_FORMAT_STRINGS to SasFileConstants

Closed this issue · 6 comments

davbre commented

The DataWriterUtil class contains the following private static method:

private static final List<String> TIME_FORMAT_STRINGS = Arrays.asList("TIME", "HHMM");

As SasFileConstants has been recently changed to a public interface, could TIME_FORMAT_STRINGS be moved to it, to accompany DATE_FORMAT_STRINGS and DATE_TIME_FORMAT_STRINGS? If so, then other formats could be added as well such as "E8601LZ", "E8601TM", "MMSS", "HOUR" and "TIMEAMPM".

Yana-Guseva commented

Currently TIME_FORMAT_STRINGS is used only to format time while writing to .csv file. We can add other formats that you listed above to support more formats when writing to the output file, but I'm not sure that we need to move TIME_FORMAT_STRINGS from DataWriterUtil class to SasFileConstants. Are you going to use these formats in any way if they bacome public?

davbre commented

Yes, I already use DATE_FORMAT_STRINGS and DATE_TIME_FORMAT_STRINGS from SasFileConstants and was looking for an equivalent for time-related formats. I thought it would be handy to have it available in SasFileConstants. For the moment I'm defining it myself within my project, as:

    Set<String> TIME_FORMAT_STRINGS = new HashSet<String>(Arrays.asList(
            "E8601LZ", "E8601TM", "HHMM", "HOUR", "MMSS", "TIME", "TIMEAMPM"
    ));
Yana-Guseva commented

Ok, I'll move it to public interface. Maybe you have source sas7bdat files which you can share with us to test these new formats? If yes, I'll try to use these formats when writing to .csv file.

davbre commented

That's great, thanks. I've created a dataset for this and added it via pull request #49.

Yana-Guseva commented

Thanks very much!

Yana-Guseva commented

TIME_FORMAT_STRINGS moved to DateTimeConstants.