Skip to content

PredicateControllerDelegate#3

Open
richardpiazza wants to merge 2 commits into
dezinezync:mainfrom
richardpiazza:feature/predicate-controller-delegate
Open

PredicateControllerDelegate#3
richardpiazza wants to merge 2 commits into
dezinezync:mainfrom
richardpiazza:feature/predicate-controller-delegate

Conversation

@richardpiazza

@richardpiazza richardpiazza commented May 9, 2026

Copy link
Copy Markdown
Contributor

I noticed the comments in PredicateController

// @todo: Refactor to call delegate

So, I thought this might be a good task to become more familiar with the code. I created the PredicateControllerDelegate protocol with methods to match the Notification Center Notifications, and called those in parallel with firing the notifications.

I also corrected a couple spelling mistakes in the comments.

/// Notified when the predicate of the `UIPredicateEditor`will change.
///
/// The `object` on the `Notification` will be the editor.
@available(*, deprecated, message: "Utilize `PredicateControllerDelegate`")

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I made the assumption that you wanted to switch to a delegate in place of using Notifications. I can easily remove the deprecation if that wasn't the intent.

Comment on lines +292 to +293
func notifyPredicateWillChange() {
delegate?.predicateWillChangeForPredicateController(self)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Since the class was already @MainActor isolated, (and the delegate is also @MainActor annotated), it didn't seem necessary to repeat the annotations here and use the detached Task { @MainActor in ... } in the updatePredicate(for:) methods above.

I'm still coming up to speed on the Swift 6 concurrency changes, so I might be missing something.

// We assume the first row is the root container if available.
// If we have no rows, we have no predicate.
guard !requiredRowTemplates.isEmpty else {
guard !requiredRowTemplates.isEmpty, !rowTemplates.isEmpty else {

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Additional check here for the rowTemplates. When this was empty the access below of rowTemplates[0] would throw an index out of bounds error.

@richardpiazza richardpiazza marked this pull request as ready for review May 9, 2026 18:07
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.

1 participant