patternjavaMajor
Countdown Code: 'League of Legends'
Viewed 0 times
countdowncodelegendsleague
Problem
I have written some code to track buffs as a side addition to the popular game League of Legends.
My code is incredibly repetitive and I also have the issue of not being able to track multiple buffs, although that might not be on topic for the question so feel free to not consider this when answering.
```
import java.util.Scanner;
public class MainProgram {
public static void main(String[] args) {
Scanner keyboard = new Scanner(System.in);
System.out.println("Jungle Timers v1.0");
System.out.println("\nSelect a buff to time:");
System.out.println("\n1. Blue");
System.out.println("2. Enemy Blue");
System.out.println("3. Red");
System.out.println("4. Enemy Red");
System.out.println("5. Dragon");
System.out.println("6. Baron");
System.out.print("\n> ");
int timerChoice = keyboard.nextInt();
keyboard.close();
switch (timerChoice) {
case 1:
friendlyBlue();
case 2:
enemyBlue();
case 3:
friendlyRed();
case 4:
enemyRed();
case 5:
Dragon();
case 6:
Baron();
}
}
public static void friendlyBlue() {
System.out.println("\nTracking your blue...");
System.out.println("5 Minutes left");
int friendlyBlue = 300;
long startTime = System.currentTimeMillis() / 1000;
long endTime = startTime + friendlyBlue;
while (System.currentTimeMillis() / 1000 1)
if (endTime - startTime == 240)
System.out.println("4 Minutes left");
if (endTime - startTime == 180)
System.out.println("3 Minutes left");
if (endTime - startTime == 120)
System.out.println("2 Minutes left");
if (endTime - startTime == 60)
System.out.println(
My code is incredibly repetitive and I also have the issue of not being able to track multiple buffs, although that might not be on topic for the question so feel free to not consider this when answering.
```
import java.util.Scanner;
public class MainProgram {
public static void main(String[] args) {
Scanner keyboard = new Scanner(System.in);
System.out.println("Jungle Timers v1.0");
System.out.println("\nSelect a buff to time:");
System.out.println("\n1. Blue");
System.out.println("2. Enemy Blue");
System.out.println("3. Red");
System.out.println("4. Enemy Red");
System.out.println("5. Dragon");
System.out.println("6. Baron");
System.out.print("\n> ");
int timerChoice = keyboard.nextInt();
keyboard.close();
switch (timerChoice) {
case 1:
friendlyBlue();
case 2:
enemyBlue();
case 3:
friendlyRed();
case 4:
enemyRed();
case 5:
Dragon();
case 6:
Baron();
}
}
public static void friendlyBlue() {
System.out.println("\nTracking your blue...");
System.out.println("5 Minutes left");
int friendlyBlue = 300;
long startTime = System.currentTimeMillis() / 1000;
long endTime = startTime + friendlyBlue;
while (System.currentTimeMillis() / 1000 1)
if (endTime - startTime == 240)
System.out.println("4 Minutes left");
if (endTime - startTime == 180)
System.out.println("3 Minutes left");
if (endTime - startTime == 120)
System.out.println("2 Minutes left");
if (endTime - startTime == 60)
System.out.println(
Solution
(relatively critical review ... apologies in advance)
The two issues I feel are most incorrect about your code is the buggy switch statement, and the poor choice of timing mechanism. The decision to use these mechanisms has lead to a poor OOP design.
Switch
First, though, the switch bug:
If the user chooses 1, they will do everything!!! Your switch cases should 'break':
Other problems related to code-conventions (like the upper-case B in
Case statements in Java are 'fall-through', and you need to break unless you want the following statements to execute as well.
Timer
In Java, the right solution for any timing problem, is to use the library timer functions. In recent Java versions, the right tool is the
I would build it something like:
Then, with the TimedEvents set up, you can schedule them in a loop, counting down to the target time.....
The two issues I feel are most incorrect about your code is the buggy switch statement, and the poor choice of timing mechanism. The decision to use these mechanisms has lead to a poor OOP design.
Switch
First, though, the switch bug:
switch (timerChoice) {
case 1:
friendlyBlue();
case 2:
enemyBlue();
case 3:
friendlyRed();
case 4:
enemyRed();
case 5:
Dragon();
case 6:
Baron();
}If the user chooses 1, they will do everything!!! Your switch cases should 'break':
switch (timerChoice) {
case 1:
friendlyBlue();
break;
case 2:
enemyBlue();
break;
case 3:
friendlyRed();
break;
case 4:
enemyRed();
break;
case 5:
Dragon();
break;
case 6:
Baron();
break;
}Other problems related to code-conventions (like the upper-case B in
Baron, and D in Dragon are not as significant).Case statements in Java are 'fall-through', and you need to break unless you want the following statements to execute as well.
Timer
In Java, the right solution for any timing problem, is to use the library timer functions. In recent Java versions, the right tool is the
ScheduledExecutorService. This service allows you to schedule tasks in the future.I would build it something like:
private final class TimedEvent {
private final long preTarget;
private final String message;
TimedEvent(long millis, String message) {
this.preTarget = millis;
this.message = message;
}
private long delayTo(final long target) {
return target - preTarget;
}
}
.....
public static final TimedEvent[] EVENTS = {
new TimedEvent(240000, "4 Minutes left"),
new TimedEvent(180000, "3 Minutes left"),
new TimedEvent(120000, "2 Minutes left"),
new TimedEvent(60000, "1 Minute left"),
new TimedEvent(30000, "30 Seconds left"),
new TimedEvent(15000, "15 Seconds left"),
new TimedEvent(5000, "5 Seconds left"),
new TimedEvent(1000, "1 Second left")
};
....Then, with the TimedEvents set up, you can schedule them in a loop, counting down to the target time.....
final long now = System.currentTimeMillis();
final long target = now + sometime; /// whatever you are counting down to....
for (final TimedEvent te : EVENTS) {
long delay = te.delayTo(target);
if (delay >= 0) {
Callable torun = new Callable() {
public void call() {
System.out.println(te.getMessage());
return null;
}
}
scheduler.schedule(torun, delay, TimeUnit.MILLISECONDS);
}
}Code Snippets
switch (timerChoice) {
case 1:
friendlyBlue();
case 2:
enemyBlue();
case 3:
friendlyRed();
case 4:
enemyRed();
case 5:
Dragon();
case 6:
Baron();
}switch (timerChoice) {
case 1:
friendlyBlue();
break;
case 2:
enemyBlue();
break;
case 3:
friendlyRed();
break;
case 4:
enemyRed();
break;
case 5:
Dragon();
break;
case 6:
Baron();
break;
}private final class TimedEvent {
private final long preTarget;
private final String message;
TimedEvent(long millis, String message) {
this.preTarget = millis;
this.message = message;
}
private long delayTo(final long target) {
return target - preTarget;
}
}
.....
public static final TimedEvent[] EVENTS = {
new TimedEvent(240000, "4 Minutes left"),
new TimedEvent(180000, "3 Minutes left"),
new TimedEvent(120000, "2 Minutes left"),
new TimedEvent(60000, "1 Minute left"),
new TimedEvent(30000, "30 Seconds left"),
new TimedEvent(15000, "15 Seconds left"),
new TimedEvent(5000, "5 Seconds left"),
new TimedEvent(1000, "1 Second left")
};
....final long now = System.currentTimeMillis();
final long target = now + sometime; /// whatever you are counting down to....
for (final TimedEvent te : EVENTS) {
long delay = te.delayTo(target);
if (delay >= 0) {
Callable<Object> torun = new Callable<Void>() {
public void call() {
System.out.println(te.getMessage());
return null;
}
}
scheduler.schedule(torun, delay, TimeUnit.MILLISECONDS);
}
}Context
StackExchange Code Review Q#48051, answer score: 21
Revisions (0)
No revisions yet.