orshachar
orshachar

Reputation: 5037

how to make the following scala immutable

Suppose I have the following code:

BroadcastMessage gets a list of groups of students in the form of iterators that can only be traversed once
On call to sendMessage it will send message to all students in the groups.

class BroadcastMessage(message:String,groups:List[Iterator[Student]]) {

  def sendMessage:Unit = {
      groups.foreach(group=>group.foreach(sendeMessage))
  }

  private def sendMessage(student:Student): Unit ={
    EmailClient.sendMessage(student.email,message)
  }

}

case class Student(id: String,email:String)

Say that student can exist in several groups and we don't want to send him more than one email.

The mutable solution would be to add mutable set and to add the id of the student to the set and only send the message if the id exists in the set.

class BroadcastMessage(message:String,groups:List[Iterator[Student]]) {
  // evil mutable set
  private var set:scala.collection.mutable.Set[String] = Set()

  def sendMessage:Unit = {
      groups.foreach(group=>group.foreach(sendeMessage))
  }

  private def sendMessage(student:Student): Unit ={
    if (set.add(student.id)) {
      EmailClient.sendMessage(student.email, message)
    }
  }

}

How do I implement it in immutable way?

Upvotes: 0

Views: 75

Answers (5)

Federico Pellegatta
Federico Pellegatta

Reputation: 4017

You can do it with a one-liner:

def sendMessage: Unit = 
  groups.reduce(_ ++ _).toStream.distinct.foreach(sendMessage)

Expanded version for learning purpose:

val students: Iterator[Student] = groups.reduce(_ ++ _)
val sStudents: Stream[Student] = students.toStream
val dStudents: Stream[Student] = sStudents.distinct
def sendMessage: Unit = sStudents.foreach(sendMessage)

Upvotes: 0

stefanobaghino
stefanobaghino

Reputation: 12804

It looks like what you are looking for is all unique Students in the nested collection.

A very simple way to do this is by flattening the collection and converting it to a Set; here is an example with Ints:

scala> val groups = List(Iterator(1,2,3), Iterator(3,4,5))
groups: List[Iterator[Int]] = List(non-empty iterator, non-empty iterator)

scala> val unique: Set[Int] = groups.flatten.toSet
unique: Set[Int] = Set(5, 1, 2, 3, 4)

A problem here is that the toSet methods actually copies your list. To avoid this, you can use this little trick (you can read more about collection.breakOut and CanBuildFrom here):

scala> val unique: Set[Int] = groups.flatMap(identity)(collection.breakOut)
unique: Set[Int] = Set(5, 1, 2, 3, 4)

However, the source of mutability here is the usage of an Iterator which will be consumed anyway, there mutating and breaking referential transparency.

Upvotes: 0

orshachar
orshachar

Reputation: 5037

I managed to do do this. I think I lost some readability but it is mutable:

class BroadcastMessage(message: String, groups: List[Iterator[Student]]) {

  def sendMessage(): Unit = {
    groups.foldLeft[Set[String]](Set.empty)(sendMessage)
  }

  private def sendMessage(sent: Set[String], group: Iterator[Student]):Set[String] = {
    group.foldLeft[Set[String]](sent)(sendMessage)
  }

  private def sendMessage(sent: Set[String], student: Student): Set[String] = {
    if (!sent.contains(student.id)) {
      EmailClient.sendMessage(student.email, message)
      return sent + student.id
    }
    sent
  }

}

Upvotes: 1

mdm
mdm

Reputation: 3988

Well, I think that you are making two different mutable things in your example, while you only really need one.

You either need a private val set : mutable.Set[Student] = mutable.Set.empty[Student] or a private var set : Set[Student] = Set.empty[Student]. That is, you either need to mutate the set itself or just the reference to it that your class holds. I would personally go for the latter, ending up with something like:

case class Student(id: String,email:String)

class BroadcastMessage(message:String,groups:List[Iterator[Student]]) {
  private var set : Set[Student] = Set.empty // <- no mutable Set, just a mutable reference to several immutable Sets

  def sendMessage:Unit = {
    groups.foreach(group=>group.foreach(sendMessage))
  }

  private def sendMessage(student:Student): Unit = {
    if (!set(student)) {
      set = set + student
      EmailClient.sendMessage(student.email, message)
    }
  }
}

Finally, you could even get rid of the sendMessage(student : Student) method all together:

case class Student(id: String,email:String)

class BroadcastMessage(message:String,groups:List[Iterator[Student]]) {
  private var set : Set[Student] = Set.empty

  def sendMessage:Unit = {

    val students = (for{
      group <- groups
      student <- group
    } yield student).toSet

    val toBeNotified = (students -- set)
    toBeNotified.foreach(student => EmailClient.sendMessage(student.email, message))

    set = set ++ toBeNotified
  }
}

I guess it depends on style then...

Upvotes: 2

mavarazy
mavarazy

Reputation: 7735

If you have no memory restrictions, you can just do this:

def sendMessage:Unit = {
    groups.flatten.distinct.foreach(sendMessage)
}

Upvotes: 0

Related Questions