Skip to main content
Code Review

Return to Revisions

3 of 7
Corrected variable initialization advice
200_success
  • 145.5k
  • 22
  • 190
  • 479

Major issues

This feels like C code more than Java. Part of the reason is the code structure — you're using class methods everywhere instead of instance methods. The bigger problem is your parameter passing. For example, in input_string(char chr, String str2), you are using chr and str2 as nothing more than local variables — you completely disregard their input values. Why not just take zero parameters and return a String?

Your input routines are unnecessarily complicated. You shouldn't have to read one character at a time. Just read a line at a time using BufferedReader.readLine().

Your xwait() function relies on busy-waiting, which you should never ever do. The first problem with busy-waiting is that it chews up CPU cycles which could be yielded to other processes running on the machine. Laptop users will hate you for wasting battery power warming up the CPU and spinning the fan. These days, you might even be running on a virtual machine on a multi-tenant host, which will cause other customers to complain! The other problem with busy-waiting is that the delay depends on the CPU's speed, which will differ wildly on various machines. Assumptions about the CPU speed are what forced early PCs to have a "Turbo" button — to slow down the CPU so that poorly written games could run at their intended speed. If you want to introduce a delay, call Thread.sleep().

Your right-bound and left-bound loops are excessively complex. The way you use an infinite for(;;) loop and break out of them is unconventional. Why not use the for-loops the way they are intended to be used? Then every programmer can tell at a glance what the structure of your loops are — what is being tested and what changes between iterations.

Nitpicks

Comments on your classes and functions might as well be JavaDoc comments. All you have to do is follow the JavaDoc formatting conventions.

Indentation should be at least four spaces. Two spaces is too narrow to read reliably, and it also encourages excessively deep nesting, which is a symptom of poorly organized code.

Method names in Java should lookLikeThis().

Your variables str1, str2, L, ch, chr, ign are all poorly named. None of them is self-descriptive. For the first three, I suggest lead, word, and delay instead. Avoid declaring all your variables at the beginning of the function. For tightest scoping and least mental workload, you should declare variables as close as possible to where they are used. Also, don't initialize all your variables to 0 or 0円 frivolously — let the Java compiler tell you when you have failed to assign them meaningful values before usage.

You should use a boolean-or rather than a bitwise-or in while((str2.length() < 1) | (str2.length() > 80)) and similar expressions.

When shrinking str1, just call str1.substring(1). Chopping off the first character is much easier than finding the last one and removing it. Also, test .isEmpty() rather than .length() == 0.

Proposed Solution

import java.io.*;
/**
 * This program was designed for use with command prompt on an ASCII
 * character set. Output is run through CMD.
 * This was designed for CMD with only an 80 ASCII character width.
 */
class Wave6 {
 /**
 * Asks for a string to "bounce" across the screen.
 */
 public static String inputString(BufferedReader in)
 throws IOException {
 String input;
 do {
 System.out.print("\n"
 + "What word do you want to input? \n"
 + "Must be less than 81 characters: ");
 input = in.readLine();
 if (null == input) throw new EOFException();
 //Checks if the string is in the acceptable range.
 } while (input.length() < 1 || input.length() > 80);
 return input;
 }
 /**
 * Asks for a screen width (this is arbitrary).
 */
 public static int inputWidth(BufferedReader in, String word)
 throws IOException {
 int width;
 do {
 System.out.print("\n"
 + "What is the width of the screen? \n"
 + "Must be less than 81, and more than "
 + word.length() + " characters: ");
 String input = in.readLine();
 if (null == input) throw new EOFException();
 width = Integer.parseInt(input);
 } while ((width <= word.length() ) | (width >= 81));
 return width;
 }
 /**
 * Asks if the user wants the program to run at full speed, or slower.
 * @return The number of milliseconds of delay per output line
 */
 public static long inputDelay(BufferedReader in)
 throws IOException {
 while (true) {
 System.out.print("\n"
 + "Do you want it slow or fast? \n"
 + "S or F: ");
 switch (in.read()) {
 case -1: throw new EOFException();
 case 'S': return 100;
 case 'F': return 0;
 }
 // Swallow the rest of the line and the end-of-line terminator
 in.readLine();
 }
 }
 public static void bounce(String word, int width, long delay)
 throws InterruptedException {
 // lead is the string containing the leading spaces.
 String lead = "";
 while (true) {
 // Right-bound loop
 for(; lead.length() + word.length() <= width; lead += " ") {
 System.out.print(lead);
 System.out.println(word);
 Thread.sleep(delay);
 }
 // Take two spaces off of lead: one because a space was added
 // when leaving the loop above, and another because we want to
 // start moving left.
 lead = lead.substring(2);
 // Left-bound loop
 for(; !lead.isEmpty(); lead = lead.substring(1)) {
 System.out.print(lead);
 System.out.println(word);
 Thread.sleep(delay);
 }
 }
 }
 public static void main(String args[])
 throws IOException {
 BufferedReader br = new BufferedReader(new InputStreamReader(System.in));
 String word = inputString(br);
 int width = inputWidth(br, word);
 long delay = inputDelay(br);
 // Part of the program that actually makes the word bounce around.
 try {
 bounce(word, width, delay);
 } catch (InterruptedException timeToQuit) {
 // Sleep interrupted? Time to quit.
 }
 }
}

Optional Suggestion

Consider pre-allocating an array of 80 (or however many) lead strings instead of creating and tossing one for each line.

200_success
  • 145.5k
  • 22
  • 190
  • 479
default

AltStyle によって変換されたページ (->オリジナル) /