1

I am refactoring some scala code and I am having problems with a while loop. The old code was:

for (s <- sentences){
// ...
   while (/*Some condition*/){
      // ...
      function(trees, ...)
   }
}

I've have translated that code into this one, using foldLeft to transverse sentences:

sentences./:(initialSeed){
 (seed, s) =>
     // ...
     // Here I've replaced the while with other foldleft 
     trees./:(seed){
         (v, n) =>
             // ....
             val updatedVariable = function(...., v)
     }
}

Now, It may be the case that I need to stop transversing trees (The inner foldLeft before it is transverse entirely, for that I've found this question:

Abort early in a fold

But I also have the following problem:

As I transverse trees, I need to accumulate values to the variable v, function takes v and returns an updated v, called here updatedVariable. The problem is that I have the feeling that this is not a proper way of coding this functionality.

Could you recommended me a functional/immutable way of doing this?

NOTE: I've simplified the code to show the actual problem, the complete code is this:

val trainVocabulart = sentences./:(Vocabulary()){
  (vocab, s) =>
    var trees = s.tree
    var i = 0
    var noConstruction = false

    trees./:(vocab){
      (v, n) =>
        if (i == trees.size - 1) {
          if (noConstruction) return v
          noConstruction = true
          i = 0
        } else {
          // Build vocabulary
          val updatedVocab = buildVocabulary(trees, v, i, Config.LeftCtx, Config.RightCtx)

          val y = estimateTrainAction(trees, i)

          val (newI, newTrees) = takeAction(trees, i, y)

          i = newI
          trees = newTrees

          // Execute the action and modify the trees
          if (y != Shift)
            noConstruction = false

          Vocabulary(v.positionVocab ++ updatedVocab.positionVocab,
            v.positionTag ++ updatedVocab.positionTag,
            v.chLVocab ++ updatedVocab.chLVocab,
            v.chLTag ++ updatedVocab.chLTag,
            v.chRVocab ++ updatedVocab.chRVocab,
            v.chRTag ++ updatedVocab.chRTag)
        }
      v
    }
}

And the old one:

for (s <- sentences) {
  var trees = s.tree
  var i = 0
  var noConstruction = false
  var exit = false
  while (trees.nonEmpty && !exit) {
    if (i == trees.size - 1) {
      if (noConstruction) exit = true
      noConstruction = true
      i = 0
    } else {
      // Build vocabulary
      buildVocabulary(trees, i, LeftCtx, RightCtx)

      val y = estimateTrainAction(trees, i)

      val (newI, newTrees) = takeAction(trees, i, y)

      i = newI
      trees = newTrees

      // Execute the action and modify the trees
      if (y != Shift)
        noConstruction = false
    }
  }
}
2
  • 1
    Here's an idea: you can use either a simple recursion or construct a stream that will be lazily evaluated and terminate upon some condition. I would write a real piece of code, but it's kinda hard without the real data. Is the code publicly available? Can you consider sharing? It's much easier to give refactoring advice if you can lay your hands on some working code (at least in this case:) ) Commented Nov 3, 2016 at 18:06
  • Of course, the whole project is at github.com/algui91/NLP_Dependency_Parsing, this piece of code is at src/main/scala/com/elbauldelprogramador/nlp/parser/SVMParser.scala Commented Nov 3, 2016 at 18:08

1 Answer 1

1

1st - You don't make this easy. Neither your simplified or complete examples are complete enough to compile.

2nd - You include a link to some reasonable solutions to the break-out-early problem. Is there a reason why none of them look workable for your situation?

3rd - Does that complete example actually work? You're folding over a var ...

trees./:(vocab){

... and inside that operation you modify/update that var ...

trees = newTrees

According to my tests that's a meaningless statement. The original iteration is unchanged by updating the collection.

4th - I'm not convinced that fold is what you want here. fold iterates over a collection and reduces it to a single value, but your aim here doesn't appear to be finding that single value. The result of your /: is thrown away. There is no val result = trees./:(vocab){...

One solution you might look at is: trees.forall{ ... At the end of each iteration you just return true if the next iteration should proceed.

Sign up to request clarification or add additional context in comments.

4 Comments

Thanks for your answer, actually, the value returned by trees./: is assigned to trainVocabulart
Ah, so it is. I missed that. The SO code window isn't long enough to allow viewing the entire example without scrolling.
It is writing a tail recursive function a better way of approaching this problem?
It could well be. Erik Meijer calls recursion the GOTO of functional programming, but it does offer more complete control over iteration/termination as well as which elements to evaluate and which ones to ignore.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.