-1

i'm writing a method that take a list: List[(String, Int)] , x:(String, Int) and a n: Int parameters and return a List[(String, Int)] The list parameter represents the input list, the x element represents the element to add to the list, the n represents the maximum dimension of the list.

The method has the following behavior:

  1. First of all check if the list has n elements.
  2. If list has less than n elements, the method add the x elements to the list.
  3. Otherwise if the list contain an elements less than x, the method remove the minimum element from the list, and add the x element to the list.

I've implemented the method as the following:

def method(list: List[(String, Int)], x: (String, Int), n: Int) = {
            if(list.size < n)
                list :+ x
            else {
               var index = -1
               var min = 10000000//some big number
               for(el <- 0 to list.size) {
                 if(list(el)._2 < x._2 && list(el)._2 < min) {
                    min = list(el)._2
                    index = el
                  }
               } 
               if(index != -1) {
                   val newList = list.drop(index)
                   newList :+ x
               }
               else list


              }


        }

Exist a way to express this behavior in a more clean way??

alberto adami
  • 729
  • 1
  • 6
  • 25
  • from the code, if the list is more than n elements, but there are no existing elements less than x, then return the list unchanged? If so, then you're really asking for something that returns the biggest N elements of all the x's passed? Do you have all the Xs in a list already? – The Archetypal Paul Oct 10 '14 at 14:41
  • In other words, would something that returns the topN items of a list fit your use-case? – The Archetypal Paul Oct 10 '14 at 15:01
  • And your code as posted doesn't compile :( It's got Java-style declaration (`int index`) and never defines `el`. Can you post your actual code? – The Archetypal Paul Oct 10 '14 at 15:07
  • And `list.drop(i)` doesn't do what you think (remove the element at index `i`). It returns all but the first `i` elements of the list – The Archetypal Paul Oct 10 '14 at 15:16
  • Your edited version still uses .drop to remove the i'th element. It doesn't. – The Archetypal Paul Oct 10 '14 at 15:52
  • Does the ordering matter? Right now, things are a lot slower thant they need be because you add x to the end of the list, which is O(N). Adding it to the start is O(1). – The Archetypal Paul Oct 10 '14 at 16:11
  • The order doesn't matter. Now i've update the code, now it compile – alberto adami Oct 13 '14 at 07:18
  • 1
    OK. then there's a better answer that maintains an ordered list of the top n. Then you just insert the new value in the list, if it's greater than something already there, and drop the least value if you inserted something and the list was more than 'n' long. A collection with O(1) size would be better, too. – The Archetypal Paul Oct 13 '14 at 09:05

2 Answers2

1

First, a corrected version of what you posted (but it appears, not what you intended)

def method(list: List[(String, Int)], x: (String, Int), n: Int) = {
    if(list.size < n)
        list :+ x
    else {
       var index = -1
       for(i <- 0 until list.size) {
         val el = list(i)
          if(el._2 < x._2)
            index = i
       }
       if(index != -1) {
           val (before, after) = list.splitAt(index)
           val newList = before ++ after.tail
           newList :+ x
       }
       else list
      }
}   

And some tests

val l1 = List(("one", 1))            

val l2 = method(l1, ("three", 3), 2)
//> l2  : List[(String, Int)] = List((one,1), (three,3))
val l3 = method(l2, ("four", 4), 2)
//> l3  : List[(String, Int)] = List((one,1), (four,4))
val l4 = method(l2, ("zero", 0), 2)
//> l4  : List[(String, Int)] = List((one,1), (three,3))

Neater version (but still not meeting the spec as mentioned in a comment from the OP)

 def method(list: List[(String, Int)], x: (String, Int), n: Int) = {
    if (list.size < n)
      list :+ x
    else {
      val (before, after) = list.span(_._2 >= x._2)
      if (after.isEmpty)
        list
      else
        before ++ after.tail :+ x
    }
  } 

Another version that always removes the minimum, if that's less than x.

 def method(list: List[(String, Int)], x: (String, Int), n: Int) = {
    if (list.size < n)
      list :+ x
    else {
      val smallest = list.minBy(_._2)
      if (smallest._2 < x._2) {
        val (before, after) = list.span(_ != smallest)
        before ++ after.tail :+ x
      } else
        list
    }
  }

and its test results

val l1 = List(("one", 1))


val l2 = method(l1, ("three", 3), 2) 
//> l2  : List[(String, Int)] = List((one,1), (three,3))
val l3 = method(l2, ("four", 4), 2) 
//> l3  : List[(String, Int)] = List((three,3), (four,4))
val l4 = method(l2, ("zero", 0), 2)
//> l4  : List[(String, Int)] = List((one,1), (three,3))

but as I say in a comment, better to use a method that processes the whole sequence of Xs and returns the top N.

The Archetypal Paul
  • 41,321
  • 20
  • 104
  • 134
  • The var l3 = method(l2, ("four", 4), 2) statement should return l3: List[(String, Int)]= List((three, 3), (four, 4)), because the (one, 1) element is the smallest in the list. – alberto adami Oct 10 '14 at 15:33
  • 1
    OK, but that's not what the code you posted does. It finds the last element that's smaller, not the smallest. And your question also says just that an element smaller than x needs to be removed. Please update your question. – The Archetypal Paul Oct 10 '14 at 15:35
  • Sorry for the late, but now i've update the specs of the question. – alberto adami Oct 13 '14 at 07:16
-1

I would separate the ordering from the method itself:

def method[A: Ordering](list: List[A], x: A, n: Int) = ...

What do you want to do if the list contains more than one element less than x? If you're happy with replacing all of them, you could use map rather than your handwritten loop:

list map {
    case a if Ordering.lt(a, x) => x
    case a => a
}

If you only want to replace the smallest element of the list, maybe it's neater to use min?

val m = list.min
if(m < x) {
  val (init, m :: tail) = list.splitAt(list.indexOf(m))
  init ::: x :: tail
else list
lmm
  • 17,386
  • 3
  • 26
  • 37
  • This simply ignores the "if the list has less than n elements" part of the problem. – The Archetypal Paul Oct 10 '14 at 14:38
  • Yes, because I don't know any way to improve that. I'm suggesting possible improvements, not offering a complete solution - I can't offer a complete solution because as I said, it's not clear what the desired behaviour is in some cases. – lmm Oct 10 '14 at 14:41
  • I think you should have said you were proposing only a partial solution. But given we have an implementaiton from the OP, I think it is reasonably obvious what the desired behaviour us – The Archetypal Paul Oct 10 '14 at 14:43
  • And the comparison is wrong. You're comparing the tuple, but it's supposed to be the second (Int) element that is being compared – The Archetypal Paul Oct 10 '14 at 15:09
  • In my problem I want to find the first n tags in the db, so i used this method to find out that. So i want to replace the smallest element of my array. A tag is in the form (String, Int), where the String is the name of the tag, and the Int is how much times the tag compare in the db. – alberto adami Oct 10 '14 at 15:29
  • Ah, so you do have a sequence of Xs. So you want a 'topN' method for a sequence. That's already been answered: http://stackoverflow.com/questions/8274726/top-n-items-in-a-list-including-duplicates – The Archetypal Paul Oct 10 '14 at 15:41
  • It's not wrong. I'm comparing according to the `Ordering` instance passed in. It will behave correctly when passed an appropriate `Ordering`. – lmm Oct 10 '14 at 15:45
  • Then define an appropriate ordering and show some code that's complete and works, as I think what you have now doesn't. There is no .lt method on the Ordering companion object. You need to pass in an ordering (possibly implicitly). – The Archetypal Paul Oct 10 '14 at 15:50