Code Refactoring Techniques Applied

[dquishpe]

This issue outlines several refactoring techniques applied to different methods in the code. Each technique aims to improve code readability, maintainability, and robustness.

1. Decompose Conditional
   Problem: The method isSuitableForVm has a complex conditional expression in its return statement, making the code hard to understand and maintain.

Refactoring: The conditional expression is broken down into individual methods with descriptive names, making it easier to understand and maintain.
Original:

public boolean isSuitableForVm(Vm vm) {
    return (getVmScheduler().getPeCapacity() >= vm.getCurrentRequestedMaxMips()
            && getVmScheduler().getAvailableMips() >= vm.getCurrentRequestedTotalMips()
            && getRamProvisioner().isSuitableForVm(vm, vm.getCurrentRequestedRam()) 
            && getBwProvisioner().isSuitableForVm(vm, vm.getCurrentRequestedBw()));
}

Refactored Code:

public boolean isSuitableForVm(Vm vm) {
    return hasSufficientPeCapacity(vm) 
            && hasSufficientAvailableMips(vm) 
            && isRamSuitable(vm) 
            && isBwSuitable(vm);
}

private boolean hasSufficientPeCapacity(Vm vm) {
    return getVmScheduler().getPeCapacity() >= vm.getCurrentRequestedMaxMips();
}

private boolean hasSufficientAvailableMips(Vm vm) {
    return getVmScheduler().getAvailableMips() >= vm.getCurrentRequestedTotalMips();
}

private boolean isRamSuitable(Vm vm) {
    return getRamProvisioner().isSuitableForVm(vm, vm.getCurrentRequestedRam());
}

private boolean isBwSuitable(Vm vm) {
    return getBwProvisioner().isSuitableForVm(vm, vm.getCurrentRequestedBw());
}

2. Parameterize Method
   Problem: The methods vmDeallocate and vmDeallocateAll perform similar actions with only differences in their internal operations.

Refactoring: The methods are combined using a parameter that specifies the operation to be performed.

Original Code:

protected void vmDeallocate(Vm vm) {
    getRamProvisioner().deallocateRamForVm(vm);
    getBwProvisioner().deallocateBwForVm(vm);
    getVmScheduler().deallocatePesForVm(vm);
    setStorage(getStorage() + vm.getSize());
}

protected void vmDeallocateAll() {
    getRamProvisioner().deallocateRamForAllVms();
    getBwProvisioner().deallocateBwForAllVms();
    getVmScheduler().deallocatePesForAllVms();
}

Refactored Code

protected void vmDeallocate(Vm vm) {
    getRamProvisioner().deallocateRamForVm(vm);
    getBwProvisioner().deallocateBwForVm(vm);
    getVmScheduler().deallocatePesForVm(vm);
    setStorage(getStorage() + vm.getSize());
}

protected void vmDeallocateAll() {
    getRamProvisioner().deallocateRamForAllVms();
    getBwProvisioner().deallocateBwForAllVms();
    getVmScheduler().deallocatePesForAllVms();
}

3. Replace Conditional with Polymorphism
   Problem: The method processEvent uses a switch statement with multiple cases to handle different event types.

Refactoring: Subclasses are created to represent different types of events and a general interface is used. The shared method is moved to the corresponding subclasses.

Original Code:

public void processEvent(SimEvent ev) {
    int srcId = -1;

    switch (ev.getTag()) {
        case CloudSimTags.RESOURCE_CHARACTERISTICS:
            srcId = (Integer) ev.getData();
            sendNow(srcId, ev.getTag(), getCharacteristics());
            break;

        // other cases

        default:
            processOtherEvent(ev);
            break;
    }
}

Refactored Code:

public interface EventProcessor {
    void process(SimEvent ev);
}

public class ResourceCharacteristicsProcessor implements EventProcessor {
    @Override
    public void process(SimEvent ev) {
        int srcId = (Integer) ev.getData();
        sendNow(srcId, ev.getTag(), getCharacteristics());
    }
}

// Other EventProcessor implementations

public void processEvent(SimEvent ev) {
    EventProcessor processor = EventProcessorFactory.getProcessor(ev.getTag());
    if (processor != null) {
        processor.process(ev);
    } else {
        processOtherEvent(ev);
    }
}

4. Extract Variable
   Problem: The method getNumHop contains a complex expression that may be difficult to understand.

Refactoring: Intermediate variables are introduced to make the code clearer.

Original Code:

public int getNumHop() {
    int PAIR = 2;
    return ((hopsNumber - PAIR) + 1) / PAIR;
}

Refactored Code:

public int getNumHop() {
    int pair = 2;
    int adjustedHopsNumber = hopsNumber - pair;
    return (adjustedHopsNumber + 1) / pair;
}

5. Replace Error Code with Exception
   Problem: The method isFileValid handles invalid file cases with warnings instead of throwing exceptions.

Refactoring: Exceptions are used to handle invalid file cases, making error handling more explicit.

Original Code:

private boolean isFileValid(File file, String methodName) {
    if (file == null) {
        Log.printConcatLine(name, ".", methodName, ": Warning - the given file is null.");
        return false;
    }

    String fileName = file.getName();
    if (fileName == null || fileName.length() == 0) {
        Log.printConcatLine(name, "." + methodName, ": Warning - invalid file name.");
        return false;
    }

    return true;
}

Refactored Code:

private void validateFile(File file, String methodName) {
    if (file == null) {
        throw new InvalidFileException(name + "." + methodName + ": The given file is null.");
    }

    String fileName = file.getName();
    if (fileName == null || fileName.length() == 0) {
        throw new InvalidFileException(name + "." + methodName + ": Invalid file name.");
    }
}

[deRemo]

Hi, @dquishpe, feel free to open a PR regarding your proposal so that we may discuss it (however, make sure you rebase your changes on the new cloudsim7g; You will have to update some method names, check the paper here