7
\$\begingroup\$

Earlier today I wrote a question (Validating a StreamingPin) and I thought it was a good idea (apparently I was the only one who thought so, but whatever), so I proceeded further with it and developed a version of this programme in F#, with the suggestions from the original question implemented as well. I'm going to be rewriting this programme in several languages largely so I can help improve my skills in each one to a small extent. (C#, F#, VB.NET, C++, Python, Perl, Pascal, PHP, Ruby, Java, JavaScript, Swift, Objective-C)

It feels fairly functional, but I assume there's more I can do to achieve that.

I wrote my own F# class (type) called StreamingPin, and then treated it the same way as the original StreamingPin class from C#. (Built the same functionality, and the suggestions.) It turned out a lot shorter and feels a lot more maintainable than the C# version, but that may be because I did mostly imperative programming in the F# version. (Imperative programming with functional syntax...eww?)

type StreamingPin(pinIn:string) =
    do if pinIn |> String.IsNullOrEmpty then
        raise (new ArgumentException "The value provided for pinIn cannot be null or empty.")

    let mutable lastCorrect = -1

    member private this.pin = pinIn

    member this.IsCorrect = (lastCorrect + 1) = this.pin.Length

    member this.ProcessCharacter c =
        if this.IsCorrect then lastCorrect <- -1

        let currentIndex = lastCorrect + 1

        match c with
        | _ when c = this.pin.Chars currentIndex -> lastCorrect <- currentIndex
        | _ when c = this.pin.Chars 0 -> lastCorrect <- 0
        | _ -> lastCorrect <- -1

        this.IsCorrect

Obviously it's pretty similar to the C# version, if the pinIn is null or empty, throw an error (raise (new ArgumentException)), make the lastCorrect mutable so that we can alter it in ProcessCharacter, create a pin member, and create a IsCorrect member.

I also changed (in the C# version as well) the ProcessCharacter method in the following two ways:

First: if this.IsCorrect is true, then I reset lastCorrect to \$-1\$.
Second: I return this.IsCorrect after the method call.

I also made a StreamingPinTests class as follows:

type StreamingPinTests() =
    interface ITests with
        member this.Name = "Streaming Pin"

        member this.TestCount = 1

        member this.RunAll() = 
            let input = "p2gsvpinnumberPinNumbessfeER#VvsdinNumberPinNumberdfvlj4kF4wfV"
            let sp = StreamingPin "PinNumber"

            let result =
                input.ToCharArray()
                |> Array.toList
                |> takeThrough (fun elem -> elem |> sp.ProcessCharacter = false)
                |> List.map (fun elem ->
                    printfn "%c" elem
                    elem)
                |> List.length <> input.Length

            if result = true then
                printfn "Pin found."
            else
                printfn "Pin not found."
            result

The output is identical to the C# version, while still following the functional paradigms in that particular function. I also wrote a takeThrough function which seems to preserve the major functional aspects.

The takeThrough is defined as follows:

let takeThrough(predicate)(source) =
    let rec loop sourceTemp =
        let head = sourceTemp |> List.head
        if head |> predicate = true then
            head :: (sourceTemp |> List.tail |> loop)
        else
            [head]
    loop source

It does the same thing as takeWhile, but it returns the next element in the list as well. Side-effects are preserved.

The ITests interface is as follows:

type ITests =
    abstract member Name : string
    abstract member TestCount : int
    abstract member RunAll : unit -> bool

Finally, and you don't have to review this part, I made the main method run a list of ITests objects:

[<EntryPoint>]
let main argv = 
    let tests : ITests list = [new StreamingPinTests()]

    let runTest(test:ITests) =
        if test.RunAll() then
            Console.ForegroundColor <- ConsoleColor.White
            printfn "Test %s ran %i passed." test.Name test.TestCount
            true
        else
            Console.ForegroundColor <- ConsoleColor.Red
            printfn "Test %s ran %i failed." test.Name test.TestCount
            false

    let passed =
        tests
        |> List.where (fun x -> x |> runTest)
        |> List.length

    Console.ForegroundColor <- ConsoleColor.White
    printfn "All tests ran."

    if passed <> tests.Length then
        Console.ForegroundColor <- ConsoleColor.Red
        printfn "%i/%i tests passed" passed tests.Length
    else
        Console.ForegroundColor <- ConsoleColor.White
        printfn "All tests passed."

    Console.ReadLine() |> ignore
    0

This main method feels a lot less functional than everything else, but I basically copied the C# Main method to F# syntax, then applied a little functionality to the F# version (mostly changing passed to be immutable):

static void Main(string[] args)
{
    var tests = new List<ITests> { new StreamingPinTests() };

    var passed = 0;

    foreach (var test in tests)
    {
        if (test.RunAll())
        {
            Console.ForegroundColor = ConsoleColor.White;
            Console.WriteLine($"Test {test.Name} ran {test.TestCount} passed.");
            passed++;
        }
        else
        {
            Console.ForegroundColor = ConsoleColor.Red;
            Console.WriteLine($"Test {test.Name} ran {test.TestCount} failed.");
        }
    }

    Console.ForegroundColor = ConsoleColor.White;
    Console.WriteLine($"All tests ran.");

    if (passed != tests.Count)
    {
        Console.ForegroundColor = ConsoleColor.Red;
        Console.WriteLine($"{passed}/{tests.Count} tests passed");
    }
    else
    {
        Console.ForegroundColor = ConsoleColor.White;
        Console.WriteLine($"All tests passed.");
    }

    Console.ReadLine();
}

All-in-all I think I like the F# version better as far as code goes, it's more readable to me.

\$\endgroup\$
  • \$\begingroup\$ I've never seen the do if ... then construct before. Do you really need the do there? \$\endgroup\$ – Hosch250 Sep 21 '16 at 15:47
  • \$\begingroup\$ @Hosch250 Yes, because it's in a type, if you omit do then you get "Unexpected keyword 'if' in type definition", the do simply tells F# that what follows is code to be run during construction. \$\endgroup\$ – Der Kommissar Sep 21 '16 at 15:49
  • \$\begingroup\$ Hmm. I'll take a look at this sometime after midterms. \$\endgroup\$ – Hosch250 Sep 21 '16 at 15:51
1
\$\begingroup\$

In an effort to answer my own question (and help remove a potential zombie) I did some self-improvement:

Obviously functional languages prefer matching to conditionals, so I rewrote takeThrough as follows:

let takeThrough(predicate)(list) =
    let rec loop sourceTemp =
        let head = sourceTemp |> List.head
        match head |> predicate with
        | true -> head :: (sourceTemp |> List.tail |> loop)
        | false -> [head]
    loop list

Pretty simple, and definitely keeps things more functional.

In the same respect, I did all the other boolean if statements the same way.

The only weird one was the passed one in main:

[<EntryPoint>]
let main argv = 
    let tests : ITests list = [new StreamingPinTests(); new ListManipulationTests()]

    let runTest(test:ITests) =
        let result = test.RunAll()
        match result with
        | true ->
            Console.ForegroundColor <- ConsoleColor.White
            printfn "Test %s ran %i passed." test.Name test.TestCount
        | false ->
            Console.ForegroundColor <- ConsoleColor.Red
            printfn "Test %s ran %i failed." test.Name test.TestCount
        result

    let passed =
        tests
        |> List.where (fun x -> x |> runTest)
        |> List.length

    Console.ForegroundColor <- ConsoleColor.White
    printfn "All tests ran."

    match passed with
    | count when count = tests.Length -> 
        Console.ForegroundColor <- ConsoleColor.White
        printfn "All tests passed."
    | _ -> 
        Console.ForegroundColor <- ConsoleColor.Red
        printfn "%i/%i tests passed" passed tests.Length

    Console.ReadLine() |> ignore
    0

I rewrote runTest as:

let runTest(test:ITests) =
    let result = test.RunAll()
    match result with
    | true ->
        Console.ForegroundColor <- ConsoleColor.White
        printfn "Test %s ran %i passed." test.Name test.TestCount
    | false ->
        Console.ForegroundColor <- ConsoleColor.Red
        printfn "Test %s ran %i failed." test.Name test.TestCount
    result

Finally, after a lot of thinking, I managed to rewrite the takeThrough function with tail-call recursion, instead of the original method (which didn't use tail-call recursion).

let takeThrough predicate list =
    let rec loop acc sourceTemp =
        let head = sourceTemp |> List.head
        match head |> predicate with
        | true -> loop (List.append acc [head]) (sourceTemp |> List.tail)
        | false -> List.append acc [head]
    loop [] list

By using an accumulator, and making the loop call the very last thing the loop function has to do, it can now use tail-call recursion to optimize itself into a cleaner loop.

I'm quite certain this follows the functional paradigms much more appropriately than the original.

\$\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.