microsoft / microsoft/BCApps

[Bug]: TryGetDictionaryValueFromKey should not be a try function

Open Beginner friendly
#8,840 2 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Approved Team: SCM
Dominant language
AL
Stars
683
Forks
459
Avg merge
3d 26m
Merged PRs (30d)
633

Description

Describe the issue

In page 9082 "Customer Statistics FactBox" there is a LOCAL function:

[TryFunction]
local procedure TryGetDictionaryValueFromKey(var DictionaryToLookIn: Dictionary of [Text, Text]; KeyToSearchFor: Text; var ReturnValue: Text)
begin
ReturnValue := DictionaryToLookIn.Get(KeyToSearchFor);
end;

And only gets used in the trigger:

trigger OnPageBackgroundTaskCompleted(TaskId: Integer; Results: Dictionary of [Text, Text])
var
CalculateCustomerStats: Codeunit "Calculate Customer Stats.";
DictionaryValue: Text;
begin
if (TaskId = TaskIdCalculateCue) then begin
if Results.Count() = 0 then
exit;

        if TryGetDictionaryValueFromKey(Results, CalculateCustomerStats.GetLastPaymentDateLabel(), DictionaryValue) then
            Evaluate(LastPaymentDate, DictionaryValue);

        if TryGetDictionaryValueFromKey(Results, CalculateCustomerStats.GetTotalAmountLCYLabel(), DictionaryValue) then
            Evaluate(TotalAmountLCY, DictionaryValue);

        if TryGetDictionaryValueFromKey(Results, CalculateCustomerStats.GetOverdueBalanceLabel(), DictionaryValue) then
            Evaluate(OverdueBalance, DictionaryValue);

        if TryGetDictionaryValueFromKey(Results, CalculateCustomerStats.GetSalesLCYLabel(), DictionaryValue) then
            Evaluate(SalesLCY, DictionaryValue);

        if TryGetDictionaryValueFromKey(Results, CalculateCustomerStats.GetInvoicedPrepmtAmountLCYLabel(), DictionaryValue) then
            Evaluate(InvoicedPrepmtAmountLCY, DictionaryValue);

        if TryGetDictionaryValueFromKey(Results, CalculateCustomerStats.GetLinkedVendorNoLabel(), DictionaryValue) then
            LinkedVendorNo := CopyStr(DictionaryValue, 1, MaxStrLen(LinkedVendorNo));
        BalanceAsVendorEnabled := LinkedVendorNo <> '';
        if BalanceAsVendorEnabled then
            if TryGetDictionaryValueFromKey(Results, CalculateCustomerStats.GetBalanceAsVendorLabel(), DictionaryValue) then
                Evaluate(BalanceAsVendor, DictionaryValue);
    end;
end;

This is extremely annoying when starting a debug session and opening a page where this factbox is shown. Next to the fact this is a weird way to retrieve a value from a dictionary. I suggest using the if contains functionality. As I will describe in the expected behavior.

Expected behavior

Remove the Try function tag and make changes to procedure like this:

local procedure TryGetDictionaryValueFromKey(var DictionaryToLookIn: Dictionary of [Text, Text]; KeyToSearchFor: Text; var ReturnValue: Text): Boolean
begin
if not DictionaryToLookIn.ContainsKey(KeyToSearchFor) then
exit(false);
ReturnValue := DictionaryToLookIn.Get(KeyToSearchFor);
exit(true);
end;

Steps to reproduce

Start debug session without any breakpoints but with "breakOnError":true, in the launch.json

Additional context

No response

I will provide a fix for a bug
  • I will provide a fix for a bug

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start at page 9082, "Customer Statistics FactBox," and inspect TryGetDictionaryValueFromKey and its calls in OnPageBackgroundTaskCompleted. Review launch.json with breakOnError enabled, then verify that missing dictionary keys no longer trigger the debugger while the expected values are still evaluated and assigned.

Written by the indexing model from the issue text.

Assessment

Domain
developer-experience
Issue type
Bug
Difficulty
2/5
Estimated time
1-3 hours
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
72/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.