What's the most efficient way to write this method?

Question

The method returns an "Ingredient" object that is constructed from a given line in a recipe txt file. Note: an InvalidIngredientException is like an Ingredient version of an InputMismatchException. This isn't thrown by any of the lines in the given recipe file.

public static Ingredient parseString(String line)
        throws InvalidIngredientException {        
    double quantity = 1;
    String measurement = "";
    String[] parts = line.split(";");
    if (parts.length == 1) {
        throw new InvalidIngredientException(EXP_MSG);
    }
    if (!parts[0].trim().isEmpty()
            && !(Double.parseDouble(parts[0]) == 1)) {
        quantity = Double.parseDouble(parts[0].trim());
    }
    if (!parts[1].trim().isEmpty()) {
        measurement = parts[1].trim();
    } 
    return new Ingredient(quantity, measurement, parts[2].trim());
}

A recipe file looks like this:

Cranberry Oatmeal Chews
8; tablespoon; butter
2; tablespoon; oil
1; cup; light brown sugar
1; ; zest of one orange
6; tablespoon; sour cream
2; teaspoon; vanilla
1.5; cup; flour
.5; teaspoon; baking soda
1; teaspoon; cinammon
.5; teaspoon; salt
2; cup; oats
1.5; cup; dried cranberries
.5; cup; walnuts

The method works, but I feel like it could use less code.

Does "efficient" mean "performant" or "fewer lines of code"? It's not clear to me which you're asking for. — D M, Apr 27 '22 at 20:33
Remove the `!(Double.parseDouble(parts[0]) == 1)` condition. Your performing an expensive operation, in order to save exactly the same operation in one case. So you save nothing but perform the operation twice when the condition is not fulfilled. — Holger, May 05 '22 at 09:02

score 1 · Answer 1 · answered Apr 27 '22 at 21:04

What you are trying to do is called "bind CSV row to an object". There are quite a few good libraries for parsing CSV, most mature ones offering the binding functionality as well. Also there are annotation-based code generation enabled frameworks like Lombok or Jackson which make Java one step closer to convenient languages like Scala, by saving you from writing very verbose getters/setters by hand (with a minor complication to the build process, maybe).

And once you use correct search term you will find a plenty of examples. One doing just what I described above is this one, below is a version adjusted to your naming. It is using Jackson.

Object definition with Jackson annotations:

@JsonPropertyOrder({"quantity", "measure", "ingredient"})
public class Ingredient {
    public double quantity;
    public String measure;
    public int ingredient;
}

Invocation code with the Jackson CsvMapper:

List<Ingredient> result = new CsvMapper()
    .readerWithTypedSchemaFor(Ingredient.class)
    .readValues(csvFile)
    .readAll();

D M · Answer 2 · 2022-04-27T21:05:42.610

I'm not used to Java, so there might be minor errors in this code. Feel free to edit if you see one.

Coming from a C# perspective, I'd make the following changes:

Move the .trim() calls to one place. Here I do that just after splitting the input lines into parts by creating a Stream using of() then calling map (assuming Java 8 or newer).
Remove the lines instantiating default values for quantity and measurement. Because we don't have to trim, we can use the ternary operator to declare and instantiate the variables on the same line.
Don't check if parts[1] is empty. Since "" is the fallback value, it doesn't matter if parts[1] is also "". This also means you don't need the intermediate measurement variable.

public static Ingredient parseString(String line)
        throws InvalidIngredientException {        
    String[] parts = Stream.of(line.split(";")).map(p => p.trim()).toArray();
    if (parts.length == 1) {
        throw new InvalidIngredientException(EXP_MSG);
    }
    double quantity = parts[0].isEmpty() ? 1 : Double.parseDouble(parts[0]);    
    return new Ingredient(quantity, parts[1], parts[2]);
}

You also have the potential to throw errors other than InvalidIngredientException if parts.length == 0 or if parts[0] cannot be parsed to a Double. I'm not sure how strict you're supposed to be when declaring which exceptions your method can throw, but here's a version that should catch any exceptions and only return the InvalidIngredientException you declared.

public static Ingredient parseString(String line)
        throws InvalidIngredientException {     
    try { 
        String[] parts = Stream.of(line.split(";")).map(p => p.trim()).toArray();
        double quantity = parts[0].isEmpty() ? 1 : Double.parseDouble(parts[0]); 
        return new Ingredient(quantity, parts[1], parts[2]);
    }
    catch (Exception e) {
        throw new InvalidIngredientException(EXP_MSG);
    }
}

score 0 · Answer 3 · answered Apr 27 '22 at 20:48

There are a few little things that you can do to make your code look better and a increase the performance a bit also.

Change the regex when splitting the line to "\s;\s". This way you avoid the trim() calls multiple times
Use else if...else syntax. This will make your code not only a bit shorter but also easier to read.

What's the most efficient way to write this method?

3 Answers3