current community

your communities

more stack exchange communities

Stack Exchange
tour help
Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.
up vote 1 down vote favorite

I'm learning Haskell on my own and I'm following the material of a course.

I would like to know if this is idiomatic Haskell.

asList :: [String] -> String
asList ss = "[" ++ (concat $ intersperse "," ss) ++ "]"
share|improve this question
    
I have arbitrarily chosen one of your four questions. You should ask about the other three functions in separate posts. If you ask about insertBST, you should probably include the corresponding searchBST function in that question too. –  200_success♦ Nov 19 '14 at 19:28
    
The homework tag would instruct reviewers to refrain from giving you complete solutions. Since you have indicated that you are writing code for a self-study course, I have removed the [homework] tag for you. –  200_success♦ Nov 19 '14 at 19:30

2 Answers 2

up vote 2 down vote accepted

It looks idiomatic to me.

There's one slight improvement to the code that I could come up with. There is a function that does what you're doing in the middle, called intercalate. Here's the definition, from Data.List:

intercalate :: [a] -> [[a]] -> [a]
intercalate xs xss = concat (intersperse xs xss)

Using that, your function becomes:

asList :: [String] -> String
asList ss = "[" ++ (intercalate "," ss) ++ "]"
share|improve this answer
    
Beautiful! When I wrote asList, I actually remember seeing intercalate - but did not take the mental leap to consider a a Char. (I considered a as String, in which case it is not as useful). –  j-a Nov 21 '14 at 5:24
    
Perhaps - you could include the performance comparison in your answer? It is interesting/useful enough not to be hidden in a comment. –  j-a Nov 21 '14 at 5:29
up vote 1 down vote

The ++ operator should be used sparingly, and especially avoided after long lists. In this case, the ++ "]" at the end would require walking down nearly the entire constructed string to append the final character.

Here is an implementation that avoids that problem:

asList :: [String] -> String
asList ss = '[' : asList' ss
  where
    asList' (a:b:ss) = a ++ (',' : asList' (b:ss))
    asList' (a:ss)   = a ++ asList' (ss)
    asList' []       = "]"
share|improve this answer
    
OK I see, thanks. To me, this is sacrificing readability for performance - is it common to go to this extent to avoid ++ operator? Or is that only done for performance-critical functions? –  j-a Nov 20 '14 at 18:21
1  
This was also interesting: stackoverflow.com/questions/5188286/… –  j-a Nov 20 '14 at 18:24
    
I agree that your original code is more readable. In the end, it's up to you to decide whether readability or performance is more important. –  200_success♦ Nov 20 '14 at 18:26
2  
I tried this with replicate 50000000 "abc" as input, and on my machine in GHCI using :set +s, the OP's code runs in about 3.5 seconds, and your code runs in about 35 seconds. I tried writing a bit of code that used foldr (:) to avoid the ++ at the end, assuming it would be faster. My code ran in about 4.2 seconds. –  Michael Shaw Nov 20 '14 at 21:12

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.