-
Notifications
You must be signed in to change notification settings - Fork 0
Margaux/buzzer score updater #128
Changes from 3 commits
5acc8e7
589dc80
bb279c5
5e7494e
4109b04
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||
---|---|---|---|---|---|---|---|---|
|
@@ -6,25 +6,43 @@ import org.junit.Test | |||||||
|
||||||||
public class BuzzerScoreUpdaterTest { | ||||||||
|
||||||||
// FIXME: Uncomment this when the infinite loop issue in the class is fixed | ||||||||
/* | ||||||||
@Test | ||||||||
fun constructorTestWithSameSizedArrays() { | ||||||||
val idArray = arrayOf(0, 1, 2, 3) | ||||||||
val idArray = arrayOf(R.id.buzzer_0, R.id.buzzer_1, R.id.buzzer_2, R.id.buzzer_3) | ||||||||
val scoreArray = arrayOf(0, 0, 0, 0) | ||||||||
val expectedMap = LinkedHashMap<Int, Int>() | ||||||||
expectedMap.put(0, 0) | ||||||||
expectedMap.put(1, 0) | ||||||||
expectedMap.put(2, 0) | ||||||||
expectedMap.put(3, 0) | ||||||||
val testUpdater = BuzzerScoreUpdater(scoreArray, idArray) | ||||||||
assert(testUpdater.getMap()==(expectedMap)) // value or reference? | ||||||||
expectedMap.put(R.id.buzzer_0, 0) | ||||||||
expectedMap.put(R.id.buzzer_1, 0) | ||||||||
expectedMap.put(R.id.buzzer_2, 0) | ||||||||
expectedMap.put(R.id.buzzer_3, 0) | ||||||||
val testUpdater = BuzzerScoreUpdater(idArray, scoreArray) | ||||||||
for (key in idArray) { | ||||||||
assert(testUpdater.getMap().keys.contains(key)) | ||||||||
} | ||||||||
for (value in scoreArray) { | ||||||||
assert(testUpdater.getMap().values.contains(value)) | ||||||||
} | ||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I was searching and I believe it is not correct to use java's assert in the test. Because in this case the java's exception will appear, not junit's exception. I would change assert on junit's assertTrue or assertEquals. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also here ExpectedMap is not used, so it can be deleted There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good point, I'll change this! |
||||||||
} | ||||||||
|
||||||||
@Test | ||||||||
fun arrayUpdateTest() { | ||||||||
|
||||||||
val idArray = arrayOf(R.id.buzzer_0, R.id.buzzer_1, R.id.buzzer_2, R.id.buzzer_3) | ||||||||
val scoreArray = arrayOf(0, 0, 0, 0) | ||||||||
val testUpdater = BuzzerScoreUpdater(idArray, scoreArray) | ||||||||
for (id in idArray) { | ||||||||
testUpdater.updateScoresArray(id) | ||||||||
assert(testUpdater.getMap()[id]==1) | ||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||
} | ||||||||
} | ||||||||
|
||||||||
*/ | ||||||||
@Test | ||||||||
fun arrayUpdateWithWrongIdReturns() { | ||||||||
val idArray = arrayOf(R.id.buzzer_0, R.id.buzzer_1, R.id.buzzer_2, R.id.buzzer_3) | ||||||||
val scoreArray = arrayOf(0, 0, 0, 0) | ||||||||
val testUpdater = BuzzerScoreUpdater(idArray, scoreArray) | ||||||||
testUpdater.updateScoresArray(-1) | ||||||||
for (id in idArray) { | ||||||||
assert(testUpdater.getMap()[id]==0) | ||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||
} | ||||||||
} | ||||||||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,38 @@ | ||
package ch.sdp.vibester | ||
|
||
import kotlin.math.min | ||
|
||
public class BuzzerScoreUpdater(ids: Array<Int>, scores: Array<Int>) { | ||
|
||
private var buzzerToScoreMap: LinkedHashMap<Int, Int> | ||
|
||
init { | ||
buzzerToScoreMap = initMap(ids, scores) | ||
} | ||
|
||
private fun initMap(ids: Array<Int>, scores: Array<Int>): LinkedHashMap<Int, Int> { | ||
var theMap = LinkedHashMap<Int, Int>() | ||
var i = 0 | ||
while (i < min(ids.size, scores.size)) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why do you take min here? Should not both arrays be the same size? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. When I tried to perform a same size check and then use one of the sizes in the while loop, the app kept crashing, even though I only have cases where the scores are initialized according to the number of players (so should always be the same size). I used the min function as a quick fix for this, and I'll try to implement a proper check when refactoring. |
||
theMap[ids[i]] = scores[i] | ||
i = i + 1 | ||
} | ||
return theMap | ||
} | ||
|
||
fun getMap(): LinkedHashMap<Int, Int> { | ||
return buzzerToScoreMap | ||
} | ||
|
||
/** | ||
* updates the score corresponding to the pressed buzzer | ||
* checks first if the given id | ||
*/ | ||
fun updateScoresArray(id: Int) { | ||
if (!buzzerToScoreMap.keys.contains(id)) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this check is needed as well? The array will be initialized with exactly entered number of ids, so there could not be a missing id in general... or am I missing something? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This check ensures that no update will be attempted if the parameter is equal to NO_BUZZER_PRESSED (which should not happen but I still added a check just in case). I'll update the doc to make this clearer. |
||
return | ||
} | ||
val updatedScore = buzzerToScoreMap.getOrDefault(id, 0) + 1 // should never get to default, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If the buzzer is initiated, there is no need for default, right? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I tried using the "get" function: however, it returns an Int? which is not castable to Int, even with a null check. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. also here you can update the score in buzzer directly, like that: |
||
buzzerToScoreMap.put(id, updatedScore) | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -10,11 +10,35 @@ import android.widget.LinearLayout | |
import android.widget.TableRow | ||
import android.widget.TextView | ||
import androidx.appcompat.app.AppCompatActivity | ||
import androidx.core.view.children | ||
import ch.sdp.vibester.BuzzerScoreUpdater | ||
import ch.sdp.vibester.R | ||
|
||
|
||
class GamescreenActivity: AppCompatActivity() { | ||
|
||
private val MAX_N_PLAYERS = 4 | ||
private val NO_BUZZER_PRESSED = -1 | ||
private val rowsIdArray = arrayOf(R.id.row_0, R.id.row_1, R.id.row_2, R.id.row_3) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Similar blocks of code found in 2 locations. Consider refactoring. |
||
private val buzIds = arrayOf(R.id.buzzer_0, R.id.buzzer_1, R.id.buzzer_2, R.id.buzzer_3) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Similar blocks of code found in 2 locations. Consider refactoring. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Would not really make sense to refactor, this is just declaration and initialisation of 2 arrays with values from the xml. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Agreed. Worst case, if these buzzers and rows are the only children of the layout, can iterate over them and use getChild to set their values. Leaving the comment here in case we need to do so in the future. |
||
|
||
var pressedBuzzer = NO_BUZZER_PRESSED | ||
|
||
private fun setPressed(id: Int) { | ||
pressedBuzzer = id | ||
} | ||
|
||
private fun fetchBuzToScoreRowMap(): Map<Int, Int> { | ||
|
||
var theMap = LinkedHashMap<Int, Int>() | ||
var i = 0 | ||
while (i < buzIds.size) { | ||
theMap.put(buzIds[i], rowsIdArray[i]) | ||
i = i + 1 | ||
} | ||
return theMap | ||
} | ||
|
||
override fun onCreate(savedInstanceState: Bundle?) { | ||
super.onCreate(savedInstanceState) | ||
requestWindowFeature(Window.FEATURE_NO_TITLE) | ||
|
@@ -28,21 +52,30 @@ class GamescreenActivity: AppCompatActivity() { | |
val answerText = findViewById<TextView>(R.id.answerText) | ||
answerText.text= "The song was Demo by The Placeholders" | ||
|
||
val allPoints = nPlayers?.let { Array<Int>(it, { i -> 0 }) } | ||
val allPoints = if (nPlayers!=null) {Array<Int>(nPlayers, { i -> 0 }) } else Array<Int>(MAX_N_PLAYERS, {i ->0}) | ||
|
||
val playersFull = getIntent?.getStringArray("Player Names") | ||
val players = nPlayers?.let { playersFull?.copyOfRange(0, it) } | ||
|
||
val buzIds = players?.let { fetchBuzIdArray(it.size) } | ||
|
||
if (players != null && allPoints != null) { buildScores(players, allPoints) } | ||
if (players != null && buzIds != null) { buildBuzzers(players, buzIds, answer) } | ||
setAnswerButton(answer, findViewById(R.id.buttonCorrect)) | ||
setAnswerButton(answer, findViewById(R.id.buttonWrong)) | ||
val updater = BuzzerScoreUpdater(buzIds, allPoints) | ||
|
||
val buzToScoreMap = fetchBuzToScoreRowMap() | ||
|
||
if (players != null) { | ||
buildScores(players, allPoints) | ||
buildBuzzers(players, answer) | ||
} | ||
setAnswerButton(answer, findViewById(R.id.buttonCorrect), updater, buzToScoreMap) | ||
setAnswerButton(answer, findViewById(R.id.buttonWrong), updater, buzToScoreMap) | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Similar blocks of code found in 2 locations. Consider refactoring. |
||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Similar blocks of code found in 2 locations. Consider refactoring. |
||
/* | ||
Programmatically builds the table of scores according to the number of players | ||
|
||
|
||
/** | ||
* Programmatically builds the table of scores according to the number of players | ||
* @param players: an array of player names | ||
* @param allPoints: an array of points/scores, all set to 0 before the function is called | ||
*/ | ||
private fun buildScores(players: Array<String>, allPoints: Array<Int>) { | ||
|
||
|
@@ -67,6 +100,7 @@ class GamescreenActivity: AppCompatActivity() { | |
points.height = 75 | ||
points.width = 150 | ||
points.gravity = Gravity.RIGHT | ||
points.id=rowsIdArray[i] | ||
|
||
score.addView(nameView) | ||
score.addView(points) | ||
|
@@ -75,26 +109,23 @@ class GamescreenActivity: AppCompatActivity() { | |
|
||
i = i + 1 | ||
} | ||
|
||
} | ||
|
||
private fun fetchBuzIdArray(size: Int): Array<Int> { | ||
var array = arrayOf(R.id.buzzer_0, R.id.buzzer_1, R.id.buzzer_2, R.id.buzzer_3, R.id.buzzer_4, R.id.buzzer_5) // replace magic number here! | ||
return array.copyOfRange(0, size) // is "size" index included or not | ||
} | ||
|
||
/* | ||
Programmatically builds the buzzers according to the number and names of players. | ||
|
||
/** | ||
* Programmatically builds the buzzers according to the number and names of players. | ||
* @param players: an array of player names | ||
* @param answer: the answer layout | ||
*/ | ||
private fun buildBuzzers(players: Array<String>, buzIds: Array<Int>, answer: LinearLayout) { | ||
private fun buildBuzzers(players: Array<String>, answer: LinearLayout) { | ||
|
||
val buzzers = findViewById<LinearLayout>(R.id.buzzersLayout) | ||
val buttons = arrayOfNulls<Button>(players.size) | ||
|
||
var i = 0 | ||
|
||
for (pName in players) { | ||
|
||
val button = Button(this) | ||
button.id = buzIds[i] | ||
button.text = pName | ||
|
@@ -103,19 +134,38 @@ class GamescreenActivity: AppCompatActivity() { | |
buttons.set(i, button) | ||
button.setOnClickListener { | ||
answer.visibility = android.view.View.VISIBLE | ||
setPressed(button.id) | ||
} | ||
buzzers.addView(button) | ||
|
||
i = i + 1 | ||
} | ||
} | ||
|
||
private fun setAnswerButton(answer: LinearLayout, button: Button) { | ||
/** | ||
* Connects the answer buttons to the answer layout's visibility | ||
* @param answer: the answer layout | ||
* @param button: the answer button to be set | ||
* @param updater: the updater for the scores | ||
* @param map: a map from the buzzers' IDs to the IDs of each score's position in the score table layout | ||
*/ | ||
private fun setAnswerButton(answer: LinearLayout, button: Button, updater: BuzzerScoreUpdater, map: Map<Int, Int>) { | ||
button.setOnClickListener { | ||
answer.visibility = android.view.View.INVISIBLE | ||
if (button.id==R.id.buttonCorrect) { | ||
if (pressedBuzzer >= 0) { | ||
updater.updateScoresArray(pressedBuzzer) | ||
val view = map[pressedBuzzer]?.let { it1 -> findViewById<TextView>(it1) } | ||
if (view != null && updater.getMap().keys.contains(pressedBuzzer)) {view.text=updater.getMap()[pressedBuzzer].toString()} | ||
} | ||
} | ||
} | ||
setPressed(NO_BUZZER_PRESSED) // reset the buzzer | ||
} | ||
|
||
|
||
/** | ||
* Fires an intent from the Gamescreen to the Ending Screen | ||
*/ | ||
fun switchToEnding(view: View) { | ||
val intent = Intent(this, GameEndingActivity::class.java) | ||
//MOCK VALUES FOR INCORRECT SONGS, ADAPT FROM GAME DATA IN THE FUTURE | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it fixed? If so, maybe it's time to remove this :)