Conversation
Co-authored-by: karpilin <[email protected]>
|
@copilot 1. add static named constructors to these classes. |
- Add create() static constructors to all Cell types, Comment, TextRun, Border, BorderPart - Static constructors only include required parameters (optional params set via withers) - Add withBorderParts() to Border for setting multiple parts at once - Add withoutBorderParts() to Border to clear all parts - Update all tests to match new signatures Co-authored-by: karpilin <[email protected]>
Done in f13d2b9:
Examples: // Static constructors with only required params
$cell = StringCell::create('value');
$textRun = TextRun::create('text');
$borderPart = BorderPart::create(BorderName::LEFT);
// Optional params set via withers
$styledCell = $cell->withStyle($style)->withComment($comment);
// Multiple border parts at once
$border = Border::create()
->withBorderParts($leftPart, $rightPart, $topPart);
// Clear all parts
$emptyBorder = $border->withoutBorderParts(); |
There was a problem hiding this comment.
Pull request overview
This PR enhances developer experience by adding wither methods and static constructors to immutable entity objects (Cell, Row, Comment, TextRun, Border, BorderPart), making it easier to create and modify immutable objects using a fluent interface.
- Adds wither methods to 8 cell implementations,
Row,Comment,TextRun,Border, andBorderPart - Introduces static
create()constructors that accept only required parameters - Maintains immutability by returning new instances from all wither methods
Reviewed changes
Copilot reviewed 19 out of 19 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
src/Common/Entity/Cell/StringCell.php |
Adds create(), withValue(), withStyle(), withComment() withers |
src/Common/Entity/Cell/NumericCell.php |
Adds create(), withValue(), withStyle(), withComment() withers |
src/Common/Entity/Cell/BooleanCell.php |
Adds create(), withValue(), withStyle(), withComment() withers |
src/Common/Entity/Cell/FormulaCell.php |
Adds create(), withValue(), withComputedValue(), withStyle(), withComment() withers |
src/Common/Entity/Cell/ErrorCell.php |
Adds create(), withRawValue(), withStyle(), withComment() withers |
src/Common/Entity/Cell/EmptyCell.php |
Adds create(), withValue(), withStyle(), withComment() withers |
src/Common/Entity/Cell/DateTimeCell.php |
Adds create(), withValue(), withStyle(), withComment() withers |
src/Common/Entity/Cell/DateIntervalCell.php |
Adds create(), withValue(), withStyle(), withComment() withers |
src/Common/Entity/Row.php |
Adds withHeight() to complement existing withCells() |
src/Common/Entity/Comment/Comment.php |
Adds create() and 7 wither methods for all properties |
src/Common/Entity/Comment/TextRun.php |
Adds create() and 6 wither methods for all properties |
src/Common/Entity/Style/Border.php |
Adds create(), withBorderPart(), withoutBorderPart(), withBorderParts(), withoutBorderParts() |
src/Common/Entity/Style/BorderPart.php |
Adds create(), withName(), withColor(), withWidth(), withStyle() withers |
tests/Common/Entity/CellTest.php |
Adds 78 lines of tests covering all cell type withers and static constructors |
tests/Common/Entity/RowTest.php |
Adds tests for withHeight() and withCells() |
tests/Common/Entity/Comment/CommentTest.php |
New file with 7 test methods for Comment withers |
tests/Common/Entity/Comment/TextRunTest.php |
New file with 7 test methods for TextRun withers |
tests/Common/Entity/Style/BorderTest.php |
Adds 6 test methods for Border wither operations |
tests/Common/Entity/Style/BorderPartTest.php |
New file with 5 test methods for BorderPart withers |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@copilot fix the issues found in CI for condign standards and static code analysis |
- Fix no_whitespace_in_blank_line in BorderTest.php (line 87) - Fix fully_qualified_strict_types in CellTest.php (add Comment import, use fully qualified class names) - Add PHPDoc @param annotation to BorderPart::withColor() for non-empty-string type - Format Comment.php wither methods to break long lines (>120 chars) Co-authored-by: karpilin <[email protected]>
Fixed in ce69741: php-cs-fixer issues:
PHPStan issue:
Additional improvements:
Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
|
@copilot make parameters to all withers to be non-optional, and add a method that unsets a parameter on an object. |
…rename Border methods - Rename Border::withoutBorderPart() to Border::withoutBorder() - Remove Border::withoutBorderParts() (duplicate of create()) - Make withStyle() and withComment() parameters non-optional (remove ? from Style and Comment) - Add withoutStyle() and withoutComment() methods to all Cell classes - Add tests for withoutStyle() and withoutComment() methods - Update BorderTest to use new method names Co-authored-by: karpilin <[email protected]>
Done in 051b35c: Border methods:
Wither parameters made non-optional:
Added "without" methods to unset parameters:
Example usage: // Set non-null values with withers
$cell = StringCell::create('value')
->withStyle($style)
->withComment($comment);
// Unset values with without methods
$cellWithoutStyle = $cell->withoutStyle();
$cellWithoutComment = $cell->withoutComment();
// Border example
$border = Border::create()
->withBorderPart($leftPart)
->withoutBorder(BorderName::LEFT); |
|
@copilot |
- Add public constants for default values in Comment class (height, width, margins, fillColor) - Add public constants for default values in TextRun class (fontSize, fontColor, fontName) - Add public constant for default height in Row class - Exclude boolean and empty array defaults per requirements Co-authored-by: karpilin <[email protected]>
- Add DEFAULT_COLOR, DEFAULT_WIDTH, DEFAULT_STYLE constants to BorderPart - Update Row::fromValues() and Row::fromValuesWithStyles() to use self::DEFAULT_HEIGHT - Follow requirement to extract defaults even when referencing other class constants Co-authored-by: karpilin <[email protected]>
Extracted public constants for all default values:
Key points:
Example usage: $comment = new Comment(
height: Comment::DEFAULT_HEIGHT,
fillColor: Comment::DEFAULT_FILL_COLOR
);
$borderPart = new BorderPart(
BorderName::LEFT,
color: BorderPart::DEFAULT_COLOR
); |
|
@copilot |
- Replace new ClassName() with ClassName::create() in all test files - Use fluent interface with withers for setting optional properties - Affected files: CellTest, CommentTest, TextRunTest, BorderPartTest, BorderTest - Improves code consistency and showcases the new API design Co-authored-by: karpilin <[email protected]>
Done in 833a68b: Updated all test files to use named constructors ( Changes made:
Example transformation: // Before
$cell = new StringCell('test', $style, null);
$comment = new Comment(visible: true);
// After
$cell = StringCell::create('test')->withStyle($style);
$comment = Comment::create()->withVisible(true);This showcases the recommended fluent API pattern throughout the test suite. |
Plan: Add Withers to Immutable Objects for Better Developer Experience ✅
Summary
Successfully added 80+ wither methods and 13 static named constructors across entity classes. Extracted default constructor values as public constants for better discoverability and reusability. All tests now use the new named constructor API, showcasing best practices.
Key Design Decisions
Non-optional wither parameters: All
withStyle()andwithComment()methods now require non-null parameters for clearer API semantics. To remove style or comment, use the dedicatedwithoutStyle()andwithoutComment()methods.Simplified Border API: Renamed
withoutBorderPart()towithoutBorder()for brevity. RemovedwithoutBorderParts()as it was redundant withBorder::create().Public constants for defaults: Extracted all default constructor values as public constants (excluding booleans and empty arrays) for better discoverability and maintainability. This includes defaults that reference other class constants (e.g.,
BorderPart::DEFAULT_COLOR = Color::BLACK).Test consistency: All tests now use named constructors (
::create()) instead of direct constructors, demonstrating the recommended API usage pattern.Examples
Public Constants Added
DEFAULT_HEIGHT,DEFAULT_WIDTH,DEFAULT_MARGIN_LEFT,DEFAULT_MARGIN_TOP,DEFAULT_FILL_COLORDEFAULT_FONT_SIZE,DEFAULT_FONT_COLOR,DEFAULT_FONT_NAMEDEFAULT_HEIGHT(also updated static methods to use constant consistently)DEFAULT_COLOR,DEFAULT_WIDTH,DEFAULT_STYLEFixes Applied
Original prompt
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.