Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Type hinting #43

Merged
merged 13 commits into from
Nov 24, 2023
Merged

Type hinting #43

merged 13 commits into from
Nov 24, 2023

Conversation

schorschy
Copy link
Contributor

No description provided.

@schorschy schorschy requested a review from alxndr-w November 21, 2023 10:47
Copy link
Member

@alxndr-w alxndr-w left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@schorschy hier müsstest du nochmal über einige Stellen rüber. Bei vielen, bei denen :object steht, gehört eigentlich eine Collection hin.

Comment on lines -12 to 15
public function getMedia()
public function getMedia(): rex_media
{
return rex_media::get($this->image);
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Kannst du hier noch als Weiche einbauen - wenn media_manager_responsive installiert ist, dann rex_media_plus-Objekt zurückgeben.

Comment on lines 30 to 33
public function getDateWhere($whereRaw = ''): object
{
return event_date::query()->joinRelation('event_category_id', 'c')->where('c.id', $this->getId())->whereRaw($whereRaw)->orderBy('startDate`, `startTime', 'DESC')->find();
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
public function getDateWhere($whereRaw = ''): object
{
return event_date::query()->joinRelation('event_category_id', 'c')->where('c.id', $this->getId())->whereRaw($whereRaw)->orderBy('startDate`, `startTime', 'DESC')->find();
}
public function getDateWhere($whereRaw = ''): ?rex_yform_manager_collection
{
return event_date::query()->joinRelation('event_category_id', 'c')->where('c.id', $this->getId())->whereRaw($whereRaw)->orderBy('startDate`, `startTime', 'DESC')->find();
}

Comment on lines +35 to 38
public function getRelatedDates($whereRaw = ''): array
{
return $this->getDateWhere($whereRaw);
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
public function getRelatedDates($whereRaw = ''): array
{
return $this->getDateWhere($whereRaw);
}
public function getRelatedDates($whereRaw = ''): ?rex_yform_manager_collection
{
return $this->getDateWhere($whereRaw);
}

Comment on lines +44 to 47
public function hasAttribute($needle): bool
{
return in_array($needle, $this->getAttributes());
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
public function hasAttribute($needle): bool
{
return in_array($needle, $this->getAttributes());
}
public function hasAttribute($needle): bool
{
return in_array($needle, $this->getAttributes(), true);
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bool kommt nur raus, wenn in_array den 3. Parameter $strict = true hat

@@ -13,18 +13,19 @@ public static function generateuuid($id = null) :string
return uuid::uuid3(uuid::NAMESPACE_URL, $id);
}

public function getCategory()
public function getCategory(): object
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
public function getCategory(): object
public function getCategory(): ?event_category

{
if (!$uuid) {
return;
return null;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
return null;
return;

Reicht so.

Comment on lines 62 to 66
public function getRegistration(): object
{
return $this->getRelatedDataset('registration_id');
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bitte korrigieren

Comment on lines 77 to 83
public static function getByUuid($uuid = null): object
{
if (!$uuid) {
return;
}
return self::query()->where('uuid', $uuid)->findOne();
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bitte korrigieren

Comment on lines 84 to 90
public static function getByHash($hash = null): object
{
if (!$hash) {
return;
}
return self::query()->where('hash', $hash)->findOne();
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bitte korrigieren

Comment on lines +91 to 94
public static function ep_saved($ep): bool
{
$lastId = $ep->getSubject()->getLastId();
$table = $ep->getParam('table');
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
public static function ep_saved($ep): bool
{
$lastId = $ep->getSubject()->getLastId();
$table = $ep->getParam('table');
public static function ep_saved(rex_extension_point $ep): bool
{
$lastId = $ep->getSubject()->getLastId();
$table = $ep->getParam('table');

@schorschy
Copy link
Contributor Author

Ich hab neue commits gepusht, aber kann den pull-request nicht erneut auslösen oder dich irgendwie drauf hinweisen dass es erledigt wurde außer per Kommentar :D kannst mir hierzu ja in der nächsten besprechung den best practice mitteilen.

@alxndr-w alxndr-w merged commit 05b55ba into main Nov 24, 2023
1 check failed
@alxndr-w alxndr-w deleted the Type-Hinting branch November 24, 2023 12:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants