[go: up one dir, main page]

Page MenuHomePhabricator

[Spike] Investigate fluent builder pattern for Codex PHP
Closed, ResolvedPublicSpike

Description

Description:
This task aims to explore and investigate the implementation of the fluent builder pattern in Codex PHP, focusing on balancing flexibility, clarity, and best practices for generating markup.

Background

As discussed in T373708, there are several considerations when implementing the fluent builder pattern in Codex PHP. These considerations include:

  • Separating object construction (via build()) from rendering (via getHtml() or render()).
  • Combining these steps into a simpler method chain for convenience, potentially merging build() and render().
  • Evaluating the use of implicit __toString() methods for output versus explicit method calls for clarity and maintainability.

The focus of this SPIKE is to determine the most efficient and developer-friendly way to implement the builder pattern, ensuring that the final API is clean, maintainable, and easy for PHP developers to use.

Goal

The goal of this SPIKE task is to evaluate different approaches to the builder pattern in Codex PHP. This will inform a final decision on the best approach to take for a clean and maintainable API that is user-friendly for PHP developers.

Investigation Areas
  1. Builder Approaches to Evaluate:
  • Approach 1: Keep build() and getHtml() separate, where build() constructs the immutable object and getHtml() returns its rendered output.
$infoChip = $codex->InfoChip()->setText( 'Info Chip' )->build()->getHtml(); // `getHtml()` can be renamed to any other descriptive method name.
  • Approach 2: Introduce a combined render() method that both constructs the immutable object and returns its rendered output.
$infoChip = $codex->InfoChip()->setText( 'Info Chip' )->render();
  • Approach 3: Introduce a combined build() method that both constructs the immutable object and returns its rendered output.
$infoChip = $codex->InfoChip()->setText( 'Info Chip' )->build();
  • Approach 4: Introduce a combined getHtml() method that both constructs the immutable object and returns its rendered output.
$infoChip = $codex->InfoChip()->setText( 'Info Chip' )->getHtml();
  • Approach 5: Use __toString() to implicitly return the HTML when constructing simpler objects like InfoChip.
$infoChip = $codex->InfoChip()->setText( 'Info Chip' );  // `__toString()` will implicitly return the HTML.
  1. Usability and Flexibility:
    • Compare the flexibility of separating object construction and rendering versus streamlining the process into a single method call.
Deliverables
  • An analysis of the different approaches based on ease of use for PHP developers.
  • Create an ADR to document the decision and reasoning behind the chosen approach.
Acceptance criteria
  • A clear recommendation on the best approach for implementing the fluent builder pattern in Codex PHP.
  • Documentation of the reasons for the chosen approach, with a focus on PHP developer experience and code maintainability.

Deadline:

This task must be completed within one week, if acceptable.

Event Timeline

Restricted Application changed the subtype of this task from "Task" to "Spike". · View Herald TranscriptTue, Sep 24, 12:31 AM
Restricted Application added a project: Design-System-Team. · View Herald Transcript
Restricted Application added a subscriber: Aklapper. · View Herald Transcript

I like approach 4 the best. getHtml() is a precise description of what it's doing (more precise than build() and render() which seem a bit vague). And I like the idea of combining ->build()->getHtml() into one method. $codex->InfoChip()->thing1()->thing2()-thing3()-finishItOffWithOneMethod() seems like a natural way to do this.

Approach 5 is my second choice.

Any of the approaches are tolerable. We don't want to spend too much time bikeshedding. But those are my thoughts :)

Dogu updated the task description. (Show Details)

Aside from bikeshedding around the method names, there seem to be two questions here:

  • should the builder return a widget object, or a HTML string?
  • if it returns a widget object, should that automatically convert to HTML when needed (via __toString()) or should that require an explicit function call?

IMO widget objects would 1) allow better type hinting (not big deal but why not) 2) make it easier to decide when to escape (you could have methods which take either a string or a widget, and escape strings). That's better both to avoid mistakes and to support automated XSS detection. And for XSS detection, it's probably better to have an explicit getHtml() method? (Although I'm not that familiar with the phan taint-checker plugin so maybe it has no problem handling automatic string conversion.)

I believe @Tgr raised a very important point that I had initially overlooked. Having the builder return a widget object is definitely a better approach. I also support the idea of a mutable builder and an immutable widget structure as the most suitable solution. I’m aware of the potential bikeshedding in this discussion, but with the many builder pattern suggestions, I felt a bit confused, so I wanted to bring this discussion forward for further clarity.

We're going to proceed with the current approach (builders that return immutable widgets which can be further passed around, combined with a dedicated getHtml() method) and see how it works.

The Codex PHP codebase is currently versioned at 0.1.0, indicating pre-release status; we can get a feel for how this pattern works out and make further changes if needed.

@Dogu feel free to close this task if you are satisfied by the discussion, and thanks to everyone for your input.

Dogu claimed this task.