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

Add helper methods to CsvOutput to bridge writing CSV Files and Iterators #1984

Merged
merged 3 commits into from May 29, 2019

Conversation

@ashleyheath
Copy link
Contributor

@ashleyheath ashleyheath commented May 29, 2019

Sometimes you already have an instance of a CsvFile or CsvIterator that you want to write out somewhere. This PR adds some static helper methods to CsvOutput to help support that.

… existing CsvFile and CsvIterator instances.
@ashleyheath ashleyheath requested a review from jodastephen May 29, 2019
* @param alwaysQuote when true, each column will be quoted, when false, quoting is selective
* @throws UncheckedIOException if an IO exception occurs
*/
public void writeRows(Iterable<? extends CsvRow> rows, boolean alwaysQuote) {
Copy link
Member

@jodastephen jodastephen May 29, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • Method should be next to writeRow (after writeLine)
  • CsvRow is final, so ? extends won't add much.

Copy link
Contributor Author

@ashleyheath ashleyheath May 29, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Keep writeLine_s_ and writeRow_s_ next to each other?

* @param file the file whose contents to output
* @param underlying the destination to write to
*/
public static void writeStandard(CsvFile file, Appendable underlying) {
Copy link
Member

@jodastephen jodastephen May 29, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think instance methods would be the better choice, as 12 methods should reduce to 2.

Copy link
Contributor Author

@ashleyheath ashleyheath May 29, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add an 'alwaysQuote' param to each (for consistency) and make that four?

Copy link
Contributor Author

@ashleyheath ashleyheath May 29, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, ignore that, safe/standard is already on the instance, isn't it.

@@ -222,6 +222,22 @@ public void writeLines(Iterable<? extends List<String>> lines, boolean alwaysQuo
}
}

/**
Copy link
Member

@jodastephen jodastephen May 29, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Move method below lines methods. ie. group all the lines methods, then all the rows methods, then all the cell methods, then the file/iterator ones

Copy link
Contributor Author

@ashleyheath ashleyheath May 29, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

@ashleyheath ashleyheath merged commit 0895623 into master May 29, 2019
1 of 3 checks passed
@delete-merged-branch delete-merged-branch bot deleted the csv_output_writers branch May 29, 2019
@jodastephen jodastephen added this to the v2.4 milestone May 31, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants