Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rework interface definition to cleanly show openAccount. Add HINTS.md… #221

Merged
merged 3 commits into from
Dec 1, 2016
Merged

Rework interface definition to cleanly show openAccount. Add HINTS.md… #221

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6881631
Rework interface definition to cleanly show openAccount. Add HINTS.md…
ricemery Nov 30, 2016
2ef6e78
Update HINTS.md
ricemery Nov 30, 2016
239650a
Rename Bank.scala to BankAccount.scala
ricemery Nov 30, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions exercises/bank-account/HINTS.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
## Hints

This exercise is testing mutable state that can be accessed saftely from multiple threads. Scala provides a variety of ways to protect
mutable state. For developers familiar with Java concurrency, Scala can utilize the Java concurrency support such as the Java synchronized block.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Although this might already be some sort of spoiler you could add something like:

In Scala there are two ways to achieve mutable state: Use a "var" or a mutable object.
Two common mistakes here are:
- Do not use a "var" that is also a mutable object. One is enough, but not both together.
- Don't expose the "var" or mutable object to the outside world. So make them "private" and change the mutable object into immutable before you return it as a value.

One could also imagine a standard format with a separate section named "Common Pitfalls" or so.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The suggested addendum by @abo64 seems like a good one, even though it is a bit of a spoiler.

6 changes: 2 additions & 4 deletions exercises/bank-account/example.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,6 @@ protected case class Account(var balance: Option[Int] = Some(0)) extends BankAcc
}
}

object BankAccount {
def apply(): BankAccount = Account()
object Bank {
def openAccount(): BankAccount = Account()
}


13 changes: 13 additions & 0 deletions exercises/bank-account/src/main/scala/Bank.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
trait BankAccount {

def closeAccount(): Unit

def getBalance: Option[Int]

def incrementBalance(increment: Int): Option[Int]
}

object Bank {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why don't you want to use the companion object anymore?
Same for the file name. AFAIK the file name is always the same as the class/object to be implemented?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, there is a Bank object being implemented of course, so that sort of makes sense. I usually named the files after the slug name, so for the BankAccount exercise, I would use BankAccount.scala.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I changed the name for the Object from BankAccount to Bank. I didn't like having a BankAccount.openAccount function. It seems like a Bank should be opening accounts not a BankAccount. I thought about having a Bank.scala and BankAccount.scala. But, then I didn't like having 2 .scala files for the exercise..

def openAccount(): BankAccount = ???
}

10 changes: 5 additions & 5 deletions exercises/bank-account/src/test/scala/BankAccountTest.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,12 @@ import org.scalatest.{Matchers, FunSuite}

class BankAccountTest extends FunSuite with Matchers with Conductors with IntegrationPatience {
test("open account") {
BankAccount().getBalance should be (Some(0))
Bank.openAccount().getBalance should be (Some(0))
}

test("incrementing and checking balance") {
pending
val acct = BankAccount()
val acct = Bank.openAccount()
acct.getBalance should be (Some(0))
acct.incrementBalance(10) should be (Some(10))
acct.getBalance should be (Some(10))
Expand All @@ -18,7 +18,7 @@ class BankAccountTest extends FunSuite with Matchers with Conductors with Integr

test("closed account should hold no balance") {
pending
val acct = BankAccount()
val acct = Bank.openAccount()
acct.closeAccount()
acct.incrementBalance(10)
acct.incrementBalance(10)
Expand All @@ -30,7 +30,7 @@ class BankAccountTest extends FunSuite with Matchers with Conductors with Integr
val conductor = new Conductor
import conductor._

val acct = BankAccount()
val acct = Bank.openAccount()

thread("t1") {
acct.incrementBalance(10)
Expand All @@ -54,7 +54,7 @@ class BankAccountTest extends FunSuite with Matchers with Conductors with Integr
val conductor = new Conductor
import conductor._

val acct = BankAccount()
val acct = Bank.openAccount()

thread("t1") {
for (a <- 1 to 10)
Expand Down