• Status: Solved
  • Priority: Medium
  • Security: Public
  • Views: 1156
  • Last Modified:

Rewrite a Java method

Here's a method that's part of an auction class that I want to rewrite so that it doesn't rely on a lot with a particular number being stored at index (number -1) in the collection.  Any suggestions?  Thanks, SP


public Lot getLot(int lotNumber)
   {
        if((lotNumber >= 1) && (lotNumber < nextLotNumber)) {
            // The number seems to be reasonable.
            Lot selectedLot = lots.get(lotNumber - 1);
            // Include a confidence check to be sure we have the
            // right lot.
            if(selectedLot.getNumber() != lotNumber) {
                System.out.println("Internal error: Lot number " +
                                   selectedLot.getNumber() +
                                   " was returned instead of " +
                                   lotNumber);
                // Don't return an invalid lot.
                selectedLot = null;
            }
            return selectedLot;
        }
        else {
            System.out.println("Lot number: " + lotNumber +
                               " does not exist.");
            return null;
        }
    }
}

Open in new window

0
spower22
Asked:
spower22
  • 6
  • 5
  • 5
  • +1
3 Solutions
 
CEHJCommented:
If it's a Collection, you need to iterate it until iterator.next().getNumber() == lotNumber
0
 
CEHJCommented:
i.e.
public Lot getLot(int lotNumber)
{
	boolean found = false;
	Lot result = null;	
	Iterator<Lot> i = lots.iterator();
	while(!found && i.hasNext())
	{
		Lot temp = i.next();
		found = (temp.getNumber() == lotNumber);
		if (found)
		{
			result = temp;
		}
	}
	return result;
}

Open in new window

0
 
spower22Author Commented:
CEJH, thanks for the input, to be honest it's a little complex for my level of understanding of java (attempting to teach myself same), is there a simpler way to achieve the desired result?
0
Industry Leaders: We Want Your Opinion!

We value your feedback.

Take our survey and automatically be enter to win anyone of the following:
Yeti Cooler, Amazon eGift Card, and Movie eGift Card!

 
lilian-arnaudCommented:
small enhancement to CEHJ code
public Lot getLot(int lotNumber)
{
        Lot result = null;      
        Iterator<Lot> i = lots.iterator();
        while(i.hasNext())
        {
                Lot temp = i.next();
                found = (temp.getNumber() == lotNumber);
                return result;
        }
        return null;
}

Open in new window

0
 
lilian-arnaudCommented:
hooops
public Lot getLot(int lotNumber)
{
        Lot result = null;      
        Iterator<Lot> i = lots.iterator();
        while(i.hasNext())
        {
                Lot temp = i.next();
                if (temp.getNumber() == lotNumber) {
                   return temp;
                }
        }
        return null;
}

Open in new window

0
 
objectsCommented:
change lots to be a Map

public Lot getLot(int lotNumber)
{
   return lots.get(lotNumber);
}

and to add a lot you would use:

lots.put(lotNumber, lot);

0
 
CEHJCommented:
>>it's a little complex for my level of understanding of java

What is it you don't understand?
0
 
spower22Author Commented:
Pretty much all of it, I'm still at the else, if, for loop statement stage!
0
 
objectsCommented:
>  is there a simpler way to achieve the desired result?

absolutely, see the code I posted above :)

0
 
objectsCommented:
If you can only change that method then the code lilian-arnaud posted is the next best thing


0
 
CEHJCommented:
Firstly what kind of collection is it? Is it a List, a Set or what?
0
 
spower22Author Commented:
OK, having looked at it, it's clearer now, maybe you could explain where the i and temp references (in CEJH and Lillian's code) come into it and what they're doing?  
0
 
CEHJCommented:
'i' iterates over the collection and 'temp' is just to temporarily hold the current Lot. Did you try the code?
0
 
spower22Author Commented:
yes I tried it and it works perfectly, I'm just trying to understand what it's doing so I can learn from it...
0
 
CEHJCommented:
btw, lilian-arnaud - that's not an 'enhancement' - it's a corruption ;-)

My code deliberately defines a single exit point, which is more readable and more maintainable IMO ;-)
0
 
objectsCommented:
if you cannot change the collection type then have a look at this:

public Lot getLot(int lotNumber)
{
        Lot result = null;
        // Iterate through the collection until you find a match    
        Iterator i = lots.iterator();
        while(result==null && i.hasNext())
        {
                // Get next Lot in collection
                Lot temp = (Lot) i.next();
                // check if lot number matches the one we are after
                if (temp.getNumber() == lotNumber) {
                   // found a match
                   result = temp;
                }
        }
        // return what we found
        return result;
}
0
 
lilian-arnaudCommented:
CEHJ : I'm so sorry !

I changed my code to follow the rule :-)
public Lot getLot(int lotNumber)
{
        Lot result = null;      
        Iterator<Lot> i = lots.iterator();
        while(i.hasNext())
        {
                Lot temp = i.next();
                if (temp.getNumber() == lotNumber) {
                   result = temp;
                   break;
                }
        }
        return result;
}

Open in new window

0
 
objectsCommented:
> CEHJ : I'm so sorry !

no need to be :)

0
 
spower22Author Commented:
Thanks for all your help, in order to be fair I'm going to split the marks as I've learned something from all three inputs and in their own way each is right.....  SP
0

Featured Post

Prep for the ITIL® Foundation Certification Exam

December’s Course of the Month is now available! Enroll to learn ITIL® Foundation best practices for delivering IT services effectively and efficiently.

  • 6
  • 5
  • 5
  • +1
Tackle projects and never again get stuck behind a technical roadblock.
Join Now