0

I cannot figure out why my function invokeAll does not give out the correct output/work properly. Any solutions? (No futures or parallel collections allowed and the return type needs to be Seq[Int])

def invokeAll(work: Seq[() => Int]): Seq[Int] = {
        //this is what we should return as an output "return res.toSeq"

        //res cannot be changed!
        val res = new Array[Int](work.length)

    var list = mutable.Set[Int]()
    var n = res.size
    val procedure = (0 until n).map(work => 
      new Runnable {
        def run {
          //add the finished element/Int to list
          list += work 
        }
      }
    )

    val threads = procedure.map(new Thread(_))
    threads.foreach(x => x.start())
    threads.foreach (x =>  (x.join()))
  
    res ++ list
    //this should be the final output ("return res.toSeq")
    return res.toSeq
  }
Lee
  • 1
  • 2
  • 1
    This kind of imperative programming is pretty much discouraged in Scala. use `Future`s instead, and immutable collections. This works fine: `def invokeAll(works: Seq[() => Int]): Future[Seq[Int]] = Future.sequence(works.map(work => Future(work())))`. – AminMal May 24 '22 at 10:03

2 Answers2

3

OMG, I know a java programmer, when I see one :) Don't do this, it's not java!

val results: Future[Seq[Int]] = Future.traverse(work)

This is how you do it in scala.

This gives you a Future with the results of all executions, that will be satisfied when all work is finished. You can use .map, .flatMap etc. to access and transform those results. For example val sumOfAll: Future[Int] = results.map(_.sum)

Or (in the worst case, when you want to just give the result back to imperative code), you could block and wait on the future to get ahold of the actual result (don't do this unless you are absolutely desperate): Await.result(results, 1 year)

If you want the results as array, results.map(_.toArray) will do that ... but you really should not: arrays aren't really a good choice for the vast majority of use cases in scala. Just stick with Seq.

Dima
  • 39,570
  • 6
  • 44
  • 70
  • Why `Future.sequence` + `map` instead of just `Future.traverse`? – Luis Miguel Mejía Suárez May 24 '22 at 13:19
  • 1
    @LuisMiguelMejíaSuárez you are right ... brain freeze :) ... I wrote `traverse` at first, and then edited to replace with `sequence` ... for some reason, that now escapes me. I'll change it back. – Dima May 24 '22 at 14:49
-2

The main problem in your code is that you are using fixed size array and trying to add some elements using ++ (concatenate) operator: res ++ list. It produces new Seq but you don't store it in some val. You could remove last line return res.toSeq and see that res ++ lest will be return value. It will be your work.length array of zeros res with some list sequence at the end. Try read more about scala collections most of them immutable and there is a good practice to use immutable data structures. In scala Arrays doesn't accumulate values using ++ operator in left operand. Array's in scala are fixed size.

Boris Azanov
  • 4,408
  • 1
  • 15
  • 28
  • Thank you for the response and clarity! My explanation wasn't great but to clarify do you know if there's a way I could merge the values in "list" to "res"? I mean fill those zero slots in res with values from "list"? I think that would make the code work. – Lee May 24 '22 at 09:23
  • look at this answer: https://stackoverflow.com/a/9384966/6770614 for updating values in array you should have `index` and `new value`: `res(index) = newValue` – Boris Azanov May 24 '22 at 10:03
  • @Lee You don't need mutating the array just to merge the set into it, `Array.from(list)` will do the trick. `list = mutable.Set(1, 2, 3, 4); Array.from(list) = Array(1, 2, 3, 4)` – AminMal May 24 '22 at 10:19
  • 1
    @BorisAzanov please do not encourage people to do C-style coding in Scala! – AminMal May 24 '22 at 10:20
  • @AminMal to be honest, there is no reason to use mutable structures, mutate states and merge some arrays with others. I just answer the concrete question, how do one thing. Sure the better way is using `Future` and `map` `flatMap` functions but how much background you need to use it right?! So my answer is minimal helpful for the concrete question. It is not best practice. For solve this problem more canonical needs to use `sequence` some `Future`'s. – Boris Azanov May 24 '22 at 11:04
  • @BorisAzanov you're completely right, sorry if I couldn't make what I meant clearer, I mean you can still propose better approaches that also satisfy the language's best practices, I did not mean your code doesn't work or is wrong or whatever, so no offense :) – AminMal May 24 '22 at 12:09