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.

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

Conversation

ricemery
Copy link
Member

  1. Rework interface definition to cleanly show openAccount.
  2. Add HINTS.md with note about mutable state.
  3. Add Bank.scala with BankAccount trait that should be implemented, and Bank object that should be filled out by the implementer with BankAccount construction.
    Refs BankAccount: not scala-ish enough? #214, Add hints to some exercises #220, Exercises do not provide a named file for the code to be delivered #137

## 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.

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..

@ricemery
Copy link
Member Author

I renamed Bank.scala to BankAccount.scala . At least now the source file is named to match the test cases file.
I also added the "Common pitfalls" text to the HINTS.

Thanks!

@ErikSchierboom ErikSchierboom merged commit d4f60ce into exercism:master Dec 1, 2016
@ricemery
Copy link
Member Author

ricemery commented Dec 1, 2016

Thanks!

@ricemery ricemery deleted the BankAccount branch January 17, 2017 13:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants