Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

Sign up
Here's how it works:
  1. Anybody can ask a question
  2. Anybody can answer
  3. The best answers are voted up and rise to the top

I wrote some code to solve this problem https://www.hackerrank.com/challenges/string-o-permute which basically states: take an even length string and swap all the even indexed characters. For example the string "abcd" should become "badc". I'm looking for a general code review as I'm very new to F# and specifically I'd really like to get rid of the ignoredTupleValue variable, it seems like a kludge to me. I'd also like to collapse the whole thing into one function but I was running into scoping issues with the StringBuilder.

open System
open System.Text

[<EntryPoint>]
let main argv =
    let numCases = Console.ReadLine() |> int
    for i = numCases downto 1 do
        let strIn = Console.ReadLine()
        let pairs = Seq.pairwise strIn
        let ignoredTupleValue = ('1', '1')
        let swappedTuples = pairs |> Seq.mapi (fun idx pair ->
                                if idx % 2 = 0 then
                                    let (a, b) = pair
                                    (b, a)
                                else
                                    ignoredTupleValue)
        let answer = Seq.fold (fun (sb : StringBuilder) pair ->
                                if pair <> ignoredTupleValue then
                                    let (a, b) = pair
                                    sb.Append(a) |> ignore
                                    sb.Append(b) |> ignore
                                sb)
                                (new StringBuilder())
                                swappedTuples
        printfn "%s" (answer.ToString())
    0
share|improve this question
1  
There are many ways to do this problem, but if you're interested in removing ignoredTupleValue , think about using Seq.filter(...) – Ray 2 days ago

I'm not very good at F#, but anytime I see a for or a do (i.e. a loop) I cringe a little bit. It's more idiomatic to use Seq.Map to apply a function to each element of the range [numCases..1]. Which, since it's descending, you need to apply the step: [numCases..-1..1].

share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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