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

Get tests running & give pseudocode in tests like Java assessment #109

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

twinlensreflex
Copy link

@twinlensreflex twinlensreflex commented Jul 16, 2024

What?

  1. Updated sbt versions for Apple Silicon, updated Play version
  2. Add pseudocode to tests showing desired assertions

Why?

  1. So that scala tests & play code run
  2. The scala test set up was a little complex to get your head around so we copied the pseudocode assertion from the Java test cases as it is a little less confusing

@twinlensreflex twinlensreflex changed the title tests running Get tests running & give pseudocode in tests like Java assessment Sep 30, 2024
@@ -45,7 +45,6 @@ class ResultsController @Inject()(val controllerComponents: ControllerComponents
}

def getScoreboard: Action[AnyContent] = Action {

Ok(Json.toJson(Scoreboard(0)))
Ok(Json.toJson(None))

Choose a reason for hiding this comment

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

From what I recall from Python & Java, this is "supposed" to fail to begin with because Scoreboard isn't adequately implemented/integrated yet. I'm not sure that's the right way but it is consistent

// - the overall winner (if there is one)
// - the seats that each party wins in Parliament
case class Scoreboard(winner: String)

Choose a reason for hiding this comment

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

Do you want winner parameter here? Feels like prejudicing the approach taken...

### Possible other implementations

- Absolute majority required. Someone needs 50% + 1 votes or a run off is triggered (check the data that's probably all constituencies)
- Allocate the seats from the total declarations based on % of vote share

Choose a reason for hiding this comment

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

Agree, the whole "other implementations" section looking at other voting systems is a bad idea (but! for fairness/consistency we should edit tasks.md for other languages)

// LD == 62
// LAB == 349
// CON == 210
// winner = LAB
}

Choose a reason for hiding this comment

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

👍 on having comments stating assertions needed & letting the candidate implement (as in other languages)

@@ -21,7 +21,7 @@ The API has 3 endpoints:

During your assessment we will ask you to work though the task in `tasks.md` with a pair. Please do not work on or complete these prior to the assessment.

:warning: If you make any changes to the code, please ensure you return it to it's initial (HEAD) state before your assessment.
__Warning:__ If you make any changes to the code, please ensure you return it to it's initial (HEAD) state before your assessment.

Choose a reason for hiding this comment

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

it's initial state

Its. Not it's. Oh dear.

@timsweetman
Copy link

In my opinion:

  • one PR for the scala-specific edits, which all look fine to me
  • another for tasks edits which should span all languages

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