Project

General

Profile

Actions

Task #99451

open

PHPDoc, name of arguments and documentation not very intuitive for ExpressionBuilder::inSet

Added by Sybille Peters over 1 year ago. Updated over 1 year ago.

Status:
New
Priority:
Should have
Assignee:
-
Category:
Code Cleanup
Target version:
-
Start date:
2023-01-03
Due date:
% Done:

0%

Estimated time:
TYPO3 Version:
11
PHP Version:
Tags:
Complexity:
Sprint Focus:

Description

Problems

1. The order is reversed: the values in ->inSet which are passed to FIND_IN_SET are reversed
2. argument names do not clearly reflect what is needle, what is haystack, what is the comma separated list
3. PHPDoc is vague, possibly also misleading:

$fieldName The field name.
string $value Argument to be used in FIND_IN_SET() comparison.

4. Possibly it might be good ideat to rename the function name (or create a new one and deprecate the old) as it is not a simple FIND_IN_SET, it is a FIND_IN_SET where the haystack refers to a database field name which contains a comma separated list of values

Suggestions

To mitigate this, I think at least the following should be done:

- function argument names should be named in a way which makes it intuitive. For example, use names such as "needle" and "haystack" or "pattern" and "fieldList" as is customary in PHP and / or SQL.
- make PHPDoc more specific if helpful in addition to argument names. Remove explanations which only repeat the name of the argumen. E.g. "field name" as explanation for fieldName is unnecessary fluff.
- possibly improve documentation: https://docs.typo3.org/m/typo3/reference-coreapi/main/en-us/ApiOverview/Database/ExpressionBuilder/Index.html#comparisons. Apply same principles here.


Related issues 1 (1 open0 closed)

Related to TYPO3 Core - Bug #83074: The inSet (FIND_IN_SET) function generates invalid SQLAccepted2017-11-23

Actions
Actions #1

Updated by Sybille Peters over 1 year ago

  • Related to Bug #83074: The inSet (FIND_IN_SET) function generates invalid SQL added
Actions #2

Updated by Sybille Peters over 1 year ago

  • Tracker changed from Feature to Task
  • TYPO3 Version set to 11
Actions #3

Updated by Sybille Peters over 1 year ago

See explanation by Alexander Opitz:

https://forge.typo3.org/issues/83074#note-4

As Morton already wrote, you are using the function wrong.

The two parameters of inSet which you are using inSet(string $fieldName, string $value, ...) are defined:
$fieldName ... The name of the field, which contains a comma separated string/set which will be searched for.
$value ... The element which is searched inside this set (This string SHOULD NOT contain commas).

As you are using a NamedParameter you do not get an exception while calling the inSet method as the NamedParameter can't be quantified at that moment.

You should use the in() method which is defined as:

$fieldName The field to search in
$value ... The comma separated string/set( or array which will be imploded) for which will be searched inside the given field.

Actions #4

Updated by Sybille Peters over 1 year ago

  • Description updated (diff)
Actions #5

Updated by Sybille Peters over 1 year ago

  • Description updated (diff)
Actions

Also available in: Atom PDF