4 Replies - 240 Views - Last Post: 07 December 2012 - 05:43 AM Rate Topic: -----

#1 theNoob  Icon User is offline

  • D.I.C Head

Reputation: 3
  • View blog
  • Posts: 66
  • Joined: 14-July 12

Improving this code?

Posted 07 December 2012 - 03:20 AM

hi guys, i have wrote this code which works perfectly fine i was just wondering if i could improve it in any ways, performance wise?


	public Object deleteItem(String key)
	{
		//TODO
		Object itemDeleted = null;
		int i = 0;
		//goes through the process of checking if there are items in the array
		if(numberOfItems == 0)
		{
			//if no items found. prints error
			System.out.println("No values to delete from the array!");
			//returns value null
			return null;
		}
		
		//Iterates through number of items in the array
		for (i= 0; i < itemArray.length; i ++)
		{
			//firstly to ensure they are not null
			if(itemArray[i] != null)
			{
				//once confirmed they are not null, it checks if
				//the key matches the key for the index in the array.
				if((itemArray[i].getKey().equals(key)))
				{
					
					//returns the data that has been deleted from the array
					itemDeleted = this.itemArray[i].getData();
					//if it does, it sets the value in the array to null/
					itemArray[i] = null;
					//decrements the numberOfItems.
					numberOfItems--;				
				}
			}
		}			
		int j = 0;
		for(j = 0; j < numberOfItems; j++)
		{
			if(itemArray[j] == null)
			{
				itemArray[j] = itemArray[j+1];
				itemArray[j+1] = null;
			}
		}
		
		int itemLength = itemArray.length/2;
		if(numberOfItems < itemLength)
		{
			decreaseArray();
		}
		return itemDeleted;
	}




Is This A Good Question/Topic? 0
  • +

Replies To: Improving this code?

#2 Mylo  Icon User is offline

  • Knows all, except most.

Reputation: 265
  • View blog
  • Posts: 747
  • Joined: 11-October 11

Re: Improving this code?

Posted 07 December 2012 - 03:35 AM

I'm going to assume this is your code from a previous thread, in which case it may be beneficial to post the surrounding code too so people know what some of your variables mean.

Line 5: Move the declaration into your loop.
Line 35: As above

If you are writing this code to be used throughout your program, and even reused in other projects, I'd remove your print statement since it is an expensive operation, but more so because you probably don't want that message to appear every time. It is perfectly fine for debugging purposes however.

You may also want to move your shifting code to another method for the sake of readability. On that note, I feel like it is doing the wrong thing. Assuming numberOfItems is the count of all items in the array. If you shift that many elements, you are shift your WHOLE array. You only want to shift what is after the element you removed.
In regards to comments, a method where most lines are commented is usually compensating for a poorly written one. We know what numberOfItems-- does for example.

Lastly, your remove method returns object.getData() instead of object. This is not what I would expect looking at the method name alone, best to return object alone unless your method specifies something else. Remove() does not make sense to do that. In this case, the return type should be the type of your itemArray, not object.

This post has been edited by Mylo: 07 December 2012 - 03:37 AM

Was This Post Helpful? 0
  • +
  • -

#3 pbl  Icon User is offline

  • There is nothing you can't do with a JTable
  • member icon

Reputation: 8332
  • View blog
  • Posts: 31,857
  • Joined: 06-March 08

Re: Improving this code?

Posted 07 December 2012 - 05:02 AM

Really don\t see why you make 2 loops
If it is the case, write 2 methods: one that set to null the founded item and then one that shift the array
What is the use to set to null if you will overwrite it anyway ? Better to simply:

for(int i = 0; i < nbItem; ++i) {
    if(itemFound) {
        saved ItemFound;
        for(int j = i; i < nbItem-1; ++j)
           array[j] = array[j+1];
        return saved itemFound
    }
}


Was This Post Helpful? 0
  • +
  • -

#4 baavgai  Icon User is offline

  • Dreaming Coder
  • member icon

Reputation: 5832
  • View blog
  • Posts: 12,685
  • Joined: 16-October 07

Re: Improving this code?

Posted 07 December 2012 - 05:40 AM

I'd do something like:
// I would think you'd need this for other things
private int findItem(String key) {
	for (int i = 0; i < itemArray.length; i++) {
		if (itemArray[i] != null && itemArray[i].getKey().equals(key)) {
			return i;
		}
	}
	return -1;
}

private void decreaseArray(boolean force) {
	if (force || numberOfItems < (itemArray.length / 2)) {
		/* your code here */
	}
}

public Object deleteItem(String key) {
	Object itemDeleted = null;
	if (numberOfItems == 0) {
		System.out.println("No values to delete from the array!");
	} else {
		int found = findItem(key);
		if (found!=-1) {
			itemDeleted = this.itemArray[found].getData();
			numberOfItems--;
			for(int i=found; i<numberOfItems; i++) {
				itemArray[i] = itemArray[i + 1];
			}
			decreaseArray(false);
		}
	}
	return itemDeleted;
}



Note, more methods more better. Also, I wouldn't bother with the no values deleted. I'd just return the value deleted. If it's null, they can figure it out. Unless you allow null data, which would be odd.

Hope this helps.
Was This Post Helpful? 2
  • +
  • -

#5 theNoob  Icon User is offline

  • D.I.C Head

Reputation: 3
  • View blog
  • Posts: 66
  • Joined: 14-July 12

Re: Improving this code?

Posted 07 December 2012 - 05:43 AM

thanks everyone for the helping me through this.. ill try and implement most of it in my program
Was This Post Helpful? 0
  • +
  • -

Page 1 of 1