Problem
I made a Java program that asks for a word, then a screen width, then whether you want it to go fast or slow. Then it outputs that word and bounces it across the screen. Is this code efficient for what it does? Is there a better way to do it (without a pre-defined method/class)? And does it look professional and easy to read?
/*
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.
str1 is the string containing the spaces.
str2 is the string containing the input word.
*/
import java.util.Scanner;
class Wave6 {
//Causes the output to lag for a set amount.
public static void xwait(long L) {
for(long c=0; c<L; c++);
}
//Asks for a string to "bounce" across the screen.
public static String input_string(char chr, String str2)
throws java.io.IOException {
do {
System.out.print("n"
+ "What word do you what to input? n"
+ "Must be less than 81 characters: ");
/*
Reads a list of characters and
inputs it into a string (to allow
for spaces and other non-letter chars).
*/
do {
chr = (char) System.in.read();
if(chr != 'n') str2 += chr;
} while(chr != 'n');
//Checks if the string is in the acceptable range.
} while((str2.length() < 1) | (str2.length() > 80));
return str2;
}
//Asks for a screen width (this is arbitrary).
public static int input_width(int width, String str2){
Scanner input = new Scanner(System.in);
do {
System.out.print("n"
+ "What is the width of the screen? n"
+ "Must be less than 81, and more than " + str2.length() + " characters: ");
width = input.nextInt();
} while((width <= str2.length() ) | (width >= 81));
return width;
}
//Asks if the user wants the program to run
//at full speed, or slower.
public static long input_speed(char ch, char ign, long L)
throws java.io.IOException {
do {
System.out.print("n
+ "Do you want it slow or fast? n"
+ "S or F: ");
ch = (char) System.in.read();
//Gets rid of unwanted chars in
//the char buffer.
do {
ign = (char) System.in.read();
} while(ign != 'n');
//checks if ch == F or S
} while( !(ch == ('F')) ^ (ch == ('S')) );
//Returns a large or small number
//to slow down the program, or not.
L = 0;
if(ch == 'S')
L = 25550000L;
else if(ch == 'F')
L = 0;
return L;
}
public static void main(String args[])
throws java.io.IOException {
String str1 = "", str2 = "";
int width = 0, i;
char ch = ' ', chr = ' ', ign = ' ';
long L = 0L;
str2 = input_string(chr, str2);
width = input_width(width, str2);
L = input_speed(ch, ign, L);
/*
Part of the program that actually makes the word
bounce around.
*/
for(;;) {
for(;;) { //Right bound
/*
If the two strings added length is equal to the
Input width +1, do not output a character return.
This is the end of the right bound, break out
of the infinite loop.
Based on the code, between the conversion from
right bound to left bound, str1 has to equal width.
Otherwise the word only goes to the width -1 space.
So str1 + str2 must equal width +1
*/
if( width + 1 == (str1.length() + str2.length()) ) {
System.out.print(str1 + str2);
xwait(L);
break;
}
else {
System.out.println(str1 + str2);
xwait(L);
}
//If the two strings are not equal to width +1
//add one space to str1
if(width + 1 != ( str1.length() + str2.length()) )
str1 += " ";
}
for(;;) { //Left bound
/*
Right bound handles the code when str1 and str2
are equal to width +1, so left bound doesn't have to.
Similarly Right bound handles the output for when
str1 equals 0, but left bound makes it equal to 0.
*/
if( width + 1 != (str1.length() + str2.length()) ) {
System.out.println(str1 + str2);
xwait(L);
}
//Take a space off of str1.
str1 = str1.substring(0, str1.lastIndexOf(" "));
if(str1.length() == 0)
break;
}
}
}
}
Also, I use n++, and I have my tab set to two ASCII character widths, so when I copied and pasted all of it was set to four spaces, is there an easy way to copy it without having to go back and delete all of the tabs and replace them with spaces? It takes forever. (this isn’t important to me, but it is a hassle when trying to ask questions about code on here)
This happened when I did this in Basic on my Ti 84, but what if I had made an output_leftBound() and output_rightBound() method, used main to call rightBound(), then had rightBound() and leftBound() call each other? Would that be efficient or would it cause the memory to build up? On the calculator, if you goto out of loops, the calculator builds up memory because it reserved it to look for an END command to signal the end of a loop, but it never finds one.
Someone mentioned that:
public static void main(String args[])
throws java.io.IOExeption {
was a code smell. How (in an inquisitive tone)? The book I am using uses it whenever it uses System.in.read(). Though I did notice that when I didn’t include it my other methods that use System.in.read(), the compiler reported an error stating:
Unreported exception IOExeption; must be caught or declared to be thrown
Is this a problem?
Solution
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 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.
private static String[] leads;
public static void bounce(String word, int width, long delay)
throws InterruptedException {
// Pre-allocate all the leading space strings, such that
// leads[i] is a string containing i spaces
if (null == leads || leads.length < width - word.length()) {
leads = new String[width - word.length() + 1];
StringBuffer sb = new StringBuffer(leads.length);
for (int i = 0; i < leads.length; i++) {
leads[i] = sb.toString();
sb.append(' ');
}
}
while (true) {
// Right-bound loop
for(int i = 0; i < width - word.length(); i++) {
System.out.print(leads[i]);
System.out.println(word);
Thread.sleep(delay);
}
// Left-bound loop
for(int i = width - word.length(); i >= 1; i--) {
System.out.print(leads[i]);
System.out.println(word);
Thread.sleep(delay);
}
}
}
Addendum: Suggestion to use printf()
Another way to print a string with leading spaces is to use System.out.printf().
public static void bounce(String word, int width, long delay)
throws InterruptedException {
while (true) {
for (int w = word.length(); w < width; w++) {
System.out.printf("%" + w + "sn", word);
Thread.sleep(delay);
}
for (int w = width; w > word.length(); w--) {
System.out.printf("%" + w + "sn", word);
Thread.sleep(delay);
}
}
}
It’s less efficient, but I think that the resulting simplicity in your own code is worth it.
One suggestion that I would make is to use better names for your variables. Typically your variable name should be long enough to describe what the variable represents (and not it’s type). Short variable names work ok for loop indexes or in a very small scope (say a lambda expression) but generally you want the reader to be able to understand what the variable is without having to read the context. For example, in your code you might want to name str2 as userInput.
Another suggestion would be, perhaps, to use a BufferedReader to read the input line.
For numeric input, where you want to handle incorrect input nicely, you might want to use Integer.parseInt and catch NumberFormatException.
For what it’s worth, I’d typically prefer a do-while(true) loop over an infinite for loop, but that’s my personal preference.