misterspeedy
AsWorkBook taking an #seq<Item> rather than Item list
Is there a specific reason why you opted for Item list instead of #seq<Item> as the type for the items argument? Utilizing #seq<Item> would have greatly simplified creating composable functions because it defers the computation of Item references until rendering is required.
Did you have another engineering consideration behind this decision? As far as I can see, the code works just as well when changing the argument type to items: #seq<Item>, without having to pre-allocate the entire list before rendering.
It seems like a list type would be beneficial if there were back references to access. However, in such cases, an array or ResizeArray would be the ideal choice. Currently, I don't see any such usage in the code that would imply the need for this.
So I would suggest changing the type to items:#seq<Item> which would not be a breaking change for current users since Item list is an compatible with #seq<Item>.
Thanks, -Patrick
You're right, and this has been on my conscience for a while. Feel free to submit a PR but if that's not convenient I'll do it when I have a moment. Thank you for the suggestion!
I can absolutely put together a pull request.
From: Kit Eason @.> Sent: Tuesday, May 30, 2023 4:02 PM To: misterspeedy/FsExcel @.> Cc: Patrick Simpson @.>; Author @.> Subject: Re: [misterspeedy/FsExcel] AsWorkBook taking an #seq<Item> rather than Item list (Issue #48)
You're right, and this has been on my conscience for a while. Feel free to submit a PR but if that's not convenient I'll do it when I have a moment. Thank you for the suggestion!
— Reply to this email directly, view it on GitHubhttps://github.com/misterspeedy/FsExcel/issues/48#issuecomment-1569167234, or unsubscribehttps://github.com/notifications/unsubscribe-auth/ADTVWHU5SK5AR4OVSLHLWRLXIZU6BANCNFSM6AAAAAAYUKXV4U. You are receiving this because you authored the thread.Message ID: @.***>