Performance of Java Object Referece

[YuenLian Wu]

Hi all,

I have an ArrayList that might contains different objects, such as Book, CD, Car, etc, they were all made by myself. I need to specified the Object when I passing this ArrayList to a function which writes the data to a file. Here is the code:

  
 
public void FileWriter(ArrayList list)
{
try 
{
FileWriter fw = new FileWriter(filepath);
BufferedWriter bw = new BufferedWriter(fw);

for ( int i = 0; i <= list.size() - 1; i++ ) {

if ( list.get(i) instanceof CD ) {
bw.write( ((CD)list.get(i)).toString() ); 
}
else
bw.write( ((Book)list.get(i)).toString() );

bw.newLine(); 
}   
 
bw.close();
fw.close();

} 
catch (IOException ioe) {
ioe.printStackTrace();
}
 
}
 

Do you think this will mess up the performance because it will check the object from the ArrayList each time, and write to the file according to which object does it belong to.

Please give me some suggestions, myriad thanks

Transistor

[Paul Clapham]

The question doesn't apply because you don't need to check what class an object belongs to just to call its toString() method. All classes inherit toString() from Object. Your if-then-else could be replaced by this:

bw.write( list.get(i).toString() );

[steve souza]

The performance of checking the Object type is trivial compared to the IO. I also agree with the previous poster that you should not cast to the underlying object type and use toString() instead.

I made a few other changes that don't effect performance, but improve the code.
1) Change FileWriter to fileWriter to follow the java naming convention
2) Change ArrayList to List to have a more flexible method
3) throw the IOException instead of gobbling/hiding it.
4) Get rid of unnecessary fw variable
5) Change 'i <= list.size() - 1' to the more standard 'i < list.size()'
6) Get rid of unnecessary fw.close() (bw.close() would have called it anyway)
7) Get rid of unnecessary casting.
8) Close the file in a finally block to ensure that it occurs.

 
 
public void fileWriter(List list) throws IOException
{
  BufferedWriter bw;
  try 
   {
    bw = new BufferedWriter(new FileWriter(filepath));

    for ( int i = 0; i < list.size(); i++ ) {
      bw.write( list.get(i).toString() ); 
      bw.newLine(); 
    }   

   } finally {
     bw.close();
   }
 
}
 

[ December 07, 2005: Message edited by: steve souza ]

[YuenLian Wu]

Hi all,

Thanks for all your replies. I have another Performance question want to ask. The original code are shown in the below:

public static String[] itemDetails =
{ "item ID", "item Title", "item Description", "item Price", "quantity in stock", "quantity in order", "release day"};

public static String[] cdDetails =
{ "CD type", "artist name", "number of Tracks", "Duration" };

public static String[] bookDetails =
{ "Book type", "ISBN", "author name", "publisher" };


public List addItem() {

boolean isCD = true;
List list = new List();
BookStoreUtility bsUtil = new BookStoreUtility();

CD cd = new CD();
Book book = new Book();

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

input = Keyboard.readStr(); // get input from users

if ( i == 0 ) {
if ( ( input.charAt(0) == 'c' || input.charAt(0) == 'C') )
isCD = true;
else
isCD = false;
}

if (isCD)
PopulateCD(i, input, cd);
else
PopulateBook(i, input, book);
}

list.add(cd);

return list;

}

I have a addItem method that get inputs from user. I have 3 classes(Object), they are Book, CD and Item. Book and CD objects are extending from the Item object. At first, the DataEntry Person would need to input the ItemID. If the ItemID is started with "C", that means he/she is inserting CD's information, and all the inputs will be populated to CD object, and it works the same way for Book Object, ...etc. All the input parameters names will be stored in a String[], such as itemDetails, cdDetails, etc.

However, the program doesn't know the User is inserting whether the Book or CD information at the beginning, so by default i will just use a forLoop to read all the parameters from String[] itemDetails. After the user entered the first parameter, the program knows the user is inserting wheter CD or Book. I would like to concat the cdDetails or bookDetails String array to the default itemDetails String array so that all the required parameters can be read from user.

I would like to get some nice suggestions on how to implement this thing. I want to learn how to design fast, dynamic and portable codes. Please help me out.

Thanks in advance

Transistor

[ December 07, 2005: Message edited by: YuenLian Wu ]
[ December 07, 2005: Message edited by: YuenLian Wu ]

[Ilja Preuss]

Two advices:

- when you have a new question, start a new topic - that way you are much more likely to get good answers, because the thread is less confusing.

- concentrate on producing nice and flexible code. Microoptimizations cause more harm than good and are a typical beginners error. Come back to performance optimization when you perceive a real problem - if your code is well designed, it will be much more easy to optimize than when you littered it with microoptimizations.

[steve souza]

Some advice. Buy the refactoring book by Martin Fowler. Your code isn't really Object oriented. You should do a better job of hiding such data structures as String[]. I will list a few general rules, and a refactoring of the code (I am doing it quickly so there may be some errors)

1)bsUtil isn't used in your code
2) Methods should reflect the concepts being executed and not implementation. Note I simplified the code by making it more conceptual and having each method do one thing
3)List list = new List(); won't compile as you can't create a List object. It is an interface. You should try to compile your code before posting.
4) Code should be made more flexible than to get input just from keyboard. I'm not sure the type of all variables that you use (for example input) as you didn't include all code. I decided to store your input as an ArrayList of String[] and send it to the populateBookStore method. You could get this data from any input source including keyboard, file, database etc.
5) In general have small methods that do one thing, and have your code read like psuedocode and you probably will win in just about every way i.e. flexible, maintainable, fast, tunable code. The catch is that it requires more up front thinking.
6) Put usage code in main method and it can double as test code. See my main method
7)PopulateCD(i, input, cd) - Any time your method has a noun and a verb you are probably missing an object. In this case cd.populate(...) would be better. In my code I opted to eliminuate this method and do the same thing with a constructor.
8) PopulateCD(i, input, cd) - Use simple objects in your method signatures. In this case you use an input stream. I opted instead to use the results of reading the inputStream (String[]) which is much more flexible. It allowed me to reuse this capability to populate the book store from a file or database easily.

Here is the code

public class BookStore {
 
   public void main(String[] args) {
     BookStore bs=new BookStore();
     bs.populate(getItemsFromKeyBoard()); // getListFromKeyBoard isn't written
 
     bs=new BookStore();
     bs.populate(getItemsFromDB()); // getItemsFromDBisn't written
 
     bs=new BookStore();
     bs.populate(getItemsFromFile()); // getItemsFromFile isn't written
 
/* note getItemsFromKeyBoard, getItemsFromDB, getItemsFromFile have a
common purpose (reading items), and note also they have a noun and a verb in
the name so a possible refactoring would be to do something like the 
following
 
keyBoard.getItems(), db.getItems(), file.getItems() i.e. inputSource.getItems()
*/
 
  }
 
    private List list = new List();
 
 
    public void addItem(Item item) {
       list.add(item);
    }
 
    
 
    public void populate(List items) {
       Iterator iter=items.iterator();
       while (iter.hasNext())
          Item item=itemFactory((String[]) iter.next());
          addItem(item);
       }
 
    }
 
   // Check the item type and build the appropriate Item
   private itemFactory(String[] itemArray) {
    String itemType=itemArray[0];
    if (isType("Book",itemType)
      return new Book(itemArray); // constructor knows how do build a book from array
    else if (isType("CD",itemType)
      return new CD(itemArray);
    else
       throw new InvalidArgumentException("Invalid Item type: "+isType);
 
   }
 
}
 
 
 
// Interfaces and supporting classes
 
public interface Item {
...
}
 
public class Book implements Item {
...
}
 
public clas CD implements Item {
...
}
  

[ December 08, 2005: Message edited by: steve souza ]
[ December 08, 2005: Message edited by: steve souza ]

[YuenLian Wu]

mmm...thanks for your nice suggestion, however, my objects are not simple as you thought. I will show you my original code as below.

  
import java.util.List;
import java.util.ArrayList;
import java.util.Date;
import java.text.SimpleDateFormat;
 
 
public class StockManagement {

public String input;
public SimpleDateFormat df;

public static String[] itemDetails = 
{ "item ID", "item Title", "item Description", "item Price", "quantity in stock", "quantity in order", "release day"};
 
public static String[] cdDetails = 
{ "CD type", "artist name", "number of Tracks", "Duration" };

public static String[] bookDetails = 
{ "Book type", "ISBN", "author name", "publisher" };


public final String[] stockmenu = 
{ "Add Item", "Update Item", "", "" };
 
 
private String artist;
private int num_of_tracks;
private float duration;
private char type;

public StockManagement() {
df = new SimpleDateFormat("DD/MM/YYYY");
}

 
public List addItem(List list) {

boolean isCD = true;
BookStoreUtility bsUtil = new BookStoreUtility();

CD cd = new CD();
Book book = new Book();

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

bsUtil.writeOutput(itemDetails[i]);
input = Keyboard.readStr();
 

if ( i == 0 ) {
if ( ( input.charAt(0) == 'c' || input.charAt(0) == 'C') )
isCD = true;
else
isCD = false; 
}
 
if (isCD)
PopulateCD(i, input, cd);
else
PopulateBook(i, input, book);
}
 
if (isCD)
list.add(cd);
else
list.add(book);

return list;

}
 
 

public void PopulateCD(int i, String paramValue, CD cd){

try {
switch (i) {
case 0:  cd.setItemID(paramValue); break;
case 1:  cd.setItemTitle(paramValue); break;
case 2:  cd.setItemDes(paramValue); break;
case 3:  cd.setItemPrice(Float.parseFloat(paramValue)); break;
case 4:  cd.setQuanInStock(Long.parseLong(paramValue)); break;
case 5:  cd.setQuanInOrder(Long.parseLong(paramValue)); break;
case 6:  cd.setRelDate(df.parse(paramValue)); break;
case 7:  cd.setType(paramValue.charAt(0)); break;
case 8:  cd.setArtist(paramValue); break;
case 9:  cd.setNum_of_tracks(Integer.parseInt(paramValue)); break;
case 10: cd.setDuration(Float.parseFloat(paramValue)); break;
}

} catch (Exception e) {};
 
}

public void PopulateBook(int i, String paramValue, Book book){

try {
switch (i) {
case 0:  book.setItemID(paramValue); break;
case 1:  book.setItemTitle(paramValue); break;
case 2:  book.setItemDes(paramValue); break;
case 3:  book.setItemPrice(Float.parseFloat(paramValue)); break;
case 4:  book.setQuanInStock(Long.parseLong(paramValue)); break;
case 5:  book.setQuanInOrder(Long.parseLong(paramValue)); break;
case 6:  book.setRelDate(df.parse(paramValue)); break;
case 7:  book.setType(paramValue.charAt(0)); break;
case 8:  book.setAuthor(paramValue); break;
case 9:  book.setISBN(paramValue); break;
case 10: book.setPublisher(paramValue); break;
}

} catch (Exception e) {};
 
}


}

and My CD class look like the following

 
 
public class CD extends Item {

private String artist;
private int num_of_tracks;
private float duration;
private char type;
public StringBuffer sb = new StringBuffer();
 
public CD() {

}
 
/**
 * @return
 */
public String getArtist() {
return artist;
}
 
/**
 * @return
 */
public float getDuration() {
return duration;
}
 
/**
 * @return
 */
public int getNum_of_tracks() {
return num_of_tracks;
}
 
/**
 * @return
 */
public char getType() {
return type;
}
 
/**
 * @param string
 */
public void setArtist(String string) {
artist = string;
}
 
/**
 * @param f
 */
public void setDuration(float f) {
duration = f;
}
 
/**
 * @param i
 */
public void setNum_of_tracks(int i) {
num_of_tracks = i;
}
 
/**
 * @param c
 */
public void setType(char c) {
type = c;
}

public String toString() {
sb.append(getItemID()); 
sb.append(",");
sb.append(getItemTitle()); 
sb.append(",");
sb.append(getItemDes()); 
sb.append(",");
sb.append(getItemPrice()); 
sb.append(",");
sb.append(getQuanInStock()); 
sb.append(",");
sb.append(getQuanInOrder()); 
sb.append(",");
sb.append(getRelDate());
sb.append(",");
sb.append(getArtist());
sb.append(",");
sb.append(getNum_of_tracks());
sb.append(",");
sb.append(getDuration());
sb.append(",");
sb.append(getType());
sb.append(",");
 
return sb.toString();
}
 
}
 

My BookStoreUtility class looks like the following

  
 
import java.util.StringTokenizer;
 
public class BookStoreUtility {


public boolean isBlank(String string) {
return ((string == null) || (string.trim().equals("")));
}

public void createMenu(String[] menuitems){
if ( (menuitems != null) || (menuitems.length > 0 ) ) {
System.out.println("Please select the command from the following Menu:");
for ( int i = 0; i <= menuitems.length - 1; i++){
System.out.println(i + 1 + ")  "  + menuitems[i]); 
}
}
}

public void writeOutput(String param){

System.out.print("Please enter the " + param + ": ");

}

public int getChoice(String[] menuItems){
 
String choice;

do {
System.out.print("Your choice is: ");
choice = Keyboard.readStr();
} while ( !validParam(menuItems, choice, -1) );

return Integer.parseInt(choice);
}
 
}
 

sorry for posting those long tedious code, but I really want to learn how to write a Real OO code.

Thanks

Transistor

[ December 08, 2005: Message edited by: YuenLian Wu ]
[ December 08, 2005: Message edited by: YuenLian Wu ]

[Ilja Preuss]

This isn't about performance anymore. Moving to our OO forum...

[Martin Mathis]

I'd be following something closer to the following

Read User Input
If user input indicates CD
  create a new CD using the constructor and add it to the list
else
  create a new Book using the constructor and add it to the list

You have Book and CD objects, they should be smart enough to populate themselves given input.

The code steve souza posted is a perfect example of what you should be doing.

[steve souza]

A couple more items.

1) Use java naming conventions - Change under_score naming such as getNum_of_tracks, and num_of_tracks to getNumOfTracks, and numOfTracks.
2) Don't gobble exceptions (i.e. catch (Exception e) { })