Thought more about Eloy's comments and think that :
- it would be inefficient to implement a method "download()" which would be an alternative to print_html. The problem is that this would entail always keeping a copy of all data added to the table. There would necessarily be a method add_data as there is now. The data added to the table would have to be stored in an array. If download is called after adding all the data to the table then another for loop would have to write these values into the data structure the export plug in would need.
- we have already told the table we are downloading by calling is_downloading(). Why do we have to call the download method to tell the table again that we are downloading. Instead add_data should do the correct thing with the data depending on what format we set the is_downloading flag to.
So I think that we should probably do a download as follows :
//start_output is always called. If this is not a download, it does nothing.
//add data to table/export here with a for loop
for ($data as $datarow)
//process data here
In the case of the export formats to spreadsheet or to CSV we will probably output the data as we go along, instead of keeping a copy in memory. The old table class currently keeps a copy of the data in memory and then constructs the html from the internal array when you call print_html. We could call print_html from the finish_output function.
I would propose we have a new plugin folder called lib/tableexport/
/ these would contain folders with files of a sub class of table_export_format that would have methods add_data, start_output and finish_output
Instead of is_downloading() calls to check whether to output html or non-html when formatting rows of data we would call $table->wants_html() there would be a wants_html() method on the tableexport plug in classes which would be set to return false on the default parent class.