[Dogstopper]

We always hear about the best things to do when you code in Java, but oftentimes, people learn by their mistakes. What are some of the WORST practices that you have seen, and what would you suggest as an alternative to that situation?

What are the types of things that will take you from Java Ninja to Java noob?

[macosxnerd101]

The one thing I can emphatically say is not to use parallel arrays EVER!!!. Use classes and objects instead! They will help you organize your code so much better, as well as develop modular and reusable components.

I would also encourage developers to use appropriate data structures. When choosing a data structure, using it should make your life easier, especially as the size of the problem grows. The structure should also provide an efficient and appropriate means of accessing data (ie., with Trees, it might take longer to traverse the Tree, but it shouldn't take O(n^3) time to access a bottom-level element in the Tree). Some examples of appropriate data structures are Maps for finding the frequencies of occurrences for elements, Graphs for GPS systems, etc.

Regarding GUIs, always separate your GUI from your data and program state. Regardless of the UI (console, GUI, etc.), the way the data is accessed, modified, and organized shouldn't change. Generally, one should design DataManager and/or StateManager classes to handle this.

Also with GUIs, design your Components with OOP in mind. They should be modular, easily reusable, and extensible. If you are getting to the point where you are setting up a method to initialize and/or return a single GUI Component, then it is probably time to extend that Component and make it its own class.

These are just a few of the things I came up with off the top of my head.

[n8schatten]

One thing that always scares me is bad exception handling and usage.
Three things come to my mind:

1) Do not hide exceptions
Exceptions are usefull. They are not something to be ashamed of and do not need to be hidden.
E.g.

try {
  ...
} catch (Exception e) {
  //do nothing here
}

By doing this you just swallow all Exceptions (including RuntimeExceptions). No one will ever notice that something went wrong. Catch only those Exceptions you really want to catch and you are capable (and willing) to handle at that time.

2) Do not be too generic with your exceptions
E.g.

public void someMethod() throws Exception {
  ...
}

Try to tell the users of your code what might happen in your method. Don't be afraid of doing something like

public void someMethod() throws NoSuchElementException, ConcurrentModificationException, IllegalArgumentException, InterruptedException {
  ...
}

Admitted, this looks a bit extreme (and is just a random example), but it tells what might happen and what the user of your code has to be prepared of.

3) Do not be afraid of defining your own exceptions
There is a huge amount of exceptions out there, but still, there might be none for your special case. If you need an exception that e.g. happens when your webcam isn't available then define a WebcamUnavailableException or something alike.
Sometimes it is also a good idea to not use a fitting exception but to create one that fits even more. E.g. you want to load an image taken by your webcam which isn't there, a FleNotFoundException will perfectly fit, however, a NoImageToLoadException will fit even more.

[m-e-g-a-z]

A really bad thing I don't like people doing is putting all of their code in a try block such as when they are reading a file for example. You should only have something that might generate an exception in the try block NOT inserting everything you read from the file into an array list within the try block.

This below just doesn't look nice..ITS NOT RIGHT. I would NOT recommend anyone to write code like this below.

	
			try {
                                ArrayList<String> a = new ArrayList<String>();
				Scanner s = new Scanner(new FileReader("abc.txt"));
				while(s.hasNext())
					a.add(s.next());
				 System.out.println(a);
				 
			} catch (FileNotFoundException e) {

				e.printStackTrace();
			}

All you would need in your try-catch blocks is this..

                       //Initialise Scanner to null etc.
			try {
				 s= new Scanner(new FileReader("abc.txt"));
	 
			} catch (FileNotFoundException e) {

				e.printStackTrace();
			}
                       
                        //Then add to arraylist

Another thing is when people use pointless print statements. I can understand print statements are a good way of debugging but really are going to have this?!

 public int getValue() {
  System.out.println("Calling the getValue Method.");
     return value;
 }     

This post has been edited by m-e-g-a-z: 04 February 2011 - 03:17 AM

[cfoley]

I was sure I posted here this morning.

Anyway, the gist of what I said was that that it's a bad idea to write code that makes thing easier in some way for the computer (i.e. uses less memory or runs quicker). Most of the time you can end up with bloated, buggy, unmaintainable code for negligible gain. Focus on writing clean, clear code and save optimisation for when you have a real performance problem.

[japanir]

1. Multi Threads
When writing multi threaded applications, when two or more threads access the same data concurrently, there exists the possibility that two threads will access or modify the same data at the same time. Many programmers tend to "cut corners" and leave their applications valunarable to thread conflicts. Which is really a bad practice.
usually this problem is easilly solved just by adding the synchronized keyword to the method declaration:

public class SafeClass {
    private int value;
    
    public synchronized int getValue(){
       return value;
    }
    
    public synchronized void setValue(int newValue){
       value = newValue;
    }
}

2. Declaring Data Members as Public
Honsetly, there is no reason to declare data members as public. Data members should be limited to private (or protected when you really have to).
Accessing those members should be done using set\get methods only.

[xclite]

I disagree with never declaring a public member. If you have a data item in a class, and it has a getter and a setter, you're just adding redirection. Don't declare public members when some sort of control or manipulation happens, sure. But what's the point of adding methods if all the method does is set or return the value?

[baavgai]

Say no to static! Technically, every Java program needs one static method, the entry point that is main. It requires no static methods after that nor should if have them. Any legit argument for more static methods will be outside the scope of most student level projects, so there really is no excuse. Static is not a crutch that should be given to procedural programmers. Remove it and watch them fall on their faces or actually learn to walk the OO path.

Public. You obviously need public. However, you don't always need public. Everything in a class should start out private. You peel back the encapsulation as needed. Again, this is teaching OO. Variables should never, ever, be public. Get your Java getter/setter grove on. It ain't pretty, but it works. Also, not everything needs a setter.

[cfoley]

The problem with that xclite, is what happens when you decide to extract an interface from the class or change the implementation. You can't extract an interface from a public variable, but you can extract interfaces for getters and setters.

For the change of implementation, a Date class is the one I always use. Consider an implementation with three variables: day, month and year. Getters and setters for each. Any method that adds or subtracts time has to check for overflows and carry the extra to the next field. If you change the implementation to just have a days variable the getYear method will look something like return days / 365; and the add time method can just update the one variable. If you had used public variables, you would also have to change every class that used this Date class. Getters and setters are an important part of decoupling.

[xclite]

cfoley, on 04 February 2011 - 10:43 AM, said:

For the change of implementation, a Date class is the one I always use. Consider an implementation with three variables: day, month and year. Getters and setters for each. Any method that adds or subtracts time has to check for overflows and carry the extra to the next field. If you change the implementation to just have a days variable the getYear method will look something like return days / 365; and the add time method can just update the one variable. If you had used public variables, you would also have to change every class that used this Date class. Getters and setters are an important part of decoupling.

That's not a great example - consider your date class, which is safe and doesn't require sweeping changes if you change how getYear() works (which is a correct example, of course).

Consider that I wrote one with public members. year, month, day, that classes read from. The problem is the manipulation you mentioned - since control is required on those data members, you should have setters and getters. Basically your example agrees with my post - any manipulation should be encapsulated in the class's getters and setters, and I would never have written the date class without them.

If my setter/getter are only dealing with stuff that can't possibly rely on anything else or that is simply a reference, I'm not going to waste the stack space.

The idea that you can NEVER use a public x is one of the Java community's "You want this feature? You can't handle this feature!" best practices.

Also static methods are pretty useful - I would hate to have to create a Double object to call parseDouble() on a string, or pretty much the entire Math library.

This post has been edited by xclite: 04 February 2011 - 09:12 AM

[cfoley]

The Date example was to illustrate changing an implementation.

Quote

If my setter/getter are only dealing with stuff that can't possibly rely on anything else or that is simply a reference, I'm not going to waste the stack space.

I'm having trouble thinking of a good example for this, but if there is one then you may still want to extract an interface some day. I've had to do this countless times as projects evolve and you can't extract members into an interface.

[xclite]

http://download.orac.../awt/Point.html

just for example.

[SpeedisaVirus]

I'd hate to work with xclite's code. There is a reason that you wrap data members with getters and setters. Its been already said. It pins you on implementation of your data unless you want to break all client code. You sometimes find things that work better and the client code needs to have a contract that it can abide by. Knowing getDaysSince() returns something specific whether I count days off of seconds, minutes, or days is a critical part of reusable code.

[m-e-g-a-z]

Oh and another java noob mistake, always use Generics! Just because ArrayList a = new ArrayList(); does compile, doesn't mean it's right! It's very messy and doesn't allow type safety and will produce an ugly deprecation warning on Eclipse .

[xclite]

SpeedisaVirus, on 04 February 2011 - 11:24 AM, said:

I'd hate to work with xclite's code. There is a reason that you wrap data members with getters and setters. Its been already said. It pins you on implementation of your data unless you want to break all client code. You sometimes find things that work better and the client code needs to have a contract that it can abide by. Knowing getDaysSince() returns something specific whether I count days off of seconds, minutes, or days is a critical part of reusable code.

You seem to have only read one line from that first post I made rather than anything else I said.
1) I would only not use getters and setters if there was no manipulation or control of the data. It's unacceptable for somebody accessing the Date class (using cfoley's implementation) to have to do calculations, and according to what I said my code wouldn't do that.
2) who wouldn't put getDaysSince() in a method? That doesn't argue for or against my point. In fact, the funny thing is getDaysSince() is pretty much irrelevant - whether you used a public var or had getters/setters does not change your internal implementation of getDaysSince() nor the ability to have it.
3) Attacking my code is childish - if you listened to anything I'd said, it would be clear that I'm not removing functionality or requiring other classes to rely on an implementation.
All I'm saying is that there is no reason to FORBID public variables, just that education on proper use is important (well I didn't say the last part, because I didn't think my code would be attacked by a random guy on the internet who didn't read more than a line of my post).

m-e-g-a-z, on 04 February 2011 - 11:27 AM, said:

Oh and another java noob mistake, always use Generics! Just because ArrayList a = new ArrayList(); does compile, doesn't mean it's right! It's very messy and doesn't allow type safety and will produce an ugly deprecation warning on Eclipse .

Definitely a good point - they're powerful but when they aren't properly understood they just cause headaches and casting and all sorts of nasty stuff.

This post has been edited by xclite: 04 February 2011 - 09:31 AM