Use Thoth.Json.Net everywhere consistently

[Zaid-Ajaj]

Part of this review by @MangelMaxime

Use Thoth.Json.Net instead of the low-level Newtonsoft.Json

[semuserable]

Hey @Zaid-Ajaj!

I was interested in the current issue, but I'm not sure if my style or approach would be valid for your project.

I just created a sample code for one part where Newtonsoft.Json is used and I would like you to have a look at it. I just want to make sure that my code is something that you could merge :)

In order to not clutter this post I'll include number lines where I thought I would replace your part with mine.

Your: https://github.com/Zaid-Ajaj/Femto/blob/master/Program.fs#L83-L113 Mine:

type RawPackage =
    {
      Dependencies : Map<string, string> option
      DevDependencies : Map<string, string> option }

let topLevelPackages =
        match Decode.Auto.fromString<RawPackage>(file, isCamelCase = true) with
        | Ok rawPackage ->
            let inline createInstalledPackage name version isDevDependency = {
                Name = name;
                Range = Some (SemVer.Range(version));
                Installed = None;
                DevDependency = isDevDependency
            }
            let inline createInstalledPackages isDevDependency (dependencies: Map<string, string> option) =
                 dependencies
                 |> Option.map (fun deps -> deps |> Seq.map (fun kvp -> createInstalledPackage kvp.Value kvp.Key isDevDependency))
                 |> Option.defaultValue Seq.empty 

            ResizeArray [
                yield! (false, rawPackage.Dependencies) ||> createInstalledPackages;
                yield! (true, rawPackage.DevDependencies) ||> createInstalledPackages
            ]
        | Error _ -> ResizeArray []

In advance, thanks for the feedback)

[Zaid-Ajaj]

Hello @semuserable, I am really glad to hear that you want to pick this up!

The code above looks good, though I am not a big fan of the ||> operator since its use here is unnecessary. I would simplify createInstalledPackage and createInstalledPackages into a single function:

let createInstalledPackages isDevDependency (dependencies: Map<string, string> option) =
    dependencies
    |> Option.defaultValue Map.empty
    |> Seq.map (fun pair -> {
        Name = pair.Key
        Range = Some (SemVer.Range(pair.Value))
        Installed = None
        DevDependency = isDevDependency
    })      

In your code you have the parameter name is bound from kvp.Value and version from kvp.Key. I think it should be the other way around if I remember correctly

[semuserable]

@Zaid-Ajaj you're absolutely right about kvp.Value|Key (I refactored it just before posting). Also, thanks for the proposed refactoring!