Adds various null checks required for AWS SDK v4

[njlr]

This PR adds null checks to TableContext.fs that are required for AWS SDK v4.

Somewhat related to #87

[bartelink]

I know you're doing it for consistency but the guard guarantees it won't be null (in fact the guard will return Null if the Item.Count = 0)

[njlr]

Yeah, I had to return an option either way so figured why not.
I guess Some response.Item might be marginally more efficient since it is one fewer null check?

[bartelink]

TL;DR Its more about not putting 'just in case' code that's not justified as that makes the reader have to think more (and/or if you were trying to autogen coverage of all paths, you'd be trying to generate a null with the guard returning true, which would be impossible)

It happens to be more efficient, but its more about keeping the code straightforward and not putting diffs in unless there is an actual correctness improvement - the IsItemSet absolutely guarantees a non-null value, and the general pattern should be to think about guarding all inspections of properties and then either do the work conditionally, or substitute an empty if the arrays are being processed in some way.

[njlr]

Makes sense; updated

[bartelink]

perhaps clearer if we switch to array comprehensions:

        let unprocessed =
            [| if response.UnprocessedItems <> null then
                   match response.UnprocessedItems.TryGetValue tableName with
                   | true, reqs ->
                       for r in reqs do
                           if r.PutRequest <> null then
                               yield r.PutRequest.Item
                   | false, _ -> () |]

NOTE in the above I did not use isNull / isNotNull
These should normally only be used when you're checking a reference to an F# type that is marked as not allowing null (the default) (to circumvent the compiler error that would arise if you did)

[njlr]

I like the comprehensions too; updated

[bartelink]

these get pretty illegible and reformatting might split the lines.

        |> Option.iter (fun r ->
            let cc = if response.ConsumedCapacity = null then [] else Seq.toList response.ConsumedCapacity
            r BatchWriteItems cc (items.Length - unprocessed.Length))

[njlr]

I will wait till you have made all comments then fix these in one go.
Can also apply Fantomas if it doesn't blow up the diff?

[samritchie]

@njlr we did a big reformat with fantomas last year so it should only update the changes

[njlr]

these get pretty illegible and reformatting might split the lines.

Much nicer; updated

@njlr we did a big reformat with fantomas last year so it should only update the changes

Great, I have applied Fantomas

[bartelink]

            let cc = if response.ConsumedCapacity = null then [] else Seq.toList response.ConsumedCapacity
            for tableName, consumedCapacity in cc |> Seq.groupBy _.TableName do
                r tableName Operation.TransactWriteItems (Seq.toList consumedCapacity) (Seq.length transactionItems))

[bartelink]

use = null and/or remove all usage of isNull/isnotNull?

(I really would like this to format as the one liner it should be! did fantomas/auto format do it this way?)

[njlr]

I find it odd this library defines an isNull when that is part of FSharp.Core 🤔

Will update the rest 👍

Edit:

(I really would like this to format as the one liner it should be! did fantomas/auto format do it this way?)

Yes, that's Fantomas