code redundancy

[Ricky James]

Hello,

I have this code (pasted below) which is part of a program where a user is asked to type 'dir' or 'file' in the command prompt and if the user types 'dir' then only the directories of the folder are displayed while if the user types 'file', both the files and directories are displayed. There are 2 methods that handle the request.

My question is that although my program works, both the methods have almost same code. Can I get rid of one of the methods by passing an if statement? I am aware we can do that but my skills in Java are not that advanced and I can't figure out how.

Shall be really grateful for help.

Ricky

=========================================
public void printContentsBoth(File folder){

indentLevel++;

File[] directorys = folder.listFiles(new IsDirectory());

for(int i=0; i < directorys.length; i++){

// code for getting the number of files and size of files
String pathOfDirectory = directorys[i].getPath();
File currentFolder = new File(pathOfDirectory);
File[] folderList = currentFolder.listFiles(new IsNotDirectory());
int numOfFiles = folderList.length;
for(int j = 0; j < numOfFiles; j++) {
fileSize += folderList[j].length();
}
// indent the name of directories and sub-directories
for(int indent = 0; indent < indentLevel; indent++) {
System.out.print(" ");
}
System.out.println(directorys[i].getName() + "(" + numOfFiles + ")" + "(" + fileSize + ")");
printContentsBoth(directorys[i]);
}

File[] files = folder.listFiles(new IsNotDirectory());

for(int i=0; i < files.length; i++){
for (int indent = 0; indent < indentLevel; indent++) {
System.out.print(" ");
}
System.out.println(files[i].getName());
}
indentLevel--;

}

/*
* This method prints the folders only in a given absolute path
*/
public void printContentsDir(File folder){

indentLevel++;

File[] directorys = folder.listFiles(new IsDirectory());

for(int i=0; i < directorys.length; i++){

// code for getting the number of files and size of files
String pathOfDirectory = directorys[i].getPath();
File currentFolder = new File(pathOfDirectory);
File[] folderList = currentFolder.listFiles(new IsNotDirectory());
int numOfFiles = folderList.length;
for(int j = 0; j < numOfFiles; j++) {
fileSize += folderList[j].length();
}
// indent the name of directories and sub-directories
for(int indent = 0; indent < indentLevel; indent++) {
System.out.print(" ");
}
System.out.println(directorys[i].getName() + "(" + numOfFiles + ")" + "(" + fileSize + ")");
printContentsDir(directorys[i]);
}
indentLevel--;

}
=======================================================
[ March 28, 2007: Message edited by: Ricky James ]

[Campbell Ritchie]

You can't "pass" an "if" statement, but you probably can shorten the code. You are doing well to have noticed that sort of thing. Please use "code" tags round your methods; they make it easier to read.

Is it possible to pull out the code which is the same in each case to form a 3rd method? Is it possible to pull out the code which is different in each case to form two new methods, then put the two original methods together to form one method? It appears on a cursory examination of your code that all you have different is the printing bit. You can put that bit inside an "if-else" and pass some argument to the method to tell it whether you are printing files or directories.

BTW: Where do you declare fileSize? Is it a field? You are increasing it every time you execute those two methods. Is it supposed to increase repeatedly like that, or do you need to reset it to 0 anywhere?

[edit for minor spelling error]
[ March 28, 2007: Message edited by: Campbell Ritchie ]

[Christian Nash]

Hi,

Maybe this will help. You can pass a boolean value called isDir for denoting whether the user has entered a dir or something else.

public void printContents(File folder, boolean isDir){
indentLevel++;
File[] directorys = folder.listFiles(new IsDirectory());

for(int i=0; i < directorys.length; i++){
// code for getting the number of files and size of files
String pathOfDirectory = directorys[i].getPath();
File currentFolder = new File(pathOfDirectory);
File[] folderList = currentFolder.listFiles(new IsNotDirectory());
int numOfFiles = folderList.length;

for(int j = 0; j < numOfFiles; j++) {
fileSize += folderList[j].length();
}
 
// indent the name of directories and sub-directories
for(int indent = 0; indent < indentLevel; indent++) {
System.out.print(" ");
}
System.out.println(directorys[i].getName() + "(" + numOfFiles + ")" + "(" + fileSize + ")");
printContents(directorys[i],isDir);
}
if(!isDir){
File[] files = folder.listFiles(new IsNotDirectory());

for(int i=0; i < files.length; i++){
for (int indent = 0; indent < indentLevel; indent++) {
System.out.print(" ");
}
System.out.println(files[i].getName());
}
}
indentLevel--;
}    

[ March 28, 2007: Message edited by: Christian Nash ]

[Nitesh Kant]

mmmmm ... just one comment apart from the refactoring.
It seems that your indentLevel variable is an instance level/static variable. If its static then its surely not thread-safe, so if two users execute this concurrently, you can(will) land into problems.
If its instance level, then you must not share the object instance between threads.
So, take care

[Ricky James]

Thanks Campbell, Christian and Nitesh for the replies.

Much appreciated.

Christian, I tried what you suggested but I couldn't manage it. I am pasting the entire code, so maybe then you guys will be able to help. Also pasted are 2 additional classes, namely, IsDirectory, IsNotDirectory.

This code redundancy thing has been bugging me so shall really appreciate if you could help me.

=====================================================
import java.io.File;
import java.io.*;

public class FileIteratorJJ {

static int indentLevel = -1;
long fileSize = 0;

/*
* This method prints both the files and directories of the absolute path stated in the argument.
*/
public void printContentsBoth(File folder){

indentLevel++;

File[] directorys = folder.listFiles(new IsDirectory());

for(int i=0; i < directorys.length; i++){

// code for getting the number of files and size of files
String pathOfDirectory = directorys[i].getPath();
File currentFolder = new File(pathOfDirectory);
File[] folderList = currentFolder.listFiles(new IsNotDirectory());
int numOfFiles = folderList.length;
for(int j = 0; j < numOfFiles; j++) {
fileSize += folderList[j].length();
}
// indent the name of directories and sub-directories
for(int indent = 0; indent < indentLevel; indent++) {
System.out.print(" ");
}
System.out.println(directorys[i].getName() + "(" + numOfFiles + ")" + "(" + fileSize + ")");
printContentsBoth(directorys[i]);
}
{
File[] files = folder.listFiles(new IsNotDirectory());

for(int i=0; i < files.length; i++){
for (int indent = 0; indent < indentLevel; indent++) {
System.out.print(" ");
}
System.out.println(files[i].getName());

}
indentLevel--;
}

}

/*
* This method prints the folders only in a given absolute path
*/
public void printContentsDir(File folder){

indentLevel++;

File[] directorys = folder.listFiles(new IsDirectory());

for(int i=0; i < directorys.length; i++){

// code for getting the number of files and size of files
String pathOfDirectory = directorys[i].getPath();
File currentFolder = new File(pathOfDirectory);
File[] folderList = currentFolder.listFiles(new IsNotDirectory());
int numOfFiles = folderList.length;
for(int j = 0; j < numOfFiles; j++) {
fileSize += folderList[j].length();
}
// indent the name of directories and sub-directories
for(int indent = 0; indent < indentLevel; indent++) {
System.out.print(" ");
}
System.out.println(directorys[i].getName() + "(" + numOfFiles + ")" + "(" + fileSize + ")");
printContentsDir(directorys[i]);
}
indentLevel--;

}

public static void main(String[] args){

BufferedReader br = new BufferedReader(new InputStreamReader(System.in));

try{
System.out.println("Please enter 'dir' if you want to " +
"print directories or 'file' if you want both the files and directorys");
String answer = br.readLine();

String path = "C:/MyBasket/";
File rootFolder = new File(path);
FileIteratorJJ fi = new FileIteratorJJ();
if (answer.equals("file")){
fi.printContentsBoth(rootFolder);
} else if (answer.equals("dir")){

fi.printContentsDir(rootFolder);
}
}
catch(IOException e){
System.out.println("Error in entering input???");

}


}
}
==========================================================
import java.io.*;
import java.util.*;
import java.lang.*;

public class IsNotDirectory implements FileFilter{

//Check if objects is a file
public boolean accept(File path) {
return path.isFile();
}
}
==================================================
import java.io.*;
import java.util.*;
import java.lang.*;

public class IsDirectory implements FileFilter{

//Check if objects is a folder
public boolean accept(File path){
return path.isDirectory();
}
}

=====================================

[Christian Nash]

import java.io.File;
import java.io.*;
 
public class FileIteratorJJ {
 
static int indentLevel = -1;
long fileSize = 0;
 
/*
* This method prints both the files and directories of the absolute path stated in the argument.
*/
 

public void printContents(File folder, boolean isDir){
indentLevel++;
File[] directorys = folder.listFiles(new IsDirectory());

for(int i=0; i < directorys.length; i++){
// code for getting the number of files and size of files

String pathOfDirectory = directorys[i].getPath();
File currentFolder = new File(pathOfDirectory);
File[] folderList = currentFolder.listFiles(new IsNotDirectory());
int numOfFiles = folderList.length;
for(int j = 0; j < numOfFiles; j++) {
fileSize += folderList[j].length();
} 
// indent the name of directories and sub-directories
for(int indent = 0; indent < indentLevel; indent++) {
System.out.print(" ");
}
System.out.println(directorys[i].getName() + "(" + numOfFiles + ")" + "(" + fileSize + ")");
printContents(directorys[i],isDir);
}
if(!isDir){
File[] files = folder.listFiles(new IsNotDirectory());
for(int i=0; i < files.length; i++){
for (int indent = 0; indent < indentLevel; indent++) {
System.out.print(" ");
}
System.out.println(files[i].getName());
}
}
indentLevel--;
} 
 
public static void main(String[] args) {
 
BufferedReader br =
new BufferedReader(new InputStreamReader(System.in));
 
try {
System.out.println(
"Please enter 'dir' if you want to "
+ "print directories or 'file' if you want both the files and directorys");
String answer = br.readLine();
boolean isDir = false;
String path = "C:/Documents and Settings/AKudwa/Desktop/Latecheck/";
File rootFolder = new File(path);
FileIteratorJJ fi = new FileIteratorJJ();
if (answer.equals("file")) {
fi.printContents(rootFolder,isDir);
} else if (answer.equals("dir")) {
isDir = true;
fi.printContents(rootFolder,isDir);
}
} catch (IOException e) {
System.out.println("Error in entering input???");
 
}
 
}
}
 
==========================================================
import java.io.*;
import java.util.*;
import java.lang.*;
 
public class IsNotDirectory implements FileFilter{
 
//Check if objects is a file
public boolean accept(File path) {
return path.isFile();
} 
}
==================================================
import java.io.*;
import java.util.*;
import java.lang.*;
 
public class IsDirectory implements FileFilter{
 
//Check if objects is a folder
public boolean accept(File path){
return path.isDirectory();
}
}
 
=====================================    

Make sure that the package structure is correct

[Ricky James]

Hello Christian,

It worked! Thanks a lot. And I understand now what you did to the code.

Thanks again. Much appreciated.

Ricky