Eliminate Redundant Checks in Codebase

[EmilGeorgiev]

In several places, I see code where we call a method from a type to perform a check and then call another method of the same type. For example:

if m.mempl.Contains(tx) {
    if err := m.mempl.Remove(tx); err != nil {
	   ...
    }
}

or

if handler.mevLane.Contains(tx) {
    if err := handler.mevLane.Remove(tx); err != nil {
         ...
    }
}

Such checks (like 'Contains(tx)' here) can be removed/avoided. Let's explore three different scenarios:

1. When, the method Remove(tx) makes the same check internally. Therefore, we can remove the 'Contains(tx)' check outside the Remove method because it is redundant and unnecessary.

2. When the 'Remove' method does not perform these checks internally, and they are mandatory. In this case, it might be better to move this check inside the Remove method to avoid duplication and to adhere to principles such as co-locating data and behaviour or the Tell, don't ask principle.

3. When the 'Remove' method does not perform these checks, and they are not mandatory, their inclusion depends on the caller of the method. In this case, such checks can be removed with better error handling. For example, the method can return a custom error with fields containing information about the error. Based on this data, the caller can execute different business logic.

err := m.mempl.Remove(tx) 
if err != nil {
	ce := err.(CustomError)
	if ce.Type == NotFound {
	    // do something
	}  
}

Using a custom error type can improves maintainability and also tends to be faster because usually it require less computational logic. Also, provide additional context and flexibility for error handling and decision-making in the application.

Note: This is not only for the method Remove. These approaches can be applied to any methods similar to it.