2
\$\begingroup\$

I'm new to F# and functional programming in general (coming from an object oriented background). I just wanted to get some criticism on an algorithm for assigning jobs to crew members. I want to be as functional as possible and avoid loops and mutable state. Here is what I've come up with.

let rec AssignJobs crewMembers jobs jobAssignments = 
    let AssignJob job crewMembers = 
        let bestCrewMember = crewMembers |> Seq.sortWith crewMemberComparison
                                         |> Seq.head
        {CrewMember = bestCrewMember; Job = job}

    match jobs with
    | job :: remainingJobs -> 
        let jobAssignment = AssignJob job crewMembers            
        let remainingCrewMembers = crewMembers |> Seq.filter ((<>)jobAssignment.CrewMember)            
        AssignJobs remainingCrewMembers remainingJobs (jobAssignments @ [jobAssignment])
    | [] -> jobAssignments

Assume that crewMemberComparison is working for this review. I'm more concerned about whether I have a good approach to managing the crewMembers, jobs and jobAssignments sequences.

I don't like how I have to invoke this with an empty list i.e.

let assignedJobs = AssignJobs crewMembers jobs []

Is there another way? Is recursion the best solution at all (presuming I'm not allowed to loop)?

Thanks in advance!

\$\endgroup\$

1 Answer 1

2
\$\begingroup\$

I don't like how I have to invoke this with an empty list i.e.

let assignedJobs = AssignJobs crewMembers jobs []

Is there another way? Is recursion the best solution at all (presuming I'm not allowed to loop)?

The 'empty list' initiation is pretty standard in F#. The nice thing about it is if you load a list of currently assigned jobs from a data-source (database, API, etc.) you can seed the function with the current assignments.

Recursion is perfectly fine here, and in fact it's the route I would go to solve this issue. That said, I would actually consider an unfold as well, which would remove the empty-list requirement.

Basically, unfold takes some seed data (here it would be the crewMembers and jobs lists) and it creates a sequence/list/array from that seed data. (It's the opposite of fold: whereas fold creates a single result from a list, even if that's another list, unfold takes a single result and creates the original list.)

With regard to your current method, I have minimal comments. This is quite idiomatic F#, and follows most of the standard themes we use.

let AssignJob job crewMembers = 
    let bestCrewMember = crewMembers |> Seq.sortWith crewMemberComparison
                                     |> Seq.head
    {CrewMember = bestCrewMember; Job = job}

I'm not a big fan of your formatting, but beyond that I see no issues here. Personally, I would break it down as follows:

let AssignJob job crewMembers = 
    let bestCrewMember = 
        crewMembers
        |> Seq.sortWith crewMemberComparison
        |> Seq.head
    { CrewMember = bestCrewMember; Job = job }

(I don't like leaving unnecessary whitespace on the left — makes it harder to find the code you need to see.)

Also, I would name that assignJob as is normal F# parlance for function names.

Lastly, you could take advantage of Seq.exept instead of Seq.filter ((<>) ...), but you need to instantiate an extra list or array to do that, so I'm not sure if it's worth it.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.