search

FelixBaensch / MORTAR

MOlecule fRagmenTAtion fRamework
MIT License
18 stars 3 forks source link

Collection util rework #53

Closed FelixBaensch closed 5 months ago

JonasSchaub commented 5 months ago

How about defining ascending/descending via a bool? This way, no JavaFx class would be used in this model class. Or can there be other sort orders, hypothetically?

JonasSchaub commented 5 months ago

The enum values (or their text representations) should then also be used in the GUI classes that define the cell value properties, right? Are these displayed and hence language-specific?

FelixBaensch commented 5 months ago

The enum values (or their text representations) should then also be used in the GUI classes that define the cell value properties, right?

Done

Are these displayed and hence language-specific?

No, they are not

JonasSchaub commented 5 months ago

Thank you, looks much better now! I'm just still hung up on this:

How about defining ascending/descending via a bool? This way, no JavaFx class would be used in this model class. Or can there be other sort orders, hypothetically?

And the TableColumn.SortType enum appears to only have the two values ascending and descending anyway.

FelixBaensch commented 5 months ago

Thank you, looks much better now! I'm just still hung up on this:

How about defining ascending/descending via a bool? This way, no JavaFx class would be used in this model class. Or can there be other sort orders, hypothetically?

And the TableColumn.SortType enum appears to only have the two values ascending and descending anyway.

Unfortunately I overlooked that. That would require a method to parse the TableColumn.SortType into a Boolean. I would rather argue in favor of moving the CollectionUtils to a controller package if you want to avoid the fx class in the model area.

JonasSchaub commented 5 months ago

That would require a method to parse the TableColumn.SortType into a Boolean.

It wouldn't be that complicated, would it? I'd name the boolean parameter "ascending" and the method call would be like "tmpSortType.equals(ASCENDING)". If false, the used order is descending. Am I missing something?

FelixBaensch commented 5 months ago

If it makes you happy

JonasSchaub commented 5 months ago

If it makes you happy

It does 🤗

sonarcloud[bot] commented 5 months ago

Quality Gate Passed Quality Gate passed

Issues
2 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

JonasSchaub commented 5 months ago

Do you want to merge this or should I?