Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move TIME_FORMAT_STRINGS to SasFileConstants #48

Closed
davbre opened this issue Jul 10, 2019 · 6 comments
Closed

Move TIME_FORMAT_STRINGS to SasFileConstants #48

davbre opened this issue Jul 10, 2019 · 6 comments

Comments

@davbre
Copy link
Contributor

davbre commented Jul 10, 2019

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
Copy link
Collaborator

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
Copy link
Contributor Author

davbre commented Jul 11, 2019

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
Copy link
Collaborator

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
Copy link
Contributor Author

davbre commented Jul 19, 2019

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

@Yana-Guseva
Copy link
Collaborator

Thanks very much!

@Yana-Guseva
Copy link
Collaborator

TIME_FORMAT_STRINGS moved to DateTimeConstants.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants