4
\$\begingroup\$

I wrote a Tamagotchi-like game where to feed the pet, you do math questions instead of just pressing a button. It's persistent, in that on loading, it figures out long it's been since the last save, and "simulates" the elapsed time. Hunger increases over time, and either causes the pet to heal, or become hurt, depending on if they're starving or not.

Pet.scala:

package pet

case class Pet(
        health: Float,
        hunger: Float,
        exp: Int,
        lastTime: Long
        ) {

    def setHealth(newHealth: Float): Pet = copy(
        health = newHealth
    )
    def setHunger(newHunger: Float): Pet = copy(
        hunger = newHunger
    )
    def setTime(newTime: Long): Pet = copy(
        lastTime = newTime
    )
    def setExp(newExp: Int): Pet = copy(
        exp = newExp
    )

    def healBy(by: Float): Pet = setHealth( (health + by) clampTo (0,100) )
    def hurtBy(by: Float): Pet = healBy(-by)

    def feed(food: Float): Pet = setHunger( (hunger - food) clampTo (0,100) ) 
    def starveBy(by: Float) : Pet = feed(-by)

    def addExp(newExp: Int): Pet = setExp(exp + newExp)
    def subExp(toSub: Int): Pet = addExp(-toSub)

    def isDead: Boolean = health <= 0
    def isStarving: Boolean = hunger >= 100
    def isFull: Boolean = !isStarving

    override def toString: String = s"Pet (HP: ${health}, Hunger: ${hunger}, Exp: ${exp})"
}

Enviro.scala:

package pet

import java.io._
import scala.util.{ Try, Success, Failure }

case class Enviro(
        pet: Pet,
        settings: Settings
        ) {

    def setPet(newPet: Pet): Enviro = copy(
        pet = newPet
    )

    //Returns how long in seconds it's been since the last save
    def getSecsElapsed: Long =
        Time.getCurSecs - pet.lastTime

    def checkSaveFile =
        if (!settings.saveExists) save

    def save =
        Serialize(settings.savePath).write(pet)

    def load: Enviro = setPet(
        if (settings.saveExists) {
            Try[Pet] (Serialize(settings.savePath).read.asInstanceOf[Pet])
                match {
                    case Success(p) => p
                    case Failure(e) => {
                        println("Save is corrupted or is an incompatible version. Resetting game :(. Error: " + e)
                        deleteSave
                        pet
                    }
            }
        }
        else pet
    )

    def deleteSave =
        if(new File(settings.savePath).delete) ()
        else println("Error deleting save")

    def feedPet(food: Float): Enviro = copy(
        pet = pet feed food
    )

    def applyMathToPet(right: Int, totalQ: Int): Enviro = setPet(
        pet.feed(settings.foodPerQuestion * right).
        addExp(right).subExp((totalQ - right) * 2)
    )


    //Gets the amount of seconds elapsed since the last save, and simulates each "step"
    //  in time to yield a "future" pet
    def simulateTime: Enviro = {
        def loop(steps:Int,p:Pet): Pet =
            if (steps <= 0) p
            else {
                val newPet: Pet =
                    if (p.isFull)
                        p.starveBy(settings.hungerPerStep).
                            healBy(settings.starvePerStep)
                    else if (p.isStarving)
                        p.hurtBy(settings.starvePerStep)
                    else { println("\"else\" condition met in simulator"); p }
                loop(steps-1,newPet)
            }
        setPet(
            loop((getSecsElapsed / settings.secondsPerStep).toInt , pet)
        )
    }




}

MathQuestions.scala:

package pet

import scala.util.Random._
import math._
import Char._

class MathQuestions(ops: List[Ops], maxNum: Int) {

    val gen = scala.util.Random

    //Returns a random question and answer in the form ("1 + 1", 2)
    def randomQuestion: (String,Int) = {
        val (n1,n2,(fnOp,strOp)) = (randomNum,randomNum,randomOp)
        ( n1 + " " + strOp + " " + n2, fnOp(n1,n2) )
    }

    //Same as randomQuestion; but disregards the sign of the answer
    //  for ease of entry
    def randomAbsQuestion: (String,Int) = {
        val (q,a) = randomQuestion
        (q,abs(a))
    }

    def timedQuestionLoop(n: Int): (Int,Float) = {
        val startT = Time.getCurSecs
        ( questionLoop(n) , (Time.getCurSecs - startT) )
    }

    //Asks n questions, then returns how many were right
    def questionLoop(n:Int): Int = {        
        var right = 0
        for(_<- 1 to n) {
            val (ques,ans) = randomAbsQuestion
            print(ques + " = ")
            val guess = safeReadInt //Discards invalid characters, then converts
            if (guess == ans) {
                println("Right\n")
                right += 1
            } else println(s"Wrong (${ans})")
        }
        right
    }

    def randomOp: ((Int,Int) => Int,String) =
        ops( gen.nextInt(ops.length) )

    def randomNum: Int =
        gen.nextInt(maxNum)

}

object MathQuestions {
    def apply(ops:List[Ops],max:Int): MathQuestions =
        new MathQuestions(ops,max)

    //Standard operations and their string representations
    def stdOps = List[Ops](
        ((_+_),"+"),
        ((_-_),"-"),
        ((_*_),"*")
    )
}

Serialize.scala:

package pet

import java.io.{    ObjectInputStream, ObjectOutputStream,
                    FileInputStream, FileOutputStream   }
//Convenience class to make serializing to file easier

class Serialize(path:String) {
    type S >: Serializable

    def write(obj:S) = {
        val os = new ObjectOutputStream(new FileOutputStream(path))
        os.writeObject(obj)
        os.close
    }

    def read: Object = {
        val is = new ObjectInputStream(new FileInputStream(path))
        val r = is.readObject
        is.close
        r
    }

}

object Serialize {
    def apply(path:String) = new Serialize(path)
}

Settings.scala:

package pet

import java.io.File

case class Settings(
        hungerPerStep: Float,
        starvePerStep: Float,
        secondsPerStep: Float,
        foodPerQuestion: Float,
        savePath: String
        ) {

    def saveExists: Boolean =
        new File(savePath).exists

    def setPath(newPath:String): Settings = copy(
            savePath = newPath
    )

    def setHPS(newHPS: Float): Settings = copy(
        hungerPerStep = newHPS
    )

    def setSPS(newSPS: Float): Settings = copy(
        starvePerStep = newSPS
    )



}

Time.scala:

package pet

import java.util.Date

object Time {
    def getCurMS: Long = (new Date).getTime

    def getCurSecs: Long =
        getCurMS / 1000

    def getCurMins: Long =
        getCurMins / 60

}

pet.scala:

import io.StdIn._

//Default Package

package object pet {

    //(Binary Operator, String Representation)
    type Ops = ((Int,Int) => Int, String)

    implicit class RichFloat(n: Float) {
        def clampTo(nMin: Float, nMax: Float): Float =
            if (n < nMin) nMin
            else if (n > nMax) nMax
            else n
    }

    //Converts input symbols to an Int, or 0 if all non-digits
    def safeReadInt: Int = fixInput(readLine)

    def removeNonNums(str:String): String = str.foldLeft("")(
        (acc:String,c:Char) =>
            if (c.isDigit) (acc + c)
            else acc
    )

    //Makes a string safe to read as an Int
    def fixInput(str:String): Int = (
        if (str.isEmpty) "0"
        else if (str forall (_.isDigit)) str
        else removeNonNums(str)
    ).toInt
}

Main.scala:

package pet

import io.StdIn._

object Main {
    def main(args: Array[String]) = {
        val s = Settings(
            hungerPerStep = 0.2F,
            starvePerStep = 1F,
            secondsPerStep = 60F,
            foodPerQuestion = 2,
            savePath = ".//saveFile.txt"
        )

        val newPet = Pet(
                health = 50,
                hunger = 30,
                lastTime = Time.getCurSecs,
                exp = 0
        )

        val e = Enviro(
            pet = newPet,
            settings = s
        )

        e.checkSaveFile

        val futureEnviro = e.load.simulateTime

        println(futureEnviro.pet)

        val checkedEnviro = 
            if (futureEnviro.pet.isDead) {
                println(s"They died\n\t(${futureEnviro.pet}")
                futureEnviro.deleteSave
                futureEnviro.setPet(newPet)
            } else futureEnviro

        mainLoop(checkedEnviro)

        def mainLoop(curEnviro: Enviro): Unit = {

            val questions = MathQuestions(MathQuestions.stdOps,9)

            print("How many questions do you want to do?\n\t")
            val nQ = safeReadInt

            val (nRight,seconds) = questions.timedQuestionLoop(nQ)
            val nWrong = nQ - nRight

            println(s"${nRight} / ${nQ} right")
            println(s"${nQ} questions in ${seconds} seconds (${seconds / nQ} seconds / question)")

            val foodPerQ = 2

            val finalEnviro = curEnviro.applyMathToPet(nRight,nQ)

            //val finalEnviro = curEnviro.feedPet(nRight * foodPerQ)

            println(finalEnviro.pet)

            finalEnviro.save
            println("Saved\n\n")

            mainLoop(finalEnviro)
        }

    }
}

I'd like tips on readability, or how to make it more functional. I'm going to replace the impure random generator with something seed-based and pure later, so that can be ignored.

I'd also like tips on safety:

  • safeReadInt seems to be safe, and I have yet to have it crash, but if there are any scenarios that would allow it to pass an invalid string to toInt, I'd like to know.
  • I'm using Try for the first time in Enviro.load to account for corrupted/incompatible saves. It turned out a little ugly, so any help in cleaning it up would be appreciated.
\$\endgroup\$
  • \$\begingroup\$ Please don't edit your code. Some people might be writing a review and their review will not correspond to the posted code. You could wait for some feedback and post a follow up with better code in a few days. \$\endgroup\$ – toto2 Jan 3 '15 at 2:10
  • \$\begingroup\$ @toto2 Sorry. Given the length of the code and the lack of attention that it's getting, I figured that it would be safe, but you're right. Were you partially through a review, so I should roll it back? \$\endgroup\$ – Carcigenicate Jan 3 '15 at 2:20
  • \$\begingroup\$ No, it's OK at the moment. But I will post a review shortly. \$\endgroup\$ – toto2 Jan 3 '15 at 2:25
2
\$\begingroup\$

Scala Style

  • You don't need the setters, just use copy directly.

  • You might be able to use pattern matching instead of if/else's in the utility methods in pet.scala.

  • For pet.hurtBy and pet.starveBy, return Option[Pet] where None means that the pet is dead. What you are currently doing is like you would do in Java where you remove the value and then checks if it is dead. But the Java way is basically wrong. You don't ever want a pet in a dead state. You are forced to handle death as soon as it happens.

  • Similarly, you should use Option when attempting to read the file.

Separation of Concern

  • Settings has some pet parameters and then a file name. Those two things do not belong together. (More comments on this later.)

  • You should cleanly separate the GUI logic from the "business logic". You do everything on the console at the moment, but you might move to a JavaFX GUI later. (Even if you don't change GUI, you should always code "as if" to keep a good separation of concerns.) You don't want to have to modify any of your business classes when you change the GUI. For example, MathQuestion has a method questionLoop which does not belong in that class.

OO Design

I would have done the Pet, Settings and Enviro OO design differently.

I would put secondsPerStep andfoodPerQuestion in some general class (maybe called Settings) that is applicable to all pets. I would put hungerPerStep and starvePerStep as parameters with Pet. You might want to have variations in your pets and draw those values randomly for each pet.

I would not put those value in the same class as the file name. Also, you are persisting the file name which does not make much sense since if you ever change the file path in the main loop, the saved values would be wrong.

Naming

  • Settings should probably be named State. You are persisting the state.

  • generateQuestion instead of randomQuestion. Method names are usually verbs.

  • Serializer instead of Serialize. Class names are usually nouns.

  • type Op instead of type Ops. Type names are usually singular.

  • utils.scala instead of pet.scala since those are utility methods.

  • futureEnviro is really currentEnviro in Main.

UI

  • Instead of asking how many questions to ask at the start, maybe you could ask the user if she/he wants to continue after each question. For example, answering 'q' to a question would stop immediately, without counting as a wrong answer. After each question, you could then display the current pet state.

Others

  • Instead of your Time utilities, I would just stick with System.currentTimeMillis(). It returns the number of milliseconds since January 1, 1970 UTC as a long. It might seem odd, but it is the default time format which is used nearly everywhere. I think it's better simply because it is more "standard".

  • I think there is too much file loading/saving in Main. I would only have one load attempt at the start (as an Option with a fallback pet creation method on None) and only one save after the main loop.

  • I don't like Enviro.load which sets the internal state of Enviro. It's not quite functional. It would be better to just have a method that returns an Option or Try of the persisted value. Also, I would only persist a tuple (pet, settings), where settings is itself just secondsPerStep andfoodPerQuestion, as discussed above. Even better: settings should actually just be hard coded values, or at least if they are persisted values, they should be persisted separately from the pets.

\$\endgroup\$
  • \$\begingroup\$ Thank you. I'll go over the answer in detail later and comment if I have any questions regarding your suggestions. \$\endgroup\$ – Carcigenicate Jan 3 '15 at 21:22

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.