search

nextcloud / tables

🍱 Nextcloud tables app
https://apps.nextcloud.com/apps/tables
GNU Affero General Public License v3.0
143 stars 24 forks source link

Refactor column creation function #1236

Closed luka-nextcloud closed 3 weeks ago

luka-nextcloud commented 2 months ago

Is your feature request related to a problem? Please describe.

Refactor column creation function to avoid passing many arguments

Describe the solution you'd like

No response

Describe alternatives you've considered

No response

Additional context

No response

blizzz commented 2 months ago

Stems from this comment https://github.com/nextcloud/tables/pull/944#pullrequestreview-2193607823

First, I agree, there's too many arguments there. Perhaps just reduce this to the very necessary ones, and all optional could be set via dedicated setters on the result object.

But we would have to call all of them anyway, wouldn't we? At least in that case. Otherwise names arguments can be used already (for immediate cure).

Anyways, the methods need an overhaul.

luka-nextcloud commented 1 month ago

@juliushaertl @blizzz I can see that function update (https://github.com/nextcloud/tables/blob/main/lib/Service/ColumnService.php#L345) also has many arguments. Should we refactor this function as well?

juliushaertl commented 1 month ago

Ideally yes, maybe we can come up with a DTO class to contain the column definition that we can pass along instead in other places as well