patterncsharpMajor
Exporting information on a collection of books to Excel
Viewed 0 times
booksexcelexportingcollectioninformation
Problem
I have the following code that loops through a collection of objects and dumps different properties out to Excel. The whole
I am constantly adding new columns at the beginning and middle, so I want to avoid hard coding column names.
j++ on every other line doesn't seem very elegant. Is there a more elegant way to have this functionality where I loop through objects in a collection and dump out properties?int rowIndex = 2;
foreach (BookInfo book in books)
{
int j = 1;
excelExport.SetCell(j, rowIndex, book.BookId);
j++;
excelExport.SetCell(j, rowIndex, book.Book);
j++;
excelExport.SetCell(j, rowIndex, book.System);
j++;
excelExport.SetCell(j, rowIndex, book.Age);
j++;
excelExport.SetCell(j, rowIndex, book.StartDate);
j++;
excelExport.SetCell(j, rowIndex, book.Pages);
rowIndex++;
}I am constantly adding new columns at the beginning and middle, so I want to avoid hard coding column names.
Solution
I think your code is perfectly understandable as it is.
Here's an idea though. (In case it is not clear: the suggestions here are more for amusement and edification than a serious suggestion. The original procedural code is just fine, but it is interesting to see how it might be done in a functional style.)
However, this still has a disappointingly large number of variable mutations. Why do we need to have local variables to track the rows and columns at all? That's the inelegant part that you want to eliminate.
Can't tell if trolling...or just addicted to lambdas
Oh, we're just getting started here. I have barely yet even begun to use lambdas. How about this?
There, now we've got a selector that contains a lambda that contains a selector that contains a lambda that iterates over a list of lambdas. We've also gotten rid of every index mutation.
That of course has far too many explanatory local variables. We should be able to do this without mutating any variable except the loop variable. Let's eliminate all the variable mutations except one:
And now we've illustrated the old saying: every programming language eventually resembles Lisp -- badly.
As Scott Rippey points out in his answer, we actually don't need to be capturing the properties as lambdas at all; we could just capture the values:
Which is actually not too bad.
But like I said, your original code is just fine.
Here's an idea though. (In case it is not clear: the suggestions here are more for amusement and edification than a serious suggestion. The original procedural code is just fine, but it is interesting to see how it might be done in a functional style.)
var funcs = new List>()
{
info=>info.BookId,
info=>info.Book,
info=>info.System // etc.
}
int rowIndex = 2;
foreach (BookInfo bookInfo in books)
{
int columnIndex = 1;
foreach(var func in funcs)
{
excelExport.SetCell(columnIndex, rowIndex, func(bookInfo));
columnIndex += 1;
}
rowIndex += 1;
}However, this still has a disappointingly large number of variable mutations. Why do we need to have local variables to track the rows and columns at all? That's the inelegant part that you want to eliminate.
Can't tell if trolling...or just addicted to lambdas
Oh, we're just getting started here. I have barely yet even begun to use lambdas. How about this?
var funcs = new List>()
{
info=>info.BookId,
info=>info.Book,
info=>info.System // etc.
}
var cells = bookInfos.SelectMany(
(bookInfo, row)=>
funcs.Select(
(func, col)=>
new {row, col, item = func(bookInfo)}));
foreach(var cell in cells)
excel.SetCell(cell.col, cell.row, cell.item);There, now we've got a selector that contains a lambda that contains a selector that contains a lambda that iterates over a list of lambdas. We've also gotten rid of every index mutation.
That of course has far too many explanatory local variables. We should be able to do this without mutating any variable except the loop variable. Let's eliminate all the variable mutations except one:
foreach(var cell in
bookInfos.SelectMany(
(bookInfo, row)=>
new List>()
{
info=>info.BookId,
info=>info.Book,
info=>info.System // etc.
}.Select(
(func, col)=>
new {row, col, item = func(bookInfo)})))
excel.SetCell(cell.col, cell.row, cell.item);And now we've illustrated the old saying: every programming language eventually resembles Lisp -- badly.
As Scott Rippey points out in his answer, we actually don't need to be capturing the properties as lambdas at all; we could just capture the values:
foreach(var cell in
bookInfos.SelectMany(
(bookInfo, row)=>
new object[]
{
bookInfo.BookId,
bookInfo.Book,
bookInfo.System // etc.
}.Select(
(item, col)=>
new {row, col, item})))
excel.SetCell(cell.col, cell.row, cell.item);Which is actually not too bad.
But like I said, your original code is just fine.
Code Snippets
var funcs = new List<Func<BookInfo, object>>()
{
info=>info.BookId,
info=>info.Book,
info=>info.System // etc.
}
int rowIndex = 2;
foreach (BookInfo bookInfo in books)
{
int columnIndex = 1;
foreach(var func in funcs)
{
excelExport.SetCell(columnIndex, rowIndex, func(bookInfo));
columnIndex += 1;
}
rowIndex += 1;
}var funcs = new List<Func<BookInfo, object>>()
{
info=>info.BookId,
info=>info.Book,
info=>info.System // etc.
}
var cells = bookInfos.SelectMany(
(bookInfo, row)=>
funcs.Select(
(func, col)=>
new {row, col, item = func(bookInfo)}));
foreach(var cell in cells)
excel.SetCell(cell.col, cell.row, cell.item);foreach(var cell in
bookInfos.SelectMany(
(bookInfo, row)=>
new List<Func<BookInfo, object>>()
{
info=>info.BookId,
info=>info.Book,
info=>info.System // etc.
}.Select(
(func, col)=>
new {row, col, item = func(bookInfo)})))
excel.SetCell(cell.col, cell.row, cell.item);foreach(var cell in
bookInfos.SelectMany(
(bookInfo, row)=>
new object[]
{
bookInfo.BookId,
bookInfo.Book,
bookInfo.System // etc.
}.Select(
(item, col)=>
new {row, col, item})))
excel.SetCell(cell.col, cell.row, cell.item);Context
StackExchange Code Review Q#7264, answer score: 30
Revisions (0)
No revisions yet.