~adj/comment-test-tracker#149: 
Improve Predicate Style (req for "or" filtering)

Currently, the Predicates for filtering techs/units uses a very Java 6/7 type of style. It basically is one method that's hundreds of lines long and has lots of repetition; it also doesn't take advantage of being able to chain predicates. Along with being more modern, chaining has a key advantage - it can use things like "or" and "not", which would allow more flexible queries. I've built in != in places, but "or" is not possible at all right now.

Initially I'd hoped to put all the tiny-predicate definitions in one place, but after playing around with it tonight, that probably isn't possible, because in many cases we aren't simply testing a property of the TECH, but the result depends on other parameters from the BIQ - for example, the era="Middle Ages" query requires both the era name, and the era list as parameters. It's thus a TriPredicate, but even if BiPredicate were sufficient, interoperability with FilterableList's plain-old Predicate would be a problem. You can't mix-and-match in the chains, or at least I haven't found a way to.

Perhaps this could be worked around with a custom FilterableList. Ignoring for a moment that it's Final, I see two options:

  • If you could mix and match chains, it could take a list of predicates-like-objects (mono/bi/tri), and apply() each in turn... although "or" might be a challenge here. Ideally you'd have a parent interface for Predicate that would allow chaining across bi/tri predicates as well.
  • Alternatively, have it take a chainable TriPredicate, where it would pass in the TECH, the query (String), and the IO (BIQ, for any cross-referencing needed). Actually, couldn't we get the latter from Main - whichever one is currently active? Then maybe it could be shrunk to a BiPredicate.

Really, I suspect what I really want, and might eventually add, is a composite filterable list + filter object, which uses a BiPredicate instead of a regular one. That would solve a lot of the interoperability issues.

Status
REPORTED
Submitter
bitbucket:QuintillusCFC
Assigned to
No-one
Submitted
6 years ago
Updated
4 years ago
Labels
No labels applied.

~adj 4 years ago

Cleaned up a decent chunk of the tech one to use chaining and be more modern - and more compact and readable at the same time. It's really easy to add one that is a boolean property on the TECH object now!

But ultimately, ran into needing BiPredicate for things like cost>50, where there is fundamentally a second parameter to be evaluated that can't simply be a boolean method or the negation thereof. Making the code truly elegant won't be possible without a BiPredicate refactor.

~adj 4 years ago

Done for 1.27.

Register here or Log in to comment, or comment via email.