CODEWITHSUNDEEP
java
user3495691
user3495691

Reputation: 481

Is trimming the method input String parameter a bad practice?

I'm debating myself whether it is a bad practice to trim the input String arguments of a method? I personally don't like modifying the input arguments but wondering if trimming is Ok?

I've code something like below

private Order retrieveOrderDetails(String productId, String cardNumber, Date purchaseDate) {
    validateInputs(trim(productId), trim(cardNumber), purchaseDate);

    List<Order> orders = pullOrdersByCardNumber(trim(cardNumber));

    return retrieveOrderDetails(orders, trim(productId), purchaseDate);     
}

Instead of using trim() in multiple places, is it acceptable to do something like below?

private Order retrieveOrderDetails(String productId, String cardNumber, Date purchaseDate) {
    productId = trim(productId);
    cardNumber = trim(cardNumber);

    validateInputs(productId, cardNumber, purchaseDate);

    List<Order> orders = pullOrdersByCardNumber(cardNumber);

    return retrieveOrderDetails(orders, productId, purchaseDate);       
}

Upvotes: 1

Views: 879

Answers (3)

Stephen C
Stephen C

Reputation: 718946

Orthogonal to John's point (which is "you shouldn't need to do this here"), you seem to be asking if it is acceptable to modify a method parameter variable.

It really comes down to readability. Which of these three versions do >>you<< think is more readable?

public void test(String arg) {
    // repeat the trim computation
    use(trim(arg));
    use2(trim(arg));
}

public void test(String arg) {
    // modify the argument
    arg = trim(arg);
    use(arg);
    use2(arg);
}

public void test(String arg) {
    // use a local variable
    String trimmedArg = trim(arg);
    use(trimmedArg);
    use2(trimmedArg);
}

Your answer will probably depend on the context.

But really, it is up to you to make your own judgement. We can't tell you which version you will find more readable. It is readability for you and your colleagues that matters. After all, you will be the ones who (may) need to read this code in N years time.

There will be small performance differences, but that should not be your primary criteria or deciding, unless you have strong evidence that this code is performance critical.

Upvotes: 2

John Kugelman
John Kugelman

Reputation: 361729

The strings should already be trimmed by the time they reach retrieveOrderDetails().

Needing to trim strings implies that they're coming from user input: form fields, a configuration file, etc. Trimming whitespace from user input is a job for user interface code or file reading code. You should not be mixing layers of abstraction, handling both UI and business logic in the same function.

Don't trim the strings here, and don't check that they're trimmed. Don't worry about trimming at all. It's the caller's problem, not this function's.

If you want to be really pure you could even replace the strings with domain-specific classes. You already have Date purchaseDate rather than String purchaseDate. Do the same thing for the other two parameters. Then where trimming belongs becomes clear: not here.

Order retrieveOrderDetails(ProductId productId, CardNumber cardNumber, Date purchaseDate) {
    ...
}

Upvotes: 4

Nghia Do
Nghia Do

Reputation: 2668

If your business function doesn't change if the input is trimmed or not so trimming is fine.

But if there is a different logic between trim and with-out trim, so you can't do that by default.

Upvotes: -3

Related Questions