patternMinor
Beginner Project: Bunny City
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:
My Bunny class (I'm not sure if my variable naming follows conventions here):
Bunn
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
you can write it more concise like this:
No need to declare them with other names inside the class. Notice however, that public getters and setters (for
Instead of creating the
The
Naming
In your
In your
In your main game class don't use
Shortcuts
In the
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
Now to your main game class.
I personally prefer using list methods instead of the for comprehension in most cases. So instead of
Here comes the "functional thinking" into to game (assuming our
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 getChildIn 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.