Project

General

Profile

Actions

Task #95722

open

Why does TYPO3 have a slug-helper?

Added by Rozbeh Chiryai Sharahi over 2 years ago. Updated 17 days ago.

Status:
Under Review
Priority:
Could have
Assignee:
-
Category:
Code Cleanup
Target version:
-
Start date:
2021-10-21
Due date:
% Done:

0%

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

Description

Hello Community,

this is just to have a general understanding of a pattern on TYPO3 code, which I do not really understand.

Since TYPO3 9 we have inhouse pretty-urls. For this, as usual these days, we have fortunately slug fields on the entities. Thank you for all this.

TYPO3 is a very mature system and it is comprehensible that we have some legacy codes here and there that look weird from today's perspective.

Now looking at the slug-helper I kind of get lost, why these kind of code-styles are still introduced on TYPO3. The slug-helper cannot be that old and at the same time TYPO3 is now enforcing dependency injection with non-public services. Therefore accessing containers and GeneralUtility::makeInstance is advised not to be done.

Well with a Helper like this it will be quite hard to obey these conventions:

$slugHelper = GeneralUtility::makeInstance(
    SlugHelper::class,
    $tableName,
    $slugFieldName,
    $fieldConfig
);

$slugHelper->generate($hereSomeArray, int $pid);
...

This way constructor injection is very hard up to not possible imho. Also it is not very intuitive to use. Why do we introduce a slug-helper in this pattern?

Imo the right way to do this would be for instance something like this:

$slugService = new SlugService();
$slugService->getGenerator($table, $fieldName)->generateByUid(int $uid)
// or
$slugService->generate($table, $fieldname, $uid)
// or 
$slugService->generate(SlugGenerateConfigurationObject)
// or
...

This is just one of many possible patterns, nevertheless this way we could

- use enforced constructor injection
- dismiss usage of general utitlity
- write tests easily by simple injection instead of registering instances on containers, ...
- understand usage simply by auto-complete

I have that feeling, that there should be some code patterns which are enforced on TYPO3 Core Code. I have this feeling, that in 5 years there will be a strong dependency on this slug-helper and there will be soon a lot of work to deprecate them.

Nevertheless I don't see exactly what kind of recommended common pattern the current implementation is following.

Please correct me with literature if I am wrong and this pattern has some advantages.

Thank you for all your great work.

Kind regards
Rozbeh Chiryai Sharahi

Actions #1

Updated by Oliver Hader over 2 years ago

Rozbeh Chiryai Sharahi wrote:

Nevertheless I don't see exactly what kind of recommended common pattern the current implementation is following.

SlugHelper does not follow any pattern by the book. And (spoiler alert), that's unfortunately not the only class in TYPO3 that mixes multiple patterns and approaches in one component - e.g. have a look into DataHandler.

So, please go ahead with implementing current SlugHelper as stateless SlugService and I'll happily look into it and review it.

Personally I'd prefer $slugService->generate(SlugGenerateConfiguration) as it seems to be as closed as possible, but still open and flexible to future changes in the SlugGenerateConfiguration DTO, e.g. adding additional properties.

Actions #2

Updated by Benni Mack over 2 years ago

  • Status changed from New to Closed

Hey Rozbeh Chiryai Sharahi,

this is all my fault, I should not have created the feature in the first place.

I can come up with excuses why I build it this in 2019 way - simply put I took the liberty of making it work "somehow" instead of coming up with a pattern due to time constraints. But now I know better and would build it differently - using all the concepts by the book. Now we have to stick with this code until someone will come up with a better alternative.

If you find the time, I'm more than happy if you can improve the concepts in this area.

I hope this answers your initial question. If you have any further questions, feel free to reach out to me directly at

All the best,
Benni.

Actions #3

Updated by Gerrit Code Review almost 2 years ago

  • Status changed from Closed to Under Review

Patch set 1 for branch main of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/74781

Actions #4

Updated by Gerrit Code Review almost 2 years ago

Patch set 2 for branch main of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/74781

Actions #5

Updated by Gerrit Code Review almost 2 years ago

Patch set 3 for branch main of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/74781

Actions #6

Updated by Gerrit Code Review almost 2 years ago

Patch set 4 for branch main of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/74781

Actions #7

Updated by Gerrit Code Review almost 2 years ago

Patch set 5 for branch main of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/74781

Actions #8

Updated by Gerrit Code Review almost 2 years ago

Patch set 6 for branch main of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/74781

Actions #9

Updated by Gerrit Code Review almost 2 years ago

Patch set 7 for branch main of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/74781

Actions #10

Updated by Gerrit Code Review almost 2 years ago

Patch set 8 for branch main of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/74781

Actions #11

Updated by Gerrit Code Review almost 2 years ago

Patch set 9 for branch main of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/74781

Actions #12

Updated by Gerrit Code Review almost 2 years ago

Patch set 10 for branch main of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/74781

Actions #13

Updated by Gerrit Code Review almost 2 years ago

Patch set 11 for branch main of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/74781

Actions #14

Updated by Gerrit Code Review almost 2 years ago

Patch set 12 for branch main of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/74781

Actions #15

Updated by Gerrit Code Review almost 2 years ago

Patch set 13 for branch main of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/74781

Actions #16

Updated by Gerrit Code Review almost 2 years ago

Patch set 14 for branch main of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/74781

Actions #17

Updated by Gerrit Code Review almost 2 years ago

Patch set 15 for branch main of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/74781

Actions #18

Updated by Gerrit Code Review almost 2 years ago

Patch set 16 for branch main of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/74781

Actions #19

Updated by Gerrit Code Review almost 2 years ago

Patch set 17 for branch main of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/74781

Actions #20

Updated by Gerrit Code Review almost 2 years ago

Patch set 18 for branch main of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/74781

Actions #21

Updated by Gerrit Code Review almost 2 years ago

Patch set 19 for branch main of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/74781

Actions #22

Updated by Gerrit Code Review almost 2 years ago

Patch set 20 for branch main of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/74781

Actions #23

Updated by Gerrit Code Review almost 2 years ago

Patch set 21 for branch main of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/74781

Actions #24

Updated by Rozbeh Chiryai Sharahi almost 2 years ago

  • Priority changed from Should have to Could have

Hello Benni Mack, Hello Oliver Hader,

Thank you for your replies to the topic. Late ... but ... I took the time to do my contribution.

Of course I learned that it was much much more work than expected, and looking at my initial message from today's perspective ... I was arrogant, not being aware of the amount of details considered behind a simple slug builder :D. I don't want to imagine doing this with time constraints.

However, i pushed a possible approach and maybe it or some parts of it are useful.

Kind Regards
Rozbeh Chiryai Sharahi

Actions #25

Updated by Gerrit Code Review almost 2 years ago

Patch set 22 for branch main of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/74781

Actions #26

Updated by Gerrit Code Review almost 2 years ago

Patch set 23 for branch main of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/74781

Actions #27

Updated by Gerrit Code Review almost 2 years ago

Patch set 24 for branch main of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/74781

Actions #28

Updated by Gerrit Code Review almost 2 years ago

Patch set 25 for branch main of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/74781

Actions #29

Updated by Gerrit Code Review almost 2 years ago

Patch set 26 for branch main of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/74781

Actions #30

Updated by Gerrit Code Review almost 2 years ago

Patch set 27 for branch main of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/74781

Actions #31

Updated by Gerrit Code Review almost 2 years ago

Patch set 28 for branch main of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/74781

Actions #32

Updated by Gerrit Code Review almost 2 years ago

Patch set 29 for branch main of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/74781

Actions #33

Updated by Gerrit Code Review almost 2 years ago

Patch set 30 for branch main of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/74781

Actions #34

Updated by Gerrit Code Review almost 2 years ago

Patch set 31 for branch main of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/74781

Actions #35

Updated by Gerrit Code Review almost 2 years ago

Patch set 32 for branch main of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/74781

Actions #36

Updated by Gerrit Code Review almost 2 years ago

Patch set 33 for branch main of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/74781

Actions #37

Updated by Gerrit Code Review almost 2 years ago

Patch set 34 for branch main of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/74781

Actions #38

Updated by Gerrit Code Review almost 2 years ago

Patch set 35 for branch main of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/74781

Actions #39

Updated by Gerrit Code Review 10 months ago

Patch set 36 for branch main of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/74781

Actions #40

Updated by Gerrit Code Review 24 days ago

Patch set 37 for branch main of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/74781

Actions #41

Updated by Gerrit Code Review 23 days ago

Patch set 38 for branch main of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/74781

Actions #42

Updated by Gerrit Code Review 22 days ago

Patch set 39 for branch main of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/74781

Actions #43

Updated by Gerrit Code Review 22 days ago

Patch set 40 for branch main of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/74781

Actions #44

Updated by Gerrit Code Review 22 days ago

Patch set 41 for branch main of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/74781

Actions #45

Updated by Gerrit Code Review 22 days ago

Patch set 42 for branch main of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/74781

Actions #46

Updated by Gerrit Code Review 22 days ago

Patch set 43 for branch main of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/74781

Actions #47

Updated by Gerrit Code Review 22 days ago

Patch set 44 for branch main of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/74781

Actions #48

Updated by Gerrit Code Review 22 days ago

Patch set 45 for branch main of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/74781

Actions #49

Updated by Gerrit Code Review 22 days ago

Patch set 46 for branch main of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/74781

Actions #50

Updated by Gerrit Code Review 22 days ago

Patch set 47 for branch main of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/74781

Actions #51

Updated by Gerrit Code Review 22 days ago

Patch set 48 for branch main of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/74781

Actions #52

Updated by Gerrit Code Review 21 days ago

Patch set 49 for branch main of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/74781

Actions #53

Updated by Gerrit Code Review 21 days ago

Patch set 50 for branch main of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/74781

Actions #54

Updated by Gerrit Code Review 18 days ago

Patch set 51 for branch main of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/74781

Actions #55

Updated by Gerrit Code Review 18 days ago

Patch set 52 for branch main of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/74781

Actions #56

Updated by Gerrit Code Review 18 days ago

Patch set 53 for branch main of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/74781

Actions #57

Updated by Gerrit Code Review 18 days ago

Patch set 54 for branch main of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/74781

Actions #58

Updated by Gerrit Code Review 17 days ago

Patch set 55 for branch main of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/74781

Actions #59

Updated by Gerrit Code Review 17 days ago

Patch set 56 for branch main of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/74781

Actions #60

Updated by Gerrit Code Review 17 days ago

Patch set 57 for branch main of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/74781

Actions #61

Updated by Gerrit Code Review 17 days ago

Patch set 58 for branch main of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/74781

Actions #62

Updated by Gerrit Code Review 17 days ago

Patch set 59 for branch main of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/74781

Actions

Also available in: Atom PDF