Skip to content
Snippets Groups Projects

Closes #3488467

Closes #3488467
issue/drupal-3488467:3488467-pp-1-introduce-a into 11.x
1 open thread
Closed
mondrakerequested to merge
issue/drupal-3488467:3488467-pp-1-introduce-a into 11.x
1 open thread

Closes #3488467

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
  • daffie
  • daffie
  • daffie
    daffie @daffie started a thread on the diff
    • core/lib/Drupal/Core/Database/Statement/ResultBase.php 0 → 100644
    1 <?php
    2
    3 declare(strict_types=1);
    4
    5 namespace Drupal\Core\Database\Statement;
    6
    7 use Drupal\Core\Database\FetchModeTrait;
    8
    9 /**
    10 * Base class for results of a data query language (DQL) statement.
    11 */
    12 abstract class ResultBase {
    • daffie
      daffie @daffie
      Developer

      Could you explain to me way ResultBase and StatementBase are 2 separate classes? Can we not move the code from ResultBase into StatementBase?

    • M
      mondrake @mondrake
      Author Contributor

      Well I honestly believe the result of a statement execution should be a separate object from the statement object. That's what mysqli does for instance: https://www.php.net/manual/en/class.mysqli-result.php

      The fact that PDOStatement incapsulate its own results in itself is not a best practice IMHO.

      And, a separate result object for prefetched data can be reused more easily by other Statement implementations without having to rewrite all the methods.

      Also, the new Result classes are not API (StatementInterface is totally untouched), just internal refactoring. But in the future, if necessary/wanted, fetch methods could be moved away from StatementInterface into a new API like other db abstraction layer did (DBAL, for instance). But I do not see need for that ATM.

      Finally, the methods in the new classes are now fully typehinted and that made development easier (for instance avoiding \PDO::FETCH* classes being passed in etc.).

    • Please register or sign in to reply
  • daffie
    daffie @daffie
    Developer

    Looks good. Just a couple of questions.

  • daffie requested changes

    requested changes

  • mondrake added 13 commits

    added 13 commits

    Compare with previous version

  • mondrake added 94 commits

    added 94 commits

    Compare with previous version

  • mondrake added 19 commits

    added 19 commits

    Compare with previous version

  • mondrake added 4 commits

    added 4 commits

    Compare with previous version

  • mondrake added 11 commits

    added 11 commits

    Compare with previous version

  • catch closed

    closed

    • Please register or sign in to reply