[RESOLVED] PMD & FindBugs Error correcting

[Backslash]

I'm just finishing up my program and I need to correct these bad practice warnings, wtc
I've done of most of them but these are the ones I cant figure out. can someone please help me.
Here are my FindBug errors:

Bad practice Warnings
Code Warning
Dm ca.bcit.cst.comp2526.assign2.a00696173.Main.copyFile(File, File) invokes System.exit(...), which shuts down the entire virtual machine


Bug type DM_EXIT (click for details)
In class ca.bcit.cst.comp2526.assign2.a00696173.Main
In method ca.bcit.cst.comp2526.assign2.a00696173.Main.copyFile(File, File)
At Main.java:[line 143]
OS ca.bcit.cst.comp2526.assign2.a00696173.Main.toSha(File) may fail to close stream


Bug type OS_OPEN_STREAM (click for details)
In class ca.bcit.cst.comp2526.assign2.a00696173.Main
In method ca.bcit.cst.comp2526.assign2.a00696173.Main.toSha(File)
Need to close java.io.InputStream
At Main.java:[line 176]
OS ca.bcit.cst.comp2526.assign2.a00696173.Main.copyFile(File, File) may fail to close stream on exception


Bug type OS_OPEN_STREAM_EXCEPTION_PATH (click for details)
In class ca.bcit.cst.comp2526.assign2.a00696173.Main
In method ca.bcit.cst.comp2526.assign2.a00696173.Main.copyFile(File, File)
Need to close java.io.InputStream
At Main.java:[line 124]
OS ca.bcit.cst.comp2526.assign2.a00696173.Main.copyFile(File, File) may fail to close stream on exception


Bug type OS_OPEN_STREAM_EXCEPTION_PATH (click for details)
In class ca.bcit.cst.comp2526.assign2.a00696173.Main
In method ca.bcit.cst.comp2526.assign2.a00696173.Main.copyFile(File, File)
Need to close java.io.OutputStream
At Main.java:[line 125]

Correctness Warnings
Code Warning
IL There is an apparent infinite loop in ca.bcit.cst.comp2526.assign2.a00696173.Main.copyFile(File, File)


Bug type IL_INFINITE_LOOP (click for details)
In class ca.bcit.cst.comp2526.assign2.a00696173.Main
In method ca.bcit.cst.comp2526.assign2.a00696173.Main.copyFile(File, File)
At Main.java:[line 136]
Loop bottom at Main.java:[line 138]
Local variable named len
Last changed at Main.java:[line 127]

Experimental Warnings
Code Warning
OBL Method ca.bcit.cst.comp2526.assign2.a00696173.Main.copyFile(File, File) may fail to clean up stream or resource of type java.io.InputStream


Bug type OBL_UNSATISFIED_OBLIGATION (click for details)
In class ca.bcit.cst.comp2526.assign2.a00696173.Main
In method ca.bcit.cst.comp2526.assign2.a00696173.Main.copyFile(File, File)
Reference type java.io.InputStream
1 instances of obligation remaining
Obligation to clean up resource created at Main.java:[line 124] is not discharged
Path continues at Main.java:[line 125]
Path continues at Main.java:[line 126]
Path continues at Main.java:[line 127]
Remaining obligations: {InputStream x 1,OutputStream x 1}
OBL Method ca.bcit.cst.comp2526.assign2.a00696173.Main.copyFile(File, File) may fail to clean up stream or resource of type java.io.OutputStream


Bug type OBL_UNSATISFIED_OBLIGATION (click for details)
In class ca.bcit.cst.comp2526.assign2.a00696173.Main
In method ca.bcit.cst.comp2526.assign2.a00696173.Main.copyFile(File, File)
Reference type java.io.OutputStream
1 instances of obligation remaining
Obligation to clean up resource created at Main.java:[line 125] is not discharged
Path continues at Main.java:[line 126]
Path continues at Main.java:[line 127]
Remaining obligations: {InputStream x 1,OutputStream x 1}
OBL Method ca.bcit.cst.comp2526.assign2.a00696173.Main.toSha(File) may fail to clean up stream or resource of type java.io.InputStream


Bug type OBL_UNSATISFIED_OBLIGATION (click for details)
In class ca.bcit.cst.comp2526.assign2.a00696173.Main
In method ca.bcit.cst.comp2526.assign2.a00696173.Main.toSha(File)
Reference type java.io.InputStream
1 instances of obligation remaining
Obligation to clean up resource created at Main.java:[line 176] is not discharged
Path continues at Main.java:[line 177]
Path continues at Main.java:[line 179]
Remaining obligations: {InputStream x 1}

Dodgy Warnings
Code Warning
DLS Dead store to hash in ca.bcit.cst.comp2526.assign2.a00696173.Main.toSha(File)


Bug type DLS_DEAD_LOCAL_STORE (click for details)
In class ca.bcit.cst.comp2526.assign2.a00696173.Main
In method ca.bcit.cst.comp2526.assign2.a00696173.Main.toSha(File)
Local variable named hash
At Main.java:[line 185]

Here are the PMD errors:
Line 96: Avoid instantiating new objects inside loops
Line 102: Avoid instantiating new objects inside loops

Here's my code :

import java.io.IOException;
import java.io.InputStream;
import java.io.OutputStream;
import java.io.PrintStream;
import java.io.File;
import java.io.FileInputStream;
import java.io.FileOutputStream;
import java.security.MessageDigest;
import java.security.NoSuchAlgorithmException;

/**
 * This program copies all files from the source directory to the destination
 * directory and renames the files to their SHA-1 hash code.
 * algorithm.
 * @version 1.0
 */

public final class Main
{
    /**
     * class constructor.
     */
    private Main()
    {
    }
    /**
     * this is main method which starts the whole program.
     * @param args the arguments passed
     * @throws java.io.IOException if an input or output exception occured
     * @throws java.security.NoSuchAlgorithmException if the algorithm is not
     * available in the default package
     */
    public static void main(final String[] args) throws IOException, NoSuchAlgorithmException
    {
        if(args.length != 2)
        {
            System.err.println("Usage: java Main <source dir> <destination dir>");
            System.exit(1);
        }
        String hash;
        File source;
        File destin;
        File manifest;
        File tempSrc;
        File tempDest;
        String[] srcList;
        final PrintStream writer;
        int i;

        source = new File(args[0]);
        destin = new File(args[1]);
        srcList = source.list();

        if(source.exists())
        {
            if(source.isFile())
            {
                System.err.println("source directory is not a directory");
                System.exit(1);
            }
        }
        else if(!source.exists())
        {
            System.err.println("source directory does not exist");
            System.exit(1);
        }

        if(destin.exists())
        {
            System.err.println("destination directory exists");
            System.exit(1);
        }
        else
        {
            if(!destin.mkdir())
            {
                System.err.println("Failed creating directory: " + args[0]);
                System.exit(1);
            }
        }

        manifest = new File(args[1], "Manifest.txt");
        if(!manifest.createNewFile())
        {
            System.err.println("Failed creating Manifest.txt");
            System.exit(1);
        }

        writer = new PrintStream(new FileOutputStream(manifest));

        for(i = 0; i < srcList.length; i++)
        {
            tempSrc = new File(source, srcList[i]);
            if(tempSrc.isDirectory())
            {
                continue;
            }
            hash = toSha(tempSrc);
            tempDest = new File(destin, hash);
            copyFile(tempSrc, tempDest);
            writer.println(srcList[i] + " = " + hash);
        }

        if(writer != null)
        {
            writer.close();
        }
    }

/**
 * The copyFile function will copy all the files.
 * @param src the source directory
 * @param dest the destination directory
 * @throws java.io.IOException if an input or output exception occured
 */
    public static void copyFile(final File src, final File dest) throws IOException
    {
        InputStream in = null;
        OutputStream out = null;
        final int a = 1024;
        in = new FileInputStream(src);
        out = new FileOutputStream(dest);
        final byte[] buff = new byte[a];
        final int len = in.read(buff);

        if(!dest.exists() && !dest.createNewFile())
        {
            System.err.println("Failed creating new file");
        }

        try
        {
            while(len > 0)
            {
                out.write(buff, 0, len);
            }
        }
        
        catch(final IOException e)
        {
            System.exit(1);
        }
        
        finally
        {
            if(out != null)
            {
                out.close();
            }

            if(in != null)
            {
                in.close();
            }
        }
    }
/**
 * The toSha function converts the filenames to their SHA-1 hash code.
 * @param f the filename
 * @return returns the sha-1 hash code
 * @throws java.io.IOException if an input or output exception occured
 * @throws java.security.NoSuchAlgorithmException if the algorithm is not
 * available in the default package
 */
    public static String toSha(final File f) throws IOException, NoSuchAlgorithmException
    {
        final int sixteen = 16;
        final int hex1 = 0xff;
        final int hex2 = 0x100;
        int bytes;
        String result = "";
        final StringBuffer buff = new StringBuffer();
        final MessageDigest sha = MessageDigest.getInstance("SHA-1");
        final InputStream in = new FileInputStream(f);
        final byte [] data = new byte [ (int)f.length()];

        bytes = in.read(data);
        if(bytes >= 0 && in != null)
        {
            in.close();
        }
        sha.update(data);
        byte[] hash = new byte[bytes];
        hash = sha.digest();

        for(byte j : hash)
        {
            buff.append(Integer.toString((j & hex1) + hex2, sixteen).substring(1));
        }
        result = buff.toString();
        return result;
    }
}

[Xeel]

What target JDK and compiller versions (exact) do you use? The bugs and warnings would depend on that.

[Xeel]

I suppose most of warnings would be killed this way:

public static void copyFile(final File src, final File dest) throws IOException{
    InputStream in = null;
    OutputStream out = null;
    final int a = 1024;

    try{
        in = new FileInputStream(src);
        out = new FileOutputStream(dest);
        final byte[] buff = new byte[a];
        final int len = in.read(buff);
        
        if(!dest.exists() && !dest.createNewFile()){
            System.err.println("Failed creating new file");
        }
        
        while(len > 0){
            out.write(buff, 0, len);
        }
    }catch(final IOException e){
        e.printStackTrace(System.out);
        System.exit(1);
    }finally{
        if(out != null){out.close();}
        if(in != null){in.close();}
    }
}
/**
 * The toSha function converts the filenames to their SHA-1 hash code.
 * @param f the filename
 * @return returns the sha-1 hash code
 * @throws java.io.IOException if an input or output exception occured
 * @throws java.security.NoSuchAlgorithmException if the algorithm is not
 * available in the default package
 */
public static String toSha(final File f) throws IOException, NoSuchAlgorithmException{
    final int sixteen = 16;
    final int hex1 = 0xff;
    final int hex2 = 0x100;
    int bytes;
    String result = "";
    final StringBuffer buff = new StringBuffer();
    final MessageDigest sha = MessageDigest.getInstance("SHA-1");
    final InputStream in = new FileInputStream(f);
    final byte [] data = new byte [ (int)f.length()];
    
    try{
        bytes = in.read(data);
        sha.update(data);
        byte[] hash = new byte[bytes];
        hash = sha.digest();

        for(byte j : hash){
            buff.append(Integer.toString((j & hex1) + hex2, sixteen).substring(1));
        }
        result = buff.toString();
    }catch(IOException e){
        e.printStackTrace(System.out);
    }finally{
        in.close();
    }
    return result;
} 

- in copyFile() you initialized in and out outside of try/catch;
- same for in inside toSha() method. Also I'd recommend closing streams inside finally just to be sure they are closed;

Run Firebug again and post what left of the warnings, we'll try to correct them as well

[dlorde]

Code Warning
Dm ca.bcit.cst.comp2526.assign2.a00696173.Main.copyFile(File, File) invokes System.exit(...), which shuts down the entire virtual machine

C'mon, guy - at least make an effort - the simplest Google search gives you this:
Use System.exit with care
"The System.exit method forces termination of all threads in the Java virtual machine. This is drastic....System.exit should be reserved for a catastrophic error exit, or for cases when a program is intended for use as a utility in a command script that may depend on the program's exit code."

Correctness Warnings
Code Warning
IL There is an apparent infinite loop in ca.bcit.cst.comp2526.assign2.a00696173.Main.copyFile(File, File)

Bug type IL_INFINITE_LOOP (click for details)
In class ca.bcit.cst.comp2526.assign2.a00696173.Main
In method ca.bcit.cst.comp2526.assign2.a00696173.Main.copyFile(File, File)
At Main.java:[line 136]
Loop bottom at Main.java:[line 138]
Local variable named len
Last changed at Main.java:[line 127]

What do you not understand about this error? There's only one loop in Main.copyFile(..), and it even gives you the line numbers. If you step through it once, you'll see...

Dodgy Warnings
Code Warning
DLS Dead store to hash in ca.bcit.cst.comp2526.assign2.a00696173.Main.toSha(File)

Bug type DLS_DEAD_LOCAL_STORE (click for details)
In class ca.bcit.cst.comp2526.assign2.a00696173.Main
In method ca.bcit.cst.comp2526.assign2.a00696173.Main.toSha(File)
Local variable named hash
At Main.java:[line 185]

Check out your local variable 'hash' at line 185 - you allocate it a new array of bytes (why?), then immediately assign the return value from sha.digest() to it, which throws away the array you just allocated. This is what it means by 'dead store' - memory you allocate but never use.

Here are the PMD errors:
Line 96: Avoid instantiating new objects inside loops
Line 102: Avoid instantiating new objects inside loops

This warning speaks for itself - don't create new objects inside loops unless you have to (e.g. you're going to keep them after the loop finishes). In this case, you don't need to create a new array at all - you never use it, so it's just a waste of time.

The other warnings are about not closing streams properly - you need to read and understand a basic tutorial on streams, e.g. this one.

The tendency to err that programmers have been noticed to share with other human beings has often been treated as though it were an awkwardness attendant upon programing's adolescence, which like acne would disappear with the craft's coming of age. It has proved otherwise...
M. Halpern

[Xeel]

"System.exit"
Shame on me, missed this one...

[Backslash]

Okay I corrected most of the errors just left a few out but thanks everyone!