Rocket装载循环逻辑问题排查：按重量限制分配Item异常分析

Hey there! Let's work through fixing your loadU1 method step by step, and also leverage the existing canCarry method to make the code cleaner and more robust.

First, let's fix the critical bugs in your original code #

Your current implementation has four major issues:

1. Missing items when creating a new rocket: When an item can't fit into the current accumulated weight, you create a new U1 but never add that item to the new rocket—so it gets completely skipped.
2. Unprocessed remaining cargo: After the loop finishes, any leftover accumulated weight (testWeight) isn't wrapped into a final U1 rocket, leaving that cargo unassigned.
3. Global variable state pollution: Using a class-level testWeight means if you call loadU1 multiple times, the leftover value from the first call will corrupt the next calculation.
4. Incorrect constructor parameter order: Your U1 instantiation new U1(120, 10000, 18000, testWeight) is misaligned with the Rocket constructor's signature (price, weight, weightOfCrago, maxWeight). You're passing 18000 as the initial cargo weight and testWeight as the max weight—this completely breaks the canCarry logic!

Also, there's a bug in your Rocket class's carry method: you're updating maxWeight instead of weightOfCrago, which will make canCarry return incorrect results. Fix that first:

@Override
public void carry(Item item) {
    this.weightOfCrago += item.getWeight(); // Correct logic: add item weight to cargo, not max capacity
}

Refactored loadU1 using canCarry #

Let's rewrite the method to use the Spaceship interface methods properly, eliminate global state, and fix all the bugs:

public List<Rocket> loadU1(List<Item> items) {
    List<Rocket> u1Rockets = new ArrayList<>();
    if (items.isEmpty()) {
        return u1Rockets;
    }

    // Initialize first U1 with correct parameter order: price, rocket weight, initial cargo weight, max total weight
    Rocket currentU1 = new U1(120, 10000, 0, 18000);

    for (Item item : items) {
        if (currentU1.canCarry(item)) {
            // Item fits, add it to current rocket
            currentU1.carry(item);
        } else {
            // Current rocket is full—add it to the list first
            u1Rockets.add(currentU1);
            
            // Create a new U1 and load the current item into it
            currentU1 = new U1(120, 10000, 0, 18000);
            
            // Guard clause: ensure the item isn't too heavy for a single U1
            if (!currentU1.canCarry(item)) {
                throw new IllegalArgumentException(
                    String.format("Item '%s' (weight: %d) exceeds U1's maximum cargo capacity!", 
                    item.getName(), item.getWeight())
                );
            }
            currentU1.carry(item);
        }
    }

    // Add the last rocket (if it has any cargo) to the list
    if (currentU1.getWeightOfCrago() > 0) {
        u1Rockets.add(currentU1);
    }

    return u1Rockets;
}

Key improvements explained: #

• No global state: We use a currentU1 instance to track the active rocket's cargo, so multiple calls to loadU1 won't interfere with each other.
• Proper item handling: When an item can't fit, we finalize the current rocket, create a new one, and immediately load the item into the new rocket—no more skipped items.
• Leverage canCarry: Instead of re-implementing weight calculation logic, we use the existing interface method, keeping code DRY (Don't Repeat Yourself) and consistent.
• Final rocket cleanup: After the loop, we add the last active rocket to the list if it has cargo, so no leftover items are unassigned.
• Error handling: Added a guard clause to catch items that are too heavy for a single U1 (since your max cargo capacity is 8000—18000 - 10000 = 8000), preventing silent failures.

Note on U1 subclass #

Make sure your U1 subclass correctly extends Rocket and uses the constructor parameters properly. For example:

public class U1 extends Rocket {
    // Optional: create a convenience constructor for U1's fixed values
    public U1(int cargoWeight) {
        super(120, 10000, cargoWeight, 18000);
    }

    // Or match the parent constructor signature if you prefer:
    public U1(int price, int weight, int weightOfCrago, int maxWeight) {
        super(price, weight, weightOfCrago, maxWeight);
    }
}

内容的提问来源于stack exchange，提问作者Ver