HiveBrain v1.2.0
Get Started
← Back to all entries
patternMinor

Beginner Project: Bunny City

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
citybeginnerprojectbunny

Problem

I am a Java programmer, and I just recently started learning Scala for fun. I found a group of projects here, and I tried to do the graduation excercise. The problem is, my code looks a lot like java, and I want to make this code more styled to fit scala, though I'm not sure what that entails. (Making it more functional?)

This is my main game class:

object Game {

  def main(args: Array[String]): Unit = {
    val list : ListBuffer[Bunny] = ListBuffer.fill(5)(BunnyFactory.getInitial)

    while(list.size != 0){

      var iterator = 0;
      //make sure we have one male for mating
      var males = 0;
      //keep track of females for mating
      var females : ListBuffer[Bunny] = new ListBuffer

      for(i = 1){
        for(i <- females){

          val child = BunnyFactory.getChild(i);
          println(s"Bunny ${child.name} was born!")
          list += child
        }
      }
      println(s"There are ${list.size} bunnies!")

      Thread.sleep(500)
    }

  }
}


My Bunny class (I'm not sure if my variable naming follows conventions here):

class Bunny(inColor : Color.Value, inGender : Gender.Value, inAge : Int, inName : String, radioactive : Boolean){
    private val _color = inColor;
    private val _gender = inGender;
    private val _name = inName;
    private var _radioactive = radioactive;
    private var _age = inAge;

    def gender = _gender
    def age = _age
    def name = _name
    def adult : Boolean = {_age >= 2 && !(_radioactive)}
    def color  = _color

    def dead : Boolean = {
      if(_radioactive){
        _age > 50;
      }else{
        _age > 10;
      }
    }

    def infect : Unit = {_radioactive = true}

    def update(list : ListBuffer[Bunny]) : Unit = {
      _age += 1
      if(_radioactive){
         list((Math.random() * list.length).toInt).infect
      }
    }
    override def toString() : String = {
      return s"${_color} ${_gender} Bunny ${_name}, ${_age} years old, Radioactive: ${_radioactive}"
    }
}


Bunn

Solution

Let me start with some suggestions to improve your code a little bit in regards of scala/functional style. I will start with by class with the points that catched my eyes first and i might edit my post later on and add additional points.

Class structure and constructors

Instead of going with

class Bunny(inColor : Color.Value, inGender : Gender.Value, inAge : Int, inName : String, radioactive : Boolean){
    private val _color = inColor;
    private val _gender = inGender;
    private val _name = inName;
    private var _radioactive = radioactive;
    private var _age = inAge;


you can write it more concise like this:

class Bunny(val color : Color.Value, val gender : Gender.Value, var age : Int, val name : String, var radioactive : Boolean){


No need to declare them with other names inside the class. Notice however, that public getters and setters (for vars) will be created automatically. Going private val color : Color.Value on the other hand will create a private getter.

Instead of creating the adult variable on the fly (def adult : Boolean = {_age >= 2 && !(_radioactive)}) it may be better to make the main constructor accept an adult variable and to create a second constructor (which calls the main constructor) that does the logic above. One important benifit is, that it will be more easy to test when you can pass in the adult variable as is.

The getChild method in your BunnyFactory class seems to belong to your Bunny class. You could consider making a class for male and female bunnies because they behave different although they are both bunnies. Traits could be a good option here to add a functionality only to the female bunny class for creating new child bunnies.

Naming

In your Bunny class you have a method update(list : ListBuffer[Bunny]) : Unit. I would call that different. Maybe infectNeighbours or similar.

In your BunnyFactory class you could call the getInitial method createRandomBunny or so. Because getInitial makes me think "huh? What does it do?". Same for getChild

In your main game class don't use list but rather existingBunnies or something like this.

Shortcuts

In the toString method of Bunny class you use return s"${_color} ${_gender} Bunny ${_name}, ${_age} years old, Radioactive: ${_radioactive}".
The more scalaish way to do this is to omit the "return". It is not needed. The result of the last expression is used as return value anyways. Same goes for most of the other returns.

Now to your main game class.
I personally prefer using list methods instead of the for comprehension in most cases. So instead of for(i <- list){ I would do a list.forEach. But first what do we want? For example, do we really want our already dead bunnies to infect other bunnies before we remove them from the list?

Here comes the "functional thinking" into to game (assuming our var list is called existingBunnies):

//lets first get a list of bunnies that are still alive
val livingBunnies = existingBunnies.filter{ bunny =>
    if (bunny.dead)
        println(s"Bunny ${bunny.name} is dead with an age of ${bunny.age}!")

    bunny.dead == false
}

//Now create a list of bunnies that are born new. That can be solved using not var but val in a more pure functional style but to me that will make it more complicated so i stayed with the if condition.
var newBornBunnies = List() //We have no new born bunnies by default

//But if there is at least one mal left, each female bunny will create a child
if(livingBunnies.count(_.gender == Gender.Male) >= 1) {
    newBornBunnies = livingBunnies.filter( _.adult && _.gender == Gender.Female ).map( _ createChild )
}

//We add the new born bonnies to the existing living ones
existingBunnies = livingBunnies ++ newBornBunnies

//At the end with our new list of existing bunnies we let them infect each other
existingBunnies forEach (_ infectNeighbours existingBunnies)

Code Snippets

class Bunny(inColor : Color.Value, inGender : Gender.Value, inAge : Int, inName : String, radioactive : Boolean){
    private val _color = inColor;
    private val _gender = inGender;
    private val _name = inName;
    private var _radioactive = radioactive;
    private var _age = inAge;
class Bunny(val color : Color.Value, val gender : Gender.Value, var age : Int, val name : String, var radioactive : Boolean){
//lets first get a list of bunnies that are still alive
val livingBunnies = existingBunnies.filter{ bunny =>
    if (bunny.dead)
        println(s"Bunny ${bunny.name} is dead with an age of ${bunny.age}!")

    bunny.dead == false
}

//Now create a list of bunnies that are born new. That can be solved using not var but val in a more pure functional style but to me that will make it more complicated so i stayed with the if condition.
var newBornBunnies = List() //We have no new born bunnies by default

//But if there is at least one mal left, each female bunny will create a child
if(livingBunnies.count(_.gender == Gender.Male) >= 1) {
    newBornBunnies = livingBunnies.filter( _.adult && _.gender == Gender.Female ).map( _ createChild )
}

//We add the new born bonnies to the existing living ones
existingBunnies = livingBunnies ++ newBornBunnies

//At the end with our new list of existing bunnies we let them infect each other
existingBunnies forEach (_ infectNeighbours existingBunnies)

Context

StackExchange Code Review Q#52483, answer score: 3

Revisions (0)

No revisions yet.