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

Added writing a row from variable number of arguments #51

Conversation

blackmo18
Copy link
Contributor

I think to be able to write a variable number of arguments is nice also without creating a list before hand.

example:

csvWriter().open(testFileName) {
    writeRow("a", "b", "c")
    writeRow("d", "e", "f")
    writeRow(1,2,3)
    writeRow(date1, date2, date3)
}

Although Kotlin already supports a variable argument in instantiating a list with initial values, I think this one will be cleaner and less boiler plate on the user end.

@codecov
Copy link

codecov bot commented May 30, 2020

Codecov Report

Merging #51 into master will increase coverage by 0.06%.
The diff coverage is 100.00%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master      #51      +/-   ##
============================================
+ Coverage     95.49%   95.56%   +0.06%     
- Complexity      145      146       +1     
============================================
  Files            21       21              
  Lines           822      834      +12     
  Branches         65       65              
============================================
+ Hits            785      797      +12     
  Misses           15       15              
  Partials         22       22              
Impacted Files Coverage Δ Complexity Δ
...thub/doyaaaaaken/kotlincsv/client/CsvFileWriter.kt 92.15% <100.00%> (+0.15%) 25.00 <1.00> (+1.00)
.../doyaaaaaken/kotlincsv/client/CsvFileWriterTest.kt 97.43% <100.00%> (+0.42%) 4.00 <0.00> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 67c6f68...b3dfac9. Read the comment docs.

* write one row
*/
override fun writeRow(vararg entry: Any) {
writeRow(entry.toList())
Copy link
Contributor Author

@blackmo18 blackmo18 May 30, 2020

Choose a reason for hiding this comment

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

Do you think it is better to specify explicitly the implementation, so it would not look like cyclic?

   override fun writeRow(vararg entry: Any) {
        willWritePreTerminator()
        writeNext(entry.toList())
        willWriteEndTerminator()

        if (writer.checkError()) {
            throw IOException("Failed to write")
        }
    }

Copy link
Owner

Choose a reason for hiding this comment

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

No, I don't think. Leave this as it is.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for giving your thoughts.

@doyaaaaaken
Copy link
Owner

Thanks a lot @blackmo18 , but in my sense, it's a little awkward to receive vararg arguments as data.
writeRow("a", "b", "c")

And as another reason, I can't come up with useful usecase to use this API.
I think in most cases, people use fun writeRow(row: List<Any?>) API.

@blackmo18
Copy link
Contributor Author

Yes, I think also it doesn't make sense in writing a large volume of entries in a file. On the other hand for small files, scripting, creating and exporting data models, and for testing purposes, it could be quite handy.

if you agree on this, I'll change the method signature into
fun writeRow(vararg entry: Any?) to make it uniform with the structure.

@doyaaaaaken
Copy link
Owner

OK, let's do so. Please go ahead and proceed with it.

@doyaaaaaken
Copy link
Owner

doyaaaaaken commented May 31, 2020

Also update README please.
Write description near this line.

writeRow(row2)

Victor Harlan D. Lacson added 3 commits June 1, 2020 06:01
…to feature/write-csv-row-from-variable-arguments
- include a sample in writing a row from variable arguments
@blackmo18
Copy link
Contributor Author

Hi @doyaaaaaken ,

already updated README and changed the method signature.

@doyaaaaaken doyaaaaaken merged commit ea55fea into doyaaaaaken:master Jun 1, 2020
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

Successfully merging this pull request may close these issues.

None yet

2 participants