An appropriate data structure with meaningful names

[JasinCole]

I've thought about this for a week and I just can't seem which way to go...

My ultimate goal is to have a data structure that contains 4 related objects and I want to be able to use that data structure in my view in something similar to a itemsControl. Like below

The issue I am having is figuring out how to mangle my data into an appropriate data structure using 'good' coding principals.

When accessing my database to get the records I get a list of the records sort and sum the records to get the total amount.

1. one issue I am mulling over is if it is worth getting all the records in one db call and then sorting them locally into different buckets or if I should be making 4 different db calls. Recent discussions lean me toward making 4 different db calls.
2. is this an appropriate use of a class? Classes should be singular objects, so I would need a List<AppropriateClassName> The class would only contain 2 members, a property to hold the string value and a decimal to hold the sum value
3. Or, would this be better as some other data structure like a Dictionary<>
4. Would an Enumeration be ok to hold the string values

This all seems to be complicated in my mind because the string values are opposites. What I mean by that is some are upcoming and other are overdue. Which means any variable name I choose to describe one makes no sense to the other. IE. DaysOverdue makes no sense to DueInDays

All I know is this is how I would like to work with the data structure

<ItemsRepeater Grid.Column="0" Grid.Row="1" Items="{CompiledBinding AccountInvoicesDue}">
  <ItemsRepeater.ItemTemplate>
    <DataTemplate>
      <Grid ColumnDefinitions="*, Auto">
        <TextBlock Grid.Column="0" Text="{CompiledBinding /*When it's due in string*/}"/>
        <TextBlock Grid.Column="1" Text="{CompiledBinding /*How much is due in decimal value*/, StringFormat={}{0:C2}}"/>
      </Grid>
   </DataTemplate>
  </ItemsRepeater.ItemTemplate>
</ItemsRepeater>

I'd love to see some psuedo code and why it is appropriate in this situation to use which ever data structure. What I wanted to use was a List<KeyValuePairs<string, decimal>> but I read that if this is what you want then just use a List<Class>. But the class just feels off to me because it seems I am trying to describe plural related objects. IE. AccountsPayableAgingInvoices

Everything I've tried just feels dirty, can anyone give some good advice??

 

[JasinCole]

4. Would an Enumeration be ok to hold the string values

I see now that this wouldn't work either.

 

[Skydiver]

1. one issue I am mulling over is if it is worth getting all the records in one db call and then sorting them locally into different buckets or if I should be making 4 different db calls. Recent discussions lean me toward making 4 different db calls.

Relational database are good at doing this work. For NoSQL databases, it's depends on the capabilities of the database. But in general, it will bu much faster to let the database do all the processing on the server side versus having the database ship the data to you on the client side, and then you doing the processing after the data has been sent.

 

[Skydiver]

2. is this an appropriate use of a class? Classes should be singular objects, so I would need a List<AppropriateClassName> The class would only contain 2 members, a property to hold the string value and a decimal to hold the sum value
3. Or, would this be better as some other data structure like a Dictionary<>

Personally, I would just go with a Dictionary<string, decimal> if the client signs in blood on the contract that those will only ever be the categories of data that they need. Going with a List<PeriodRevenue> is a case of YAGNI, albeit it makes using an ItemRepeater so much easier.

 

[JasinCole]

albeit it makes using an ItemRepeater so much easier.

Would it be more appropriate to use a List<PeriodRevenue> to keep that ItemRepeater implementation easier and have PeriodRevenue be a Struct instead of a class. Since I am only storing strings and decimals I believe this is the perfect use of a struct. Or am I wrong?

public struct PeriodRevenue
{
    public string SomeName;
    public decimal Sum
}

The only other thing that bothers me about this code I am writing is this

.Where(inv => inv.Duedte >= today.AddDays(-60) && inv.Duedte < today.AddDays(-30))

.Where(inv => inv.Duedte >= today.AddDays(-90) && inv.Duedte < today.AddDays(-60))

.Where(inv => inv.Duedte > today && inv.Duedte <= today.AddDays(7))

.Where(inv => inv.Duedte < today.AddDays(-90))

.Where(inv =>inv.Duedte > today)

I started to write a extension function when I realized it wasn't as straightforward as I thought

public static IQueryable<IInvoices> FilterByDueDate(this IQueryable<IInvoices> invoices,
                                                 DateTime fromDueDate, Datetime toDueDate)
{
    return invoices.Where(inv => inv.Duedte >= fromDueDate && inv.Duedte <= toDueDate;
}

Works for dates that have a known period range. Doesn't work on the last two.
It simply does not work if from date is greater than the to date. I did something similar to this for that situation, still doesn't work on the last two.

if(fromDueDate > toDueDate)
{
    (fromDueDate, toDueDate) => (toDueDate, fromDueDate);
}

Now I am wondering if I should check for null and give my parameters a default value of null and do something similar to the following psuedo

extension function that accepts 2 DateTime parameters - fromDueDate & toDueDate
    Check both params for null - throw error;
    Check fromDueDate for null - return IQueryable filtering by toDueDate only
    Check toDueDate for null - return IQueryable filtering by fromDueDate only
    Check fromDueDate > toDueDate - if true swap parameters via deconstruction
    finally return IQueryable filtering by fromDueDate through toDueDate

 

[Skydiver]

// from beginning of time to 90 days ago
...FilterByDueDate(fromDueDate: DateTime.MinValue, toDueDate: DateTime.Today.AddDays(-90));

// from today onwards
...FilterByDueDate(fromDueDate: DateTime.Today, toDueDate: DateTime.MaxValue);

 

[JasinCole]

// from beginning of time to 90 days ago
...FilterByDueDate(fromDueDate: DateTime.MinValue, toDueDate: DateTime.Today.AddDays(-90));

// from today onwards
...FilterByDueDate(fromDueDate: DateTime.Today, toDueDate: DateTime.MaxValue);

Ah yes. Not sure why I try to overcomplicate everything.

 

[cjard]

Absent any direction that one day you'll have to extend this to cater for infinite varions on the "is it overdue by x/underfur by y" I'd go with the 4 I have and just issue a query like:

var d = db.Invoices.GroupBy(x =>
  x.DueDate < today.AddDays(-30) ? "o30" :
  x.DueDate < today ? "o" :
  x.DueDate < today.AddDays(7) ? "d7" :
  x.DueDate < today.AddDays(30) ? "d30" : "df"
)
.ToDictionary(g => g.Key, g => g.Count());

A class like this can make sense of those straight out the dict:

public class X{
  Dictionary<string, int> _d;
  public OverdueBy30 => _d.GetValueOrDefault("o30");

  ...
}

And you could bind those props

Or if you have some row-ar control, just choose the labels ("o30", "o") so they sort nicely (like the order you want left to right) and that they are the names of the strings in the localization system and bind the control to the dictionary, ordered by key, and keys used to lookup translated labels

Since I am only storing strings and decimals I believe this is the perfect use of a struct. Or am I wrong?

I don't personally think structs should contain strings, but it's unlikely to really matter here

 

[JasinCole]

This looks a little voodoo-ish to me. Let me break it down in words, let me know if I am correct.

var d = db.Invoices.GroupBy(x =>
  x.DueDate < today.AddDays(-30) ? "o30" :
  x.DueDate < today ? "o" :
  x.DueDate < today.AddDays(7) ? "d7" :
  x.DueDate < today.AddDays(30) ? "d30" : "df"
)
.ToDictionary(g => g.Key, g => g.Sum(inv => inv.Invnet));

1. Grab all the invoices
2. Group invoices - loop over invoices
2a. Does invoice duedate fall before 30 days ago? Yes - add to bucket o30
2b. Does invoice duedate fall before today? Yes - add to bucket o
2c. Does invoice duedate fall before 7 days from today? Yes - add to bucket d7
2d. Does invoice duedate fall before 30 days from today? Yes - add to bucket d30, No - catch all bucket
3. Take returned collection of type IGrouping and create dictionary where each group is added to dictionary as key-value pairs
where d.key is the group.key and the d.value is group.sum by inv.Invnet

public class X{
  Dictionary<string, int> _d;
  public OverdueBy30 => _d.GetValueOrDefault("o30");

  ...
}

A class that holds the returned dictionary from above with properties that are initialized upon class initialization(which means the class can't be static)

 

[cjard]

A class that holds the returned dictionary from above with properties that are initialized upon class initialization

Sorry, I should have filled out that class a little more; I forgot the constructor and made a typo. Here is a more full class:

public class X{
  private Dictionary<string, int> _d;

  public int OverdueBy30Plus => _d.GetValueOrDefault("o30");

  public int OverdueByUpto30 => _d.GetValueOrDefault("o");

  public int DueBetween7And30 => _d.GetValueOrDefault("d30");
 
  public int DueInFuture => _d.GetValueOrDefault("df");

  public X(Dictionary<string, int> d){
    _d = d;
  }
}

And you might use it like:

  var x = new X(d); //d being the dict from the query

You might have some simpler poco:

  record X(int OverdueBy30Plus, int OverDuByUpto30, int DueBetween7And30, int DueInFuture);

And use it like:

  var x = new X(
    d.GetValueOrDefault("o30"), 
    d.GetValueOrDefault("o"),
    d.GetValueOrDefault("d30"), 
    d.GetValueOrDefault("df")
  );

It matters little in this sense. This code here is a pivot operation, taking N rows to be N columns, notionally, if one equates items in a dictionary with rows and properties with columns

which means the class can't be static

Strive to avoid static in OO. It has its uses but generally you shouldn't be trying to find ways to make things static or even keeping it as a thing in your mind

Here it's just a helper to a pivot operation, and it's a data focused operation: it makes sense to have a class hold that data in named properties so that UI elements can be bound to it

---

The voodoo is just 4 stacked test ? value if true : value if false statements; I'd have used a switch expression if lambda expressions supported them

It ends up producing a query like:

SELECT x, count(*)
FROM(
  SELECT CASE
    WHEN DueDate < blah THEN 'o30'
    WHEN DueDate < blah THEN 'o'
    WHEN DueDate < blah THEN 'd7'
    WHEN DueDate < blah THEN 'd30'
    ELSE 'df'
  END as x
) y
GROUP BY x

This does your counting etc on the DB and in one hit; you leave it to the DB to decide how to process this. Careful indexing can help but the biggest improvement this gives your code is not dragging thousands of invoices over a network to count them on the client

If you're displaying the detail of them too then perhaps you would, but if it were paginated you'd probably run a summary query because you need the info and then take a page at a time

The query nicely partitions invoices into buckets based on date, with no invoice appearing in multiple or wrong buckets because CASE WHEN proceeds linearly and quits at the first applicable case. The time cases are listed in order from past to future with the comparator being "less than", meaning that an invoice falls into a relevant bucket at the first available opportunity. We don't use the d7 bucket in code, but we need it to exist to stop those invoices falling into the d30 bucket. We could also use a where clause to exclude d7 invoices, it matters little

 

[JasinCole]

I actually posted my last reply before writing any code in the IDE. It made more sense once I started writing the code as I could see the output of the SQL commands.

This does your counting etc on the DB and in one hit; you leave it to the DB to decide how to process this. Careful indexing can help but the biggest improvement this gives your code is not dragging thousands of invoices over a network to count them on the client

The funny thing is though, Rider yells at me about this.

DB records: a database command that returns many records
Last observation: 6/7/2023 9:49 AM JetBrains.Debugger.Worker64c.exe
  The command was executed 1 time, 1 time above 100 records threshold
  Total number of records: 145 records
  Max number of records among all executions: 145 records
  Average number of records: 145 records

SELECT [t].[Key], [t].[_idnum], [t].[actper], [t].[adjust], [t].[amtpad], [t].[apinte], [t].[btcnum], [t].[cmpamt], [t].[ctcnum], [t].[dscavl], [t].[dscdte], [t].[dscrpt], [t].[dsctkn], [t].[duedte], [t].[entdte], [t].[freigh], [t].[gstamt], [t].[hldamt], [t].[hldbll], [t].[hldrem], [t].[hotlst], [t].[hstamt], [t].[insdte], [t].[insusr], [t].[invamt], [t].[invbal], [t].[invdte], [t].[invnet], [t].[invnum], [t].[invttl], [t].[invtyp], [t].[jobnum], [t].[lgrrec], [t].[ntetxt], [t].[payee2], [t].[pchord], [t].[phsnum], [t].[postyr], [t].[pstamt], [t].[qstamt], [t].[rcpamt], [t].[recnum], [t].[refnum], [t].[retain], [t].[setpay], [t].[shpnum], [t].[slstax], [t].[status], [t].[subttl], [t].[taxcde], [t].[ttlpad], [t].[upddte], [t].[updusr], [t].[usedst], [t].[usetax], [t].[usettl], [t].[usrdf1], [t].[usrdf2], [t].[usrnme], [t].[vndnum], [t].[vodrec]
FROM (
    SELECT [a].[_idnum], [a].[actper], [a].[adjust], [a].[amtpad], [a].[apinte], [a].[btcnum], [a].[cmpamt], [a].[ctcnum], [a].[dscavl], [a].[dscdte], [a].[dscrpt], [a].[dsctkn], [a].[duedte], [a].[entdte], [a].[freigh], [a].[gstamt], [a].[hldamt], [a].[hldbll], [a].[hldrem], [a].[hotlst], [a].[hstamt], [a].[insdte], [a].[insusr], [a].[invamt], [a].[invbal], [a].[invdte], [a].[invnet], [a].[invnum], [a].[invttl], [a].[invtyp], [a].[jobnum], [a].[lgrrec], [a].[ntetxt], [a].[payee2], [a].[pchord], [a].[phsnum], [a].[postyr], [a].[pstamt], [a].[qstamt], [a].[rcpamt], [a].[recnum], [a].[refnum], [a].[retain], [a].[setpay], [a].[shpnum], [a].[slstax], [a].[status], [a].[subttl], [a].[taxcde], [a].[ttlpad], [a].[upddte], [a].[updusr], [a].[usedst], [a].[usetax], [a].[usettl], [a].[usrdf1], [a].[usrdf2], [a].[usrnme], [a].[vndnum], [a].[vodrec], CASE
        WHEN [a].[duedte] < @__AddDays_1 THEN N'OverdueThirtyPlusDays'
        WHEN [a].[duedte] < @__today_2 THEN N'OverdueOneToThirtyDays'
        WHEN [a].[duedte] < @__AddDays_3 THEN N'DueInSevenDays'
        WHEN [a].[duedte] < @__AddDays_4 THEN N'DueInThirtyDays'
        ELSE N'DueInThirtyPlusDays'
    END AS [Key]
    FROM [acpinv] AS [a]
    WHERE [a].[status] = @__status_0
) AS [t]
ORDER BY [t].[Key]

Using this command

return new PeriodRevenue(await invoices
            .FilterByStatus(InvoiceStatus.Open)
            .GroupBy(inv =>
                inv.Duedte < today.AddDays(-30) ? "OverdueThirtyPlusDays" :
                inv.Duedte < today ? "OverdueOneToThirtyDays" :
                inv.Duedte < today.AddDays(7) ? "DueInSevenDays" :
                inv.Duedte < today.AddDays(30) ? "DueInThirtyDays" : "DueInThirtyPlusDays")
            .ToDictionaryAsync(group => group.Key,
                group => group.Sum(inv => inv.Invnet)));

It seems the Sum operation is not being run DB side but instead in-memory? I do not understand exactly the mechanism that is causing this?

 

[Skydiver]

Rider is correct about the sum being done on the client side.

 

[JasinCole]

I still think this is calculating the SUM in-memory but now I am asking for the Invnet column and as such Rider doesn't yell at me anymore.

await invoices
            .FilterByStatus(InvoiceStatus.Open)
            .GroupBy(inv =>
                inv.Duedte < today.AddDays(-30) ? "OverdueThirtyPlusDays" :
                inv.Duedte < today ? "OverdueOneToThirtyDays" :
                inv.Duedte < today.AddDays(7) ? "DueInSevenDays" :
                inv.Duedte < today.AddDays(30) ? "DueInThirtyDays" : "DueInThirtyPlusDays")
            .Select(group => new { Key = group.Key, Sum = group.Sum(inv => inv.Invnet) })
            .ToDictionaryAsync(group => group.Key, group => group.Sum));

FROM (
    SELECT [a].[invnet], CASE
        WHEN [a].[duedte] < @__AddDays_1 THEN N'OverdueThirtyPlusDays'
        WHEN [a].[duedte] < @__today_2 THEN N'OverdueOneToThirtyDays'
        WHEN [a].[duedte] < @__AddDays_3 THEN N'DueInSevenDays'
        WHEN [a].[duedte] < @__AddDays_4 THEN N'DueInThirtyDays'
        ELSE N'DueInThirtyPlusDays'
    END AS [Key]
    FROM [acrinv] AS [a]
    WHERE [a].[status] = @__status_0
) AS [t]
GROUP BY [t].[Key]

 

[Skydiver]

Is that the complete SQL? If so, then yes, that is still calculating the sum client side because the SQL doesn't have any SUM in SQL statement. As far as I know, GROUP BY does not do an implied summation of a group.

 

[cjard]

Yes, add the .Select you proposed. The query you posted appears to be incomplete; the "FROM" should have something like SELECT t.Key, SUM(t.Invnet) as [Sum] above it