Bug in code, need an outside perspective - cannot figure it out.

[Jeff Yan]

hey, basically i have programmed a nextFit algorithm that is a cable ordering system. the user orders a length and type of cable and the program processes the order cutting the cable that is required at the required length. the orders are generated using a seperate class and are passed as a parameter to the algorithm to be processed.
i have this code working; however - it works only for the first run of each type of cable.

e.g.

-programme starts
-one order gets processed for 28m of aluminium,
-there is no aluminium in the store so more is ordered (a random integer between 100 and 200 used to represent the length ordered)
-the 28m gets taken of the ordered aluminium, say 190m was ordered from the store, the remaining aluminium after this has been processed is 168m

then after this one run, when another order of aluminium is processed, it does not take the value off the remaining aluminium variable.

i dont know if this is something to do with using the same object to process all the data?? but it should work. i cannot see the bug anywhere and have been looking for hours to no avail. the code for the class with the bug is below. any help much appreciated.

  

/**
 * @author Stephen John Smith a8120280
 * @version 1.0
 */
public class NextFit
{
    String OrderType;
    int OrderLength;
    String LastOrderType;
    int RemainingCopper;
    int RemainingAluminium;
    int WastedCable;
    long StartTime;
    long EndTime;
    long TotalTime;
    int Temp;
    
    public NextFit()
    {
        String OrderType = "";
        int OrderLength = 0;
        String LastOrderType = "";
        int RemainingCopper = 0;
        int RemainingAluminium = 0;
        int WastedCable = 0;
        int StartTime = 0;
        int EndTime = 0;
        int TotalTime = 0;
        int Temp = 0;
    }
    
    // This method implements the algorithm and processes the order.
    public void OrderProcess(String type ,int length)
    {
        OrderType = type;
        OrderLength = length;
        StartTime = System.currentTimeMillis();
        if (OrderType.equals( "Aluminium"))
        {            
            if (OrderLength > RemainingAluminium)
            {
                OrderAluminium OA = new OrderAluminium();
                Temp = OA.BuyAluminium();
                System.out.println("                ORDER: Remaining Aluminium is only " + RemainingAluminium + "m");
                System.out.println("                    Aluminium Length " + Temp + "m Ordered");
                RemainingAluminium = RemainingAluminium + Temp;
                
                RemainingAluminium = RemainingAluminium - OrderLength;
                
                LastOrderType = OrderType;
                
                 // This checks if the length of the cables are less than 4m, if so then they are scrapped
                 // and the length is set to 0m for more to be ordered in
                if (RemainingAluminium < 4)
                {
                    WastedCable = WastedCable + RemainingAluminium;
                    RemainingAluminium = 0;
                    System.out.println("    Aluminium Cable Discarded");
                }
            }
            EndTime = System.currentTimeMillis();
            TotalTime = TotalTime + (EndTime - StartTime);
            System.out.println("    ORDER PROCESSED SUCCESFULLY");
        }
        
        if (OrderType.equals("Copper"))
        {
            if (OrderLength > RemainingCopper)
            {
                OrderCopper OC = new OrderCopper();
                Temp = OC.BuyCopper();
                System.out.println("                ORDER: Remaining Copper is only " + RemainingCopper + "m");
                RemainingCopper = RemainingCopper + Temp;
                System.out.println("                    Copper Length " + Temp + "m Ordered");
                
                RemainingCopper = RemainingCopper - OrderLength;
                LastOrderType = OrderType;
                
                 // This checks if the length of the cables are less than 4m, if so then they are scrapped
                 // and the length is set to 0m for more to be ordered in
                if (RemainingCopper < 4)
                {
                    WastedCable = WastedCable + RemainingCopper;
                    RemainingCopper = 0;
                    System.out.println("    Copper Cable Discarded");
                }
            }
            EndTime = System.currentTimeMillis();
            TotalTime = TotalTime + (EndTime - StartTime);
            System.out.println("    ORDER PROCESSED SUCCESFULLY");
        }
        
        if (OrderType.equals("Any"))
        {
            System.out.println("    Deciding Cable for Any");
            
            if(LastOrderType == null)
            {
                OrderType = "Copper";
                System.out.println("    Your order is: " + OrderType + " " + OrderLength + "m");
                EndTime = System.currentTimeMillis();
                TotalTime = TotalTime + (EndTime - StartTime);
                OrderProcess(OrderType, OrderLength);
            }else if (LastOrderType.equals("Aluminium"))
            {
                OrderType = "Copper";
                System.out.println("    Your order is: " + OrderType + " " + OrderLength + "m");
                EndTime = System.currentTimeMillis();
                TotalTime = TotalTime + (EndTime - StartTime);
                OrderProcess(OrderType, OrderLength);
            }else if (LastOrderType.equals("Copper"))
            {
                OrderType = "Aluminium";
                System.out.println("    Your order is: " + OrderType + " " + OrderLength + "m");
                EndTime = System.currentTimeMillis();
                TotalTime = TotalTime + (EndTime - StartTime);
                OrderProcess(OrderType, OrderLength);
        }
    }
}
        
    public void PrintStats()
    {
        System.out.println("\nRemaining Copper = " + RemainingCopper + "m");
        System.out.println("Remaining Aluminium = " + RemainingAluminium + "m");
        System.out.println("Wasted Cable = " + WastedCable + "m");
        System.out.println("Total Time To Finish Algorithm and process = " + TotalTime + "ms");
    }
}

  

[Jeff Yan]

anybody?

[D. Ogranos]

You only check if the orderlength is greater than the remaining aluminium length. But what is if it is NOT greater?

[Rob Spoor]

Jeff Yan wrote:anybody?

PatienceIsAVirtue.

[David Newton]

Jeff Yan wrote:anybody?

You waited like 30 minutes--not long enough--so you get code comments.

- The use of non-standard Java naming conventions will slow down some Java developers (like me) as they read over your code: in general, member variables should be named starting with a lower-case letter.
- Access should always be specified instead of using the default.
- The initialization you *think* you doing in your constructor isn't; those are local to the constructor.
- The class is a jumble of concerns and functionality--not necessarily an issue, but even something this short becomes more difficult than necessary to understand and debug.
- Along with that, make sure that duplicated code is identified and removed as early in the process as possible.
- Notably, the block of code for ordering copper and ordering aluminium are essentially identical: in this case, that indicates something funky in the design process.
- Note that OrderProcess is recursive: I can think of no good reason to make it so.
- Be wary of making things instance variables that don't need to be: relying on side effects can make debugging... troublesome.
- You spelled aluminum wrong. (Okay, I'm kidding with that one

[Jeff Yan]

sorry for the impatience, i was getting stressed about it! and the code is just a basic outline it needs refinement, but the first answer hit the nail on the head, so thank you for that. problem resolved.