-
-
Notifications
You must be signed in to change notification settings - Fork 640
Initial commit to support Cell styling and number formats in styles #209
Changes from 8 commits
4f5218d
6705219
e4ccef8
def9336
5388764
3b03a9c
eda5e3b
7baf549
73df02f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -133,10 +133,18 @@ public function addRow($dataRow, $style) | |
|
||
$rowXML = '<row r="' . $rowIndex . '" spans="1:' . $numCells . '">'; | ||
|
||
foreach($dataRow as $cellValue) { | ||
foreach($dataRow as $cell) { | ||
if (is_array($cell)) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not a fan of this, where a value can have multiple types. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why shouldn't a value be able to be generated with multiple types (I agree, would be nicer with a class instead of whats happening now). Since a value and its interaction within the sheet can be different depending on type, especially when running around with different formats. I could also change this so you set a styling object for a specific Column Index (Which will introduce another feature). That way you won't have the multiple types for a cell There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. A parameter that can be of multiple types hides the complexity of the program. If you pass an number, it will do X but if you pass an array it will do Y. While it's fine to do different things, the expectation of what a function does should be clear. In this example, there should probably be 2 functions (one taking a number, one taking an array) with explicit names. The problem is less the difference between a number and a string but more between a single element VS a collection. Those represent really different things and should be treated carefully. |
||
$cellValue = $cell[0]; | ||
$cellStyle = $cell[1]; | ||
} else { | ||
$cellValue = $cell; | ||
$cellStyle = $style; | ||
} | ||
|
||
$columnIndex = CellHelper::getCellIndexFromColumnIndex($cellNumber); | ||
$cellXML = '<c r="' . $columnIndex . $rowIndex . '"'; | ||
$cellXML .= ' s="' . $style->getId() . '"'; | ||
$cellXML .= ' s="' . $cellStyle->getId() . '"'; | ||
|
||
if (CellHelper::isNonEmptyString($cellValue)) { | ||
if ($this->shouldUseInlineStrings) { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,113 @@ | ||
<?php | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Tests should be written with PHPUnit (look at examples in the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This was just a quick test file to test what the performance decrease/impact would be. Since the premise of being able to style cells would seem te be a lot heavier, never intended to use this as a unit test |
||
|
||
include_once 'src/Spout/Autoloader/autoload.php'; | ||
|
||
use Box\Spout\Common\Type; | ||
use Box\Spout\Writer\WriterFactory; | ||
use Box\Spout\Writer\Style\StyleBuilder; | ||
|
||
# UNIX_DATE = (EXCEL_DATE - 25569) * 86400 : Excel -> unix | ||
# EXCEL_DATE = 25569 + (UNIX_DATE / 86400) : Unix -> excel | ||
|
||
$test = function ($name, $sheets, $rows, $cols, $groupedRows = FALSE) { | ||
$start = microtime(TRUE); | ||
$writer = WriterFactory::create(Type::XLSX); // for XLSX files | ||
$writer->openToFile('__'.$name.'.xlsx'); | ||
|
||
$styleDef = (new StyleBuilder())->setNumberFormat('0.00000')->build(); | ||
$styleRed = (new StyleBuilder())->setNumberFormat('[Red]0.00000')->build(); | ||
$styleDate = (new StyleBuilder())->setNumberFormat('d-mmm-YY HH:mm:ss')->build(); | ||
$writer->registerStyle($styleDef); | ||
$writer->registerStyle($styleRed); | ||
$writer->registerStyle($styleDate); | ||
|
||
for ($sheetNum = 0; $sheetNum < $sheets; $sheetNum++) { | ||
if ($sheetNum != 0) { | ||
$sheet = $writer->addNewSheetAndMakeItCurrent(); | ||
} else { | ||
$sheet = $writer->getCurrentSheet(); | ||
} | ||
$sheet->setName('data'.$sheetNum); | ||
|
||
$rowsArr = array(); | ||
for ($rowNum = 0; $rowNum < $rows; $rowNum++) { | ||
$row = array(); | ||
for ($colNum = 0; $colNum < $cols; $colNum++) { | ||
switch ($colNum % 6) { | ||
default: | ||
$row[] = $colNum; | ||
break; | ||
case 1: | ||
case 3: | ||
$row[] = array($colNum, $styleDef); | ||
break; | ||
case 2: | ||
case 4: | ||
$row[] = array($colNum, $styleRed); | ||
break; | ||
case 5: | ||
$row[] = array(25569 + (time() / 86400), $styleDate); | ||
break; | ||
} | ||
} | ||
|
||
$rowsArr[] = $row; | ||
if (!is_int($groupedRows)) { | ||
$writer->addRow($row); | ||
$rowsArr = array(); | ||
} else if (count($rowsArr) >= $groupedRows) { | ||
$writer->addRows($rowsArr); | ||
$rowsArr = array(); | ||
} | ||
} | ||
|
||
if (count($rowsArr)) { | ||
$writer->addRows($rowsArr); | ||
} | ||
} | ||
|
||
$writer->close(); | ||
|
||
$duration = number_format(microtime(TRUE) - $start, 2); | ||
$str = sprintf(' %s took %s seconds to run (%d r/%d c in %d sheets)', $name, $duration, $rows, $cols, $sheets); | ||
echo $str . PHP_EOL; | ||
}; | ||
|
||
/** | ||
* Bottleneck ATM: Disk usage | ||
*/ | ||
|
||
$start_mem = memory_get_usage(TRUE); | ||
echo PHP_EOL."Memory Consumption is "; | ||
echo round($start_mem/1048576,2).''.' MB'.PHP_EOL; | ||
|
||
$test('1_mini_grouped_00000', 1, 50, 25, FALSE); | ||
|
||
exit (0); | ||
|
||
$cur_mem = memory_get_usage(TRUE); | ||
echo " Current Consumption is "; | ||
echo round($cur_mem/1048576,2).''.' MB'.PHP_EOL; | ||
|
||
$test('2_small_grouped_00000', 10, 7500, 50, FALSE); | ||
|
||
$cur_mem = memory_get_usage(TRUE); | ||
echo " Current Consumption is "; | ||
echo round($cur_mem/1048576,2).''.' MB'.PHP_EOL; | ||
|
||
$test('3_medium_grouped_00000', 10, 7500, 500, FALSE); | ||
|
||
$cur_mem = memory_get_usage(TRUE); | ||
echo " Current Consumption is "; | ||
echo round($cur_mem/1048576,2).''.' MB'.PHP_EOL; | ||
|
||
$test('4_large_grouped_00000', 10, 150000, 10000, FALSE); | ||
|
||
$cur_mem = memory_get_usage(TRUE); | ||
echo " Current Consumption is "; | ||
echo round($cur_mem/1048576,2).''.' MB'.PHP_EOL; | ||
|
||
echo " Peak Consumption is "; | ||
echo round(memory_get_peak_usage(TRUE)/1048576,2).''.' MB'.PHP_EOL; | ||
|
||
exit(0); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are 2 different changes here: alignment and number format. Can you please split these changes into 2 branches? It will reduce the scope of each commit and make the review easier/faster. Thanks!