Skip to main content
added 281 characters in body
Source Link
  • Naming

    • Use something shorter and more generic, like a and b, instead of list1 and list2
    • curry is mispelled -- should be carry
  • You are repeating yourself a lot. For example, except in the case where both lists are empty:

    • In each case, you have three adds that are exactly the same.
    • All three cases have a very similar structure -- the only difference is basically which numbers are being added
  • Consider defining a class to represent numbers. That way, you can easily change the representation in the future.

  • I don't think your implementation is tail-recursive. This will lead to stack-overflows for large lists. This might or might not be a problem, depending on your application.

  • Use "word-at-a-time" thinking only as a last resort. In this case, you are directly recursing over the lists -- one element ("word") at a time. Try to find a higher-level abstraction, such as map, that you can use.

    In this particular case, in order to handle the carries, you really need some sort of "map plus accumulator" abstraction. To my knowledge, there is no such function built-in to Scala. So you might have to write your own. Still, I would suggest implementing a generic, higher-order function to do this. That way, you can reuse that function later for other things.

  • Try to decompose your problem. In this case, the problem is already fairly simple. Still, you might (or might not -- I'm not sure) benefit from drawing inspiration from hardware adders.

  • Naming

    • Use something shorter and more generic, like a and b, instead of list1 and list2
    • curry is mispelled -- should be carry
  • Consider defining a class to represent numbers. That way, you can easily change the representation in the future.

  • I don't think your implementation is tail-recursive. This will lead to stack-overflows for large lists. This might or might not be a problem, depending on your application.

  • Use "word-at-a-time" thinking only as a last resort. In this case, you are directly recursing over the lists -- one element ("word") at a time. Try to find a higher-level abstraction, such as map, that you can use.

    In this particular case, in order to handle the carries, you really need some sort of "map plus accumulator" abstraction. To my knowledge, there is no such function built-in to Scala. So you might have to write your own. Still, I would suggest implementing a generic, higher-order function to do this. That way, you can reuse that function later for other things.

  • Try to decompose your problem. In this case, the problem is already fairly simple. Still, you might (or might not -- I'm not sure) benefit from drawing inspiration from hardware adders.

  • Naming

    • Use something shorter and more generic, like a and b, instead of list1 and list2
    • curry is mispelled -- should be carry
  • You are repeating yourself a lot. For example, except in the case where both lists are empty:

    • In each case, you have three adds that are exactly the same.
    • All three cases have a very similar structure -- the only difference is basically which numbers are being added
  • Consider defining a class to represent numbers. That way, you can easily change the representation in the future.

  • I don't think your implementation is tail-recursive. This will lead to stack-overflows for large lists. This might or might not be a problem, depending on your application.

  • Use "word-at-a-time" thinking only as a last resort. In this case, you are directly recursing over the lists -- one element ("word") at a time. Try to find a higher-level abstraction, such as map, that you can use.

    In this particular case, in order to handle the carries, you really need some sort of "map plus accumulator" abstraction. To my knowledge, there is no such function built-in to Scala. So you might have to write your own. Still, I would suggest implementing a generic, higher-order function to do this. That way, you can reuse that function later for other things.

  • Try to decompose your problem. In this case, the problem is already fairly simple. Still, you might (or might not -- I'm not sure) benefit from drawing inspiration from hardware adders.

Source Link

  • Naming

    • Use something shorter and more generic, like a and b, instead of list1 and list2
    • curry is mispelled -- should be carry
  • Consider defining a class to represent numbers. That way, you can easily change the representation in the future.

  • I don't think your implementation is tail-recursive. This will lead to stack-overflows for large lists. This might or might not be a problem, depending on your application.

  • Use "word-at-a-time" thinking only as a last resort. In this case, you are directly recursing over the lists -- one element ("word") at a time. Try to find a higher-level abstraction, such as map, that you can use.

    In this particular case, in order to handle the carries, you really need some sort of "map plus accumulator" abstraction. To my knowledge, there is no such function built-in to Scala. So you might have to write your own. Still, I would suggest implementing a generic, higher-order function to do this. That way, you can reuse that function later for other things.

  • Try to decompose your problem. In this case, the problem is already fairly simple. Still, you might (or might not -- I'm not sure) benefit from drawing inspiration from hardware adders.