Proper Java Coding

Cyan

Well-Known Member
#1
Hello. I've decided to start trying to better myself, and one of the first ways I want to do this is to practice coding properly. Comments, private variables, protected variabales, getters, setters, and all the other good stuff.

I've decided to start where any good programmer would: Hello World.

...Do we have code tags?

-------------------------------------------
package cypri.games.properhelloworld;
public class ProperHelloWorld { //variable to use for System.exit(); private static int EXIT_SUCCESS = 0; //The starting point of the program. public static void main(String[] args){ //Tell the program to output the words "Hello World" to the console. System.out.println("Hello World");t face="Arial, Verdana" size="2"> //Exit the system properly. System.exit(ProperHelloWorld.EXIT_SUCCESS); }}-------------------------------------------



Now you might be wondering what I mean between proper, and improper. Well, I'll show you a example of a improper Hello World.

------------------------------------------
public class ProperHelloWorld {
public static void main(String[] a){
System.out.println("Hello World");
System.exit(0);
}
}
------------------------------------------

Notice it still does the same things, but it's not in a package, there are no comments, the variable names are just one letter, and I used a hardcoded 0 instead of a descriptive variable.

So now you know the difference!
I hope you'll enjoy whats to come (And implement a code tag >_>)
 

nerdman

pig's gotta fly
#4
There are 10 kinds of people in the world, those who program and those who don't.
I hope you keep this up. I need to step up my programming skills.
 
#5
I never understood how one could want to code improperly. Hard to keep track of your code that way.
I usually do mine like this:
public class ProperHelloWorld{
public static void main (String[] args) {
System.out.println("Hello World");
System.exit(0);

}

}
 

nerdman

pig's gotta fly
#6
I never understood how one could want to code improperly. Hard to keep track of your code that way.
Sometimes when I am in a hurry to test something, I will forget to add comments and hardcore values. I tell myself I'll clean it up after the test, but what happens is a month later I got a fucking mess on my hands.
 

Koenig

The Architect
#7
I never understood how one could want to code improperly. Hard to keep track of your code that way.
I usually do mine like this:
public class ProperHelloWorld{
public static void main (String[] args) {
System.out.println("Hello World");
System.exit(0);

}

}
I could understand why someone might want to use "Spaghetti Code" though, as that can be potentially more efficient. However I also understand why that is frowned upon considering how quickly its complexity would make handling it nigh impossible.
 

Cyan

Well-Known Member
#8
My next Proper Project code is ready to be released into the wild! It's a calculator! ...Okay, so I know it's not that cool, but it is from a proper coding standpoint. The only thing I don't like is the while(!input.hasNext()){} lines. Can anyone think of a way to make them look less hacky? Also should I throw some Thread.sleep() methods (functions, for the C* people) in there?

Well here it is. Enjoy!

--------------------------------------------------------------------------------
package cypri.games.propercalculator;

import java.util.Scanner;

public class ProperCalculator {
//variable to use for System.exit();
private static int EXIT_SUCCESS = 0;

//Variable to keep track of what version of this software you are using.
private static String version = "1.0b";

//This allows us to get input from the console.
private static Scanner input = new Scanner(System.in);

//Checks to see if the input string is garbage. Returns true if it's a command.
public static boolean iscommand(String myInput){
//if input is for Add the input is not garbage.
if(myInput.equalsIgnoreCase("a")) return true;

//if input is for Subtract the input is not garbage.
if(myInput.equalsIgnoreCase("s")) return true;


//if input is for Multiply the input is not garbage.
if(myInput.equalsIgnoreCase("m")) return true;

//if input is for Divide the input is not garbage.
if(myInput.equalsIgnoreCase("d")) return true;

//if input is for Quit the input is not garbage.
if(myInput.equalsIgnoreCase("q")) return true;

//the input is garbage.
return false;
}

//The starting point of the program.
public static void main(String[] args){

//Create variable for console input as string.
String inputStr = null;
//While user does not want to quit (Achieved by pressing 'q').

//Ask user for first input.
System.out.println("Please input a command.");

while(!(inputStr = input.next()).equalsIgnoreCase("q")){
//If the user isn't asking for help continue with the program.
if(!inputStr.equalsIgnoreCase("help")){
//if inputStr's value isn't garbage.
if(ProperCalculator.iscommand(inputStr)){
float left = 0;
float right = 0;
float result = 0;

//Ask user for left number.
System.out.println("Please input the left number.");
//Pause until user gives input for left number.
while(!input.hasNext()){}

//Check if the input given was in fact a number. If it was add it to the 'left' variable.
if(input.hasNextFloat())
left = input.nextFloat();

/*Input given was not a number. Read next input line to get rid of it,
and inform the user that the value of the 'left' variable will remain at 0.*/
else{
input.next();
System.out.println("That was not a number. Value of the left number is now 0");
}

//Ask user for right number.
System.out.println("Please input the right number.");
//Pause until user gives input for right number.
while(!input.hasNext()){}

//Check if the input given was in fact a number. If it was add it to the 'right' variable.
if(input.hasNextFloat())
right = input.nextFloat();

/*Input given was not a number. Read next input line to get rid of it,
and inform the user that the value of the 'right' variable will remain at 0.*/
else{
input.next();
System.out.println("That was not a number. Value of the right number is now 0");
}

switch(inputStr.toLowerCase()){
case "a": //Add
result = left + right;
break;

case "s": //Subtract
result = left - right;
break;

case "m": //Multiply
result = left * right;
break;

case "d": //Divide
//Keep the user from dividing by 0.
if(left != 0 && right != 0) result = left/right;
//the user tried to divide by 0.
else System.out.println("Can't divide by 0.");
break;
}

//Print out the result of the users request.
System.out.println("Your result is: " + result);
}

//inputStr is garbage.
else{
System.out.println("Please input a proper command. If you need help type "help" wiout the quotations.");
}
}

//The user is asking for help.
else{
//Show user how to use the program.
System.out.println("[a] to Add, to Subtract, [m] to Multiply, [d] to Divide, and [q] to Quit.");
}

//Ask user for next input.
System.out.println("Please input a command.");
}



//Say goodbye to user.
System.out.println("Thank you for using Proper Calculator version - " + version + " Have a nice day.");

//Exit the system properly.
System.exit(ProperCalculator.EXIT_SUCCESS);
}
}
------------------------------------------------------------
 

DarkDepths

Your friendly neighbourhood robot overlord
#9
Hey Cyan. I don't have time to write one of my own, but I do have some things I'd like to mention about your code that I'm not particularly fond of!1) Your package name doesn't follow the established naming convention. It should be a reversed internet domain name with that package name at the end. For me, for example, it could be "com.calvinrempel.proper". I'd leave out "calculator" from the package name because then your class can simply be called "Calculator" and you would be free to have other things in the package as well, like "HelloWorld".2) Calling System.exit isn't really needed. The JVM will shutdown just fine without it when it reaches the end of main. It is sometimes useful in more complex programs where you can trigger the exiting of the program from elsewhere in the code - especially when you are running many daemons, since System.exit will shut them all down. The other time it's useful is when you want to return an error code. However, in this case, you are always returning 0 for success (correctly).3) Constants should be declared as such. In this case, you should get rid of "EXIT_SUCCESS" completely, but assuming it had a good reason to be there, it should be "private static final int EXIT_SUCCESS;" Similarly, other constants, such as the version, should be declared final as well.
4) Another matter of convention, but as a general rule in Java, method names are camel case, so "iscommand" should be "isCommand".
5) In "isCommand" you've named your parameter "myInput" which, in this case, is true. However, it isn't a very descriptive name because (a) at this point, the data passed in may
have come from user input or it may not
have, we don't know at this point, and (b) all parameters are input to a method, so taking (a) into consideration, the only thing this name tells us with certainty is that the data is intended to be input to the method. I'd recommend a more descriptive name, even "command" for example.
6) You've gone comment crazy! The amount of comments you've put in simply pads it and actually makes it more difficult to read. Keep in mind that when you are commenting, you are 99.9% of the time writing comments for other programmers or yourself. In other words, people who understand the language and how it works. What they may not understand is your particular logic. So for example, there is no real value in this comment:
53px; background-color: rgb(252, 252, 252);">//Ask user for next input.
System.out.println("Please input a command.");
It is obvious what that code does. Even if someone didn't know Java, they would recognize that this is going to print out the message asking for the next input. The comment is just getting in the way. Similarly, this is not needed either:
//Check if the input given was in fact a number. If it was add it to the 'left' variable.
if(input.hasNextFloat())
left = input.nextFloat();
7) You are declaring left, right, and result several times per execution. You only need to declare them once outside of the while loop. Some people prefer to declare all variables at the beginning of a function, some say you should declare them when you need them. But I think just about everyone would agree that you shouldn't declare variables every iteration of a loop.-height: 18.200000762939453px; background-color: rgb(252, 252, 252);">
8) Your main function is very long. Again, as a general rule, it's better to break long methods up into smaller methods because (a) they are easier to test and (b) it's easier to read.
9) Your switch statement could be made more efficient. Switches on Strings are, most times, not very efficient, for a few reasons. If a hash table is used for the switch (which it may, but not necessarily, be) then hashing the String takes some time. Depending on collisions, it may also require equality checks which, as you know, require a method call "equals". If the switch is expanded into a series of "if else" statements by the compiler, then those equal methods will definitely need to be called. In this case, you are only using single characters in your switch, so you can the String method charAt() to get the operation code as a char. Since a char is an integer type, comparisons / hashing will be much faster and your switch will be more efficient.
10) A logic error, I think. "if(left != 0 && right != 0) result = left/right;" You are doing an extra equality check that you don't need to do. It shouldn't matter whether left is 0 or not, 0 / x is valid.
11) Even though you've validated your input. It is good practice to include a "default" case at the end of a switch. Even if you think it's unreachable, put it in and throw a runtime error with some info about the value. That way, when it screws up in the future somehow, you will have somewhere to start looking.-size: 15px; line-height: 18.200000762939453px;">
12) Use Javadocs!
13) For programs like this, I like to make the class usable from other classes. In this case, the most you could do from another class is check whether or not a command is valid. However, if you pulled all the functionality out into separate methods and in main simply create a new instance of ProperCalculator and call some function "run" or whatever, then you could use all the functionality of this class from another and at the same time let this class be executable on it's own.
14) Finally, just a little trick for the "iscommand" method.
public boolean isCommand(String command) { if (command.length() != 1) { return false; }
r: rgb(252, 252, 252); color: rgb(51, 51, 51); font-family: Arial, Tahoma, Calibri, Verdana, Geneva, sans-serif; font-size: 15px; line-height: 18.200000762939453px;"> String validCommands = "asmdq"; char c = command.charAt(0); return (validCommands.indexOf(c) != -1);}



I think that's all I've got. Keep in mind that in some cases I'm being picky because you titles this thread "Proper
Java Coding". Also, no, I see no reason for Thread.sleep(). Why make the program artificially slower? And, instead of "while(!input.hasNext()) {}" perhaps you could do something like:
while (!input.hasNextFloat()) { System.out.println("Please enter a numeric value: ");}float = input.nextFloat();
And it's totally fine, it's a sentinel-control loop waiting for valid user input.
 

DarkDepths

Your friendly neighbourhood robot overlord
#10
I'm actually going to do a quick double post just to show you what I think a "proper" HelloWorld class would look like:
package com.calvinrempel.proper;
/** * HelloWorld prints the message "Hello World!" to standard output. * * @author Calvin * @version 1.0 */public class HelloWorld {
/** * The entry point for the JVM. It does nothing but prints "Hello World!" to * standard output. * * @param args command line arguments. */ public static void main(String [] args) { System.out.println("Hello World!"); }}

edit: heh, I forgot about the mentions! =P

edit2: Or, my other point about being able to reuse these classes elsewhere, you could do something like:
/** * HelloWorld prints the message "Hello World!" to standard output. * * @author Calvin * @version 1.0 */public class HelloWorld {
/**face="Arial, Verdana" size="2"> * Print "Hello World!" to standard output. */ public void printMessage() { System.out.println("Hello World!"); }
/** * The entry point for the JVM. It does nothing but prints "Hello World!" to * standard output. * * @param args command line arguments. */ public static void main(String [] args) { HelloWorld helloWorld = new HelloWorld(); helloWorld.printMessage(); }}
 
#11
I never understood how one could want to code improperly. Hard to keep track of your code that way.
Sometimes when I am in a hurry to test something, I will forget to add comments and hardcore values. I tell myself I'll clean it up after the test, but what happens is a month later I got a fucking mess on my hands.
Yeah, have to admit I'm guilty of sloppy coding myself sometimes. I have every intention of fixing it later, until I get caught up in the next job ;) But (following this post), I'm gonna try harder to get into the habit of proper coding in future.
Thanks Cyan for the inspiration :)
 

DarkDepths

Your friendly neighbourhood robot overlord
#12
Jeez... looks like some history got dropped. I had a post saying that you could put your code in "pre" html tags and it would at least come out monospaced and retain spacing, like so:


[pre]
public static void main(String [] args) {
System.out.println("Tada!");
}
[/pre]
 

Cyan

Well-Known Member
#13
@DarkDepths
1) Okay, cool. I didn't know that. Thank you for letting me know. :D

2) I am indifferant on the System.exit() call. I was taught to put that at the end of my Hello World and smaller programs, so I do it. For me to change this habit I'd need a link to a discussion on this, not that I am doubting your ability in any way, but It could all come down to preference in the end.

3) Great catch, I forgot about keyword "final." I also agree that EXIT_SUCCESS shouldn't be there. I just put it there to make System.exit() look nicer, lol.

4) Right again, I agree that "command" would have made a better variable name. I didn't think about it though.

5) I thought there were too many comments too, but I'm trying to get commenting back in my system, so I kinda overdid it on purpose.

7)Yup, another good catch. I was coding that part at like 2 AM, so I guess this one slipped by me.

8)Yeah, I'll keep that in mind for the next Proper project. :)

9) Did not know that about switch statements. I'll try to keep that in mind as well.

10) x/0 is valid? Really?

11) I was thinking about a default case, but figured it'd be bloat.

12) As in the comment generator program and comment syntax, or are you telling me to look stuff up? lol.

13) Maybe it's cause of all this typing/reading but I lost you on this one. Could you explain in better detail, and/or maybe even give an example?

14) Wow, that's great! I wouldn't have thought of that. Thank you for the idea. :D (PS: yeah the lower case C on my isCommand is a typo.)

Thank you very much for taking the time to reply to me, and adding so much to the convocation. :D
I look forward to your next post here. :)

Edit: Great idea about the <pre> tag by the way!
 

DarkDepths

Your friendly neighbourhood robot overlord
#14
10) x/0 is valid? Really?No, but 0/x is. Unless I read it wrong, your code doesn't allow this case.11) I was thinking about a default case, but figured it'd be bloat.It sort of is, but in a larger application with more complicated input, even though you think you've validated it properly, its possible that you made a mistake. So it's a good practice to put in a default case and throw a runtime exception with some information about the case that broke it.
12) As in the comment generator program and comment syntax, or are you telling me to look stuff up? lol.
As in the comment syntax! I know not everyone likes it, but it makes it so easy to produce nice documentation!
13) Maybe it's cause of all this typing/reading but I lost you on this one. Could you explain in better detail, and/or maybe even give an example?
I just meant that you could pull out functionality into separate methods with the correct visibilities, then in main(), do nothing but create a new instance of the class. It's not really a big thing, it's just something I sometimes like to do to try and make things more modular. I'll put a little example below.


[pre]public class Example {
private long myMoney = 0;

public void run() {
while (!exitCondition) {
giveMeMoney();
}
}

private void giveMeMoney() {
int choice;
// Get input from user
switch(choice) {
case ....
case 3:
increaseMoney();
break;
default:
break;
}
}

private increaseMoney() {
if (myMoney < Long.MAX_VALUE - 1000000) {
myMoney += 1000000;
}
}

public static void main(String [] args) {
Example e = new Example();
e.run();
}
}
[/pre]

Obviously that's not going to compile, but ignore that... For this example it's not so great. But if you have a bunch of useful functionality, that might be useful elsewhere in another program, this might be a decent way to go, I think.
 

Cyan

Well-Known Member
#15
@DarkDepths I think the method [pre]giveMeMoney();[/pre] should open this page on the user's default browser: https://www.youtube.com/watch?v=TC1Vfoq3PvU

Yes, I'm slightly drunk. :)
 
Top