microsoft / AL

Home of the Dynamics 365 Business Central AL Language extension for Visual Studio Code. Used to track issues regarding the latest version of the AL compiler and developer tools available in the Visual Studio Code Marketplace or as part of the AL Developer Preview builds for Dynamics 365 Business Central.
MIT License
750 stars 246 forks source link

[Suggestion] Use of Getter functions instead adding more paramters to events #2837

Open NAVRockClimber opened 6 years ago

NAVRockClimber commented 6 years ago

Hi,

this issue is related to this issue 2835.

I find myself quite often in the situation of asking for additional parameters for events in high frequently used codeunits. Just for getting the information in a global variable. I would like to suggest for often frequently used codeunits like 90,80,22,... to enable the include sender property on the event and to add a getter function for a global variable instead of adding the variable as parameter.

This would cause less "noise" by changed events in the codeunits and we partners would have to check less events if they are still working with every release.

Related to my issue 2835 this would look like this.

Current code:

OnBeforeReturnShptLineInsert(ReturnShptLine,ReturnShptHeader,PurchLine);
ReturnShptLine.INSERT(TRUE);
OnAfterReturnShptLineInsert(ReturnShptLine,ReturnShptHeader,PurchLine,ItemLedgShptEntryNo,WhseShip,WhseReceive);

CheckCertificateOfSupplyStatus(ReturnShptHeader,ReturnShptLine);

Current publisher:

LOCAL [IntegrationEvent] OnAfterReturnShptLineInsert(VAR ReturnShptLine : Record "Return Shipment Line";ReturnShptHeader : Record "Return Shipment Header";PurchLine : Record "Purchase Line";ItemLedgShptEntryNo : Integer;WhseShip : Boolean;WhseReceive : Boolean)

Sugessted Change / Pattern:

Event Publisher:

IncludeSender: **Yes**

LOCAL [IntegrationEvent] OnAfterReturnShptLineInsert(VAR ReturnShptLine : Record "Return Shipment Line";ReturnShptHeader : Record "Return Shipment Header";PurchLine : Record "Purchase Line")

Event Call:

OnBeforeReturnShptLineInsert(ReturnShptLine,ReturnShptHeader,PurchLine);
ReturnShptLine.INSERT(TRUE);
OnAfterReturnShptLineInsert(ReturnShptLine,ReturnShptHeader,PurchLine);

CheckCertificateOfSupplyStatus(ReturnShptHeader,ReturnShptLine);

Getter functions:

[External] GetItemLedgShptEntryNo() : Integer
EXIT(ItemLedgShptEntryNo);

[External] GetWhseShip() : Boolean
EXIT(WhseShip);

[External] GetWhseReceive() : Boolean
EXIT(WhseReceive);

[External] GetTempWhseShptHeader(VAR WarehouseShipmentHeader : Record "Warehouse Shipment Header")
WarehouseShipmentHeader := TempWhseShptHeader;

This would enable everybody to get required information from a codeunit and without adding endless parameters to the event publishers

jrrdk commented 6 years ago

Remove the property IncludeSender on publisher and make it mandatory on all events so you always have a sender

And instead of making Getter functions make e new property on globel varibels so you can set then to be a property on the object (set;get;) this property should be enabled as default

NKarolak commented 6 years ago

@AlexanderYakunin Any news on this?

AlexanderYakunin commented 6 years ago

Original ask will be considered for new events. We cannot change already implemented ones. Last comment is enhancement and should be reassigned to Modern Dev team.

dzzzb commented 4 years ago

I disagree that adding more global variables is ever a solution to anything. I thought old NAV code had proved that well...?

A better idea might be 'argument tables', where a record is passed that contains an arbitrary number of name/value pairs. Doing that while retaining proper type info would require a lot of table objects though. Letting the language support 'structs' could obviate that. Or, to avoid polluting the global namespace with publisher-specific arg tables, make it possible to declare a table within the namespace of the publisher only. Sames goes for structs.