Add IsHandled := false; when creating OnBefore-publishers

[rvanbekkum]

Hi @DavidFeldhoff,

Thanks again for the wonderful extension. I am making use of the code-actions for creating publishers and extracting procedures quite often. I have one suggestion for when an OnBefore-publisher is created with an IsHandled parameter though: Could you also add a statement IsHandled := false; in front of the call to the publisher?

So that we get:

procedure DoStuff()
var
    IsHandled: Boolean;
begin
    IsHandled := false;
    OnBeforeDoStuff(IsHandled);
    if IsHandled then
        exit;

    Message('I did stuff!');
end;

What do you think? I see it being used as a consistent pattern for newly added events, and it might be nice to be explicit about the initial value of the variable.

[DavidFeldhoff]

Hi Rob :) nice to see you here!

Yes, that's doable of course. I have another thing in the pipeline which I'd like to finish first. I hope you have no pressure here?

[rvanbekkum]

Nice. And, of course, this is not a priority. :)

[DavidFeldhoff]

Wow, February.. Time is really running. I knew that I was off for some time (due to marriage and loved the free time afterwards), but I did not think that I paused that long 🤔😄 I just published it, so it's now there in case you still wanted to have it ^^

Hope to see you at the TechDays? Best regards David

[rvanbekkum]

Nice! 😄 Congratulations on your marriage and the best wishes for both of you!

And yes, I will be at BC TechDays, hope to see you then! 😊

[DavidFeldhoff]

Hi @rvanbekkum,

I just had a chat with another community friend and after that I noticed that the code cop AA0205 is fine with the IsHandledVariable not being initialized by default if it's handed over as var parameter. https://learn.microsoft.com/en-us/dynamics365/business-central/dev-itpro/developer/analyzers/codecop-aa0205

As I want to comply with Microsoft's Standard, I decided to deactivate the behaviour by default. Nevertheless I added a setting alCodeActions.initializeIsHandledVariableWhenCreatingOnBeforePublisher so that you're still able to make it explit in your company :) I hope that's fine!

Best regards David

[rvanbekkum]

Hi @DavidFeldhoff, Yes, the code cop won't complain about it, but the reason to consistently set IsHandled to false is more so to prevent collisions with other event publishers (P.S. if you structure your code right, you won't often see multiple event publishers with an IsHandled parameter invoked from the same procedure though 😉). I don't think being more explicit is a bad thing. That is also why I like to see the following as good habits:

• Set the DataPerCompany property on table objects explicitly, so we know that the developer makes the decision between true or false. (Same thing for the Access property on objects.)
• Pass true or falseas the first parameter toInsert(...),Modify(...)`, ... statements, to show your choice for whether to run it with or without validation was thought true.
• And so, also, set initial values for variables explicitly, so that you better show the intention, and not that it is something that was accidentally left out.

Adding the setting is fine to me 😊, but I just wanted to share some more thoughts on the reasoning: being more explicit and clear to others that look at your code.

P.S. Of course you can always take 'being explicit' to far, making it 'superfluous', but that is a balance to carefully consider while developing. 😄

[DavidFeldhoff]

Thanks for taking the time to share some more thoughts on that!

And yes, I agree that it makes sense to be generally more explicit, for example on Insert, Modifyetc calls. But in this case I really think that it's not necessary, because we're talking about an OnBefore Publisher which is by design always at the beginning of the procedure, so there can't be another event or statement in front of it that could have already changed the IsHandled. That's simply implicit because of the OnBefore Pattern :)

Glad that you're happy with that solution as well and best regards David