Skip to content

Change addError method to public and virtual#423

Open
wimvelzeboer wants to merge 2 commits into
apex-enterprise-patterns:masterfrom
wimvelzeboer:feature/AddError
Open

Change addError method to public and virtual#423
wimvelzeboer wants to merge 2 commits into
apex-enterprise-patterns:masterfrom
wimvelzeboer:feature/AddError

Conversation

@wimvelzeboer
Copy link
Copy Markdown
Contributor

@wimvelzeboer wimvelzeboer commented Aug 2, 2022

It can be useful to add one error string to all the records of a domain directly from a service class, therefore it needs to be public.
And it is also useful to change its behaviour just like the other addError methods, therefore adding virtual


This change is Reviewable

wimvelzeboer and others added 2 commits August 2, 2022 10:17
It can be useful to add one error string to all the records of an domain, directly from a service class, therefore it needs to be public.
Sometimes it is also useful to change its behavior just like the other addError methods, therefore adding virtual
@afawcett
Copy link
Copy Markdown
Contributor

Hi @wimvelzeboer — thanks again for this PR.

We've been discussing it among maintainers. The use case (e.g. applying a single non-field error across all rows from outside a domain subclass, such as from a service) makes sense.

One concern raised internally is consistency: today fflib_SObjects exposes several protected helpers that apply the same kind of operation to every record in the bag (not only addError(String)). If we only promote addError(String) to public, callers can attach object-level errors from outside the hierarchy but still cannot use the same visibility for the closely related APIs, e.g.:

addError(Schema.SObjectField field, String message)
clearField / clearFields
…which feels asymmetric if we accept the idea that orchestration code may legitimately manipulate this record collection.

Proposal: extend this PR (or a follow-up commit on the same branch) so that the same "bulk mutator" family is aligned with addError(String) — at minimum public virtual on the field-level addError overload and on clearField / clearFields, with virtual kept where it already exists so domain subclasses can still override behavior the same way as for addError(String).

If you're willing to update the PR along those lines, we can take another pass for review.

Thanks for contributing.

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.

2 participants