starred/jellyfin

Fork 0

mirror of https://github.com/jellyfin/jellyfin.git synced 2026-01-23 23:20:51 +01:00

Add constructor parameters for optional entity properties #1698

New issue

Open

opened 2025-12-21 19:18:03 +01:00 by backuprepo · 4 comments

backuprepo commented

2025-12-21 19:18:03 +01:00

Owner

Originally created by @mark-monteiro on GitHub (May 15, 2020).

As discussed here , we should consider updating the new EF models to include all properties as constructor parameters, instead of only the required properties as is now.

This would prevent having to instantiate new instances like this, which is a bit ugly:

new Entity(
    requiredProp: requiredPropValue,
    anotherRequiredProp: anotherRequiredPropValue)
{
    OptionalProp = optionalValue
}

and instead you could write this:

new Entity(
    requiredProp: requiredPropValue,
    anotherRequiredProp: anotherRequiredPropValue,
    optionalProp: optionalValue);

Adding the discussion needed tag to get some feedback on whether this is a desired change.

Originally created by @mark-monteiro on GitHub (May 15, 2020). As discussed [here](https://github.com/jellyfin/jellyfin/pull/2970#discussion_r424871196) , we should consider updating the new EF models to include all properties as constructor parameters, instead of only the required properties as is now. This would prevent having to instantiate new instances like this, which is a bit ugly: ```cs new Entity( requiredProp: requiredPropValue, anotherRequiredProp: anotherRequiredPropValue) { OptionalProp = optionalValue } ``` and instead you could write this: ```cs new Entity( requiredProp: requiredPropValue, anotherRequiredProp: anotherRequiredPropValue, optionalProp: optionalValue); ``` Adding the `discussion needed` tag to get some feedback on whether this is a desired change.

backuprepo added the

enhancement

discussion needed

labels 2025-12-21 19:18:03 +01:00

backuprepo commented

2025-12-21 19:18:05 +01:00

Author

Owner

@barronpm commented on GitHub (May 20, 2020):

I'm against this, because for the more complicated entities, we would need over 20 optional parameters in a constructor, which I don't think would be ideal. Instead, I think we should try to limit constructor parameters to what is both required and has no default value.

@barronpm commented on GitHub (May 20, 2020): I'm against this, because for the more complicated entities, we would need over 20 optional parameters in a constructor, which I don't think would be ideal. Instead, I think we should try to limit constructor parameters to what is both required and has no default value.

backuprepo commented

2025-12-21 19:18:06 +01:00

Author

Owner

@barronpm commented on GitHub (Jun 4, 2020):

This I think will be a good solution when we move to .NET 5: https://devblogs.microsoft.com/dotnet/welcome-to-c-9-0/#init-only-properties

@barronpm commented on GitHub (Jun 4, 2020): This I think will be a good solution when we move to .NET 5: https://devblogs.microsoft.com/dotnet/welcome-to-c-9-0/#init-only-properties

backuprepo commented

2025-12-21 19:18:08 +01:00

Author

Owner

@edrohler commented on GitHub (Jun 4, 2021):

Hi all. Is this still being discussed? I think it has overlap with 3122.

@edrohler commented on GitHub (Jun 4, 2021): Hi all. Is this still being discussed? I think it has overlap with [3122](https://github.com/jellyfin/jellyfin/issues/3122).

backuprepo commented

2025-12-21 19:18:10 +01:00

Author

Owner

@mark-monteiro commented on GitHub (Jun 14, 2021):

I don't think any relevant changes have been made so yeah, I think this is still up for discussion.

Now that we're using .NET 5, I think @barronpm is right in that init-only properties is the way to go.

@mark-monteiro commented on GitHub (Jun 14, 2021): I don't think any relevant changes have been made so yeah, I think this is still up for discussion. Now that we're using .NET 5, I think @barronpm is right in that init-only properties is the way to go.

backuprepo referenced this issue

2025-12-22 07:03:16 +01:00

[PR #1709] [MERGED] Fix crash due to missing userRootFolder data #8687

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: starred/jellyfin#1698

No description provided.

Rows
Columns