Win a copy of The Journey To Enterprise Agility this week in the Agile and Other Processes forum! And see the welcome thread for 20% off.
  • Post Reply Bookmark Topic Watch Topic
  • New Topic
programming forums Java Mobile Certification Databases Caching Books Engineering Micro Controllers OS Languages Paradigms IDEs Build Tools Frameworks Application Servers Open Source This Site Careers Other all forums
this forum made possible by our volunteer staff, including ...
Marshals:
  • Jeanne Boyarsky
  • Liutauras Vilda
  • Campbell Ritchie
  • Tim Cooke
  • Bear Bibeault
Sheriffs:
  • Paul Clapham
  • Junilu Lacar
  • Knute Snortum
Saloon Keepers:
  • Ron McLeod
  • Ganesh Patekar
  • Tim Moores
  • Pete Letkeman
  • Stephan van Hulst
Bartenders:
  • Carey Brown
  • Tim Holloway
  • Joe Ess

code redundancy  RSS feed

 
Ranch Hand
Posts: 83
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
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 ]
 
Marshal
Posts: 59725
187
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
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 ]
 
Ranch Hand
Posts: 107
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
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.


[ March 28, 2007: Message edited by: Christian Nash ]
 
Bartender
Posts: 1638
IntelliJ IDE Java MySQL Database
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
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
Ranch Hand
Posts: 83
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
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
Ranch Hand
Posts: 107
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator


Make sure that the package structure is correct
 
Ricky James
Ranch Hand
Posts: 83
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Hello Christian,

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

Thanks again. Much appreciated.

Ricky
 
  • Post Reply Bookmark Topic Watch Topic
  • New Topic
Boost this thread!