HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaMajor

Capitals and countries game

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
andcountriesgamecapitals

Problem

This is my simple game that gives you a country and has you enter the corresponding capital. What do you think? (Before running it you need to do score.txt.)

```
package com.company;

import java.io.File;
import java.io.FileNotFoundException;
import java.io.PrintWriter;
import java.util.Random;
import java.util.Scanner;

public class Main {

public static void main(String[] args) throws FileNotFoundException{

Random r = new Random();

Scanner input = new Scanner(System.in);

String[] capital = {"Lisbon", "Madrid", "Paris", "Berlin", "Warsaw", "Kiev", "Moscow", "Prague", "Rome"};
String[] country = {"Portugal", "Spain", "France", "Germany", "Poland", "Ukraine", "Russia", "Czech Republic", "Italy"};
Boolean[] was = new Boolean[9];

int score = 0;
int good = 0;
int bad = 0;

File file = new File("score.txt");
Scanner lastscoreopener = new Scanner(file);
String lastscore = lastscoreopener.nextLine();
System.out.print("Your last score was: " +lastscore +"\n");

for (int d = 1; d<4; d++) {
int randomint = r.nextInt(9);
while (was[randomint] == Boolean.FALSE){
randomint = r.nextInt(9);
}
was[randomint] = false;

while (was[randomint] == true);
for (int x = 0; x < 3; x++) {
System.out.print("What's capital of " + country[randomint] + "\n");
String answer = input.nextLine();
if (answer.toLowerCase().equals(capital[randomint].toLowerCase())) {
if (x==0){
score = score+3;
good++;
}
else if(x==1){
score = score+2;
good++;
}
else if (x==2){
score = score+1;
good++;
}

Solution

String[] capital = {"Lisbon", "Madrid", "Paris", "Berlin", "Warsaw", "Kiev", "Moscow", "Prague", "Rome"};
String[] country = {"Portugal", "Spain", "France", "Germany", "Poland", "Ukraine", "Russia", "Czech Republic", "Italy"};


Two string arrays with members that are supposed to match by index? Extremely difficult to maintain, as I learned from my own experience. Use a map instead, so you can do something like:

String capital = countryCapitalMap.get(countryName);


This will help ensure there is an entry for each country/capital and make it so you only have to update things in one place.

Don't hardcode the length of the array:

int randomint = r.nextInt(9);


Use the length of the array instead, or for a map, use the size() method.

Your naming isn't always clear. How do I know what r stands for without seeing the definition? How did you determine to use d as your loop variable?

Just use the ordinary false keyword instead of the explicit Boolean.FALSE field here:

while (was[randomint] == Boolean.FALSE)

Code Snippets

String[] capital = {"Lisbon", "Madrid", "Paris", "Berlin", "Warsaw", "Kiev", "Moscow", "Prague", "Rome"};
String[] country = {"Portugal", "Spain", "France", "Germany", "Poland", "Ukraine", "Russia", "Czech Republic", "Italy"};
String capital = countryCapitalMap.get(countryName);
int randomint = r.nextInt(9);
while (was[randomint] == Boolean.FALSE)

Context

StackExchange Code Review Q#139704, answer score: 23

Revisions (0)

No revisions yet.