2
\$\begingroup\$

Find the limit to the number of items that can be prepared with given amount of ingredients -

Let's try to understand with input/output pattern:
Input

4
2 5 6 3
20 40 90 50

Output

8

1st Line: Number Of Ingredients
2nd Line: Each unit required from total ingredients given
3rd Line: Each value represent total number of ingredients in our hand.

Explanation:
for each item:

2 units will be used from 20
5 units will be used from 40
6 units will be used from 90
3 units will be used from 50

until we run out of any ingredient.
After 8th attempt we are running out with ingredient 40, and hence maximum items can be prepared will be 8.

I have proposed the below solution, written in Java and it is working fine for all the test cases, but for some test cases it is taking 1 sec of time to execute, I want to improve the code, Please suggest me some best practices applicable which is helpful in order to optimize the code.

package com.mzk.poi;

import java.util.Collections;
import java.util.HashSet;
import java.util.Scanner;
import java.util.Set;

public class PowerPuffGirls {

    private static final String SPACE = " ";
    private static final Integer INITAL_IDX = 0;
    private static final Integer LOWER_IDX = 1;
    private static final Integer SECOND_IDX = 2;
    private static final Integer MAX_LINES = 3;
    private static final Integer UPPER_IDX = 10000000;
    private static long[] quantityOfIngredients;
    private static long[] quantityOfLabIngredients;
    private static int size = 0;
    private static long numberOfIngredients = 0;

    /**
     * This method will terminate the execution
     */
    private static void terminate() {
        System.exit(INITAL_IDX);
    }

    /**
     * This method validated the input as per the specified range
     * 
     * @param eachQunatity
     * @return boolean
     */
    private static boolean validateQuantityOfEachIngredients(long eachQunatity) {
        return eachQunatity >= INITAL_IDX && eachQunatity > Long.MAX_VALUE ? true : false;
    }

    /**
     * This helper method will parse the string and return long value
     * 
     * @param input
     * @return long
     */
    private static long getNumberOfIngredients(String input) {
        return Long.parseLong(input);
    }

    /**
     * This method validates the first input
     * 
     * @param noOfIngredients
     * @return boolean
     */
    private static boolean validateNumberOfIngredients(String noOfIngredients) {
        numberOfIngredients = getNumberOfIngredients(noOfIngredients);
        return numberOfIngredients >= LOWER_IDX && numberOfIngredients <= UPPER_IDX ? true : false;
    }

    /**
     * This utility method convert the String array to Integer array
     * 
     * @param size
     * @param specifiedArrayOfUnits
     * @return long[]
     */
    private static long[] convertToLongArray(String[] arrayToBeParsed) throws NumberFormatException {
        long array[] = new long[size];
        for (int i = INITAL_IDX; i < size; i++) {
            array[i] = Long.parseLong(arrayToBeParsed[i]);
        }
        return array;
    }

    public static void main(String[] args) {
        String[] arrOfQuantityOfIngredients = null;
        String[] arrOfQuantityOfLabIngredients = null;
        Set<Long> maxPowerPuffGirlsCreationList = new HashSet<Long>();
        Scanner stdin = new Scanner(System.in);
        String[] input = new String[MAX_LINES];
        try {
            for (int i = 0; i < input.length; i++) {
                input[i] = stdin.nextLine();
            }
        } finally {
            stdin.close();
        }
        if (!validateNumberOfIngredients(input[INITAL_IDX])) {
            terminate();
        }
        String quantityOfEachIngredients = input[LOWER_IDX];
        String quantityOfEachLabIngredients = input[SECOND_IDX];

        arrOfQuantityOfIngredients = quantityOfEachIngredients.split(SPACE);
        arrOfQuantityOfLabIngredients = quantityOfEachLabIngredients.split(SPACE);

        size = arrOfQuantityOfIngredients.length;

        try {
            quantityOfIngredients = convertToLongArray(arrOfQuantityOfIngredients);
            for (int i = 0; i <= quantityOfIngredients.length - 1; i++) {
                if (validateQuantityOfEachIngredients(quantityOfIngredients[i])) {
                    terminate();
                }
            }

            quantityOfLabIngredients = convertToLongArray(arrOfQuantityOfLabIngredients);
            for (int i = 0; i <= quantityOfLabIngredients.length - 1; i++) {
                if (validateQuantityOfEachIngredients(quantityOfLabIngredients[i])) {
                    terminate();
                }
            }

            for (int i = 0; i < numberOfIngredients; i++) {
                long min = quantityOfLabIngredients[i] / quantityOfIngredients[i];
                maxPowerPuffGirlsCreationList.add(quantityOfLabIngredients[i] / quantityOfIngredients[i]);
            }
            System.out.println(Collections.min(maxPowerPuffGirlsCreationList));
        } catch (Exception ex) {
            System.out.println(ex.getMessage());
        }

    }

}
\$\endgroup\$
  • \$\begingroup\$ Thanks @greybeard, any suggestion or best practice from your side \$\endgroup\$ – Zia May 30 at 11:58
  • \$\begingroup\$ With mathematics, best practice is looking for generalisations, simplifications and abstractions. There is one applying to subtract until what is left is too small. "Improving the code" would mean a code review - probably not even when the sun is down. \$\endgroup\$ – greybeard May 30 at 15:14
  • 1
    \$\begingroup\$ Please come again, I am not able to understand what exactly you meant for? \$\endgroup\$ – Zia May 30 at 15:45
  • \$\begingroup\$ (Now you've got Gilbert Le Blanc's answer for the math angle and Bobby's with coding advice: The grey is natural, my energy limited, and I've got to judiciously spend it.) \$\endgroup\$ – greybeard May 31 at 3:48
3
\$\begingroup\$

Everything in your class is static, that's most likely not what you wanted.


public class PowerPuffGirls {

Funny...or maybe not, stop that. In my experience, trying to be funny when writing code only results in somebody having to undo the funny. So don't do it in the first place.


private static final String SPACE = " ";

There's a fine line between obvious and useful variable names. This should rather be called SEPARATOR or VALUE_SEPARATOR.


private static final Integer INITAL_IDX = 0;

That sounds like something that would never change and does not need to be a static variable, because you can always deduce the meaning from the context.


private static final Integer INITAL_IDX = 0;

You might as well write INTIAL_INDEX here, you're not going to gain anything from shortening it.


private static void terminate() {
    System.exit(INITAL_IDX);
}

Now that sounds wrong. First, System.exit does not terminate the JVM, it exits. The difference being that terminating the JVM would give it time to shutdown, execute finalizers and so on, System.exit will simply stop and kill the process. This might not be what you wanted.

Also...why are you calling it with that constant?


return eachQunatity >= INITAL_IDX && eachQunatity > Long.MAX_VALUE ? true : false;

You can directly return the boolean expression:

return eachQunatity >= INITAL_IDX && eachQunatity > Long.MAX_VALUE;

String[] arrOfQuantityOfIngredients = null;

Again, there is a fine line between obvious and useful names, ingredientsQuantities would be a better name.


Scanner stdin = new Scanner(System.in);

That is a badly chosen variable name, inputScanner or scanner or even input would be a better one. stdin refers to the standard input stream, which is not the case here.

Also you could use try-with-resources here:

try (Scanner scanner = new Scanner(System.in)) {
    // TODO
}

The resource will be automatically closed when leaving the block.


if (!validateNumberOfIngredients(input[INITAL_IDX])) {
    terminate();
}

Error reporting would be a nice addition.


for (int i = 0; i <= quantityOfIngredients.length - 1; i++) {

I'm still a fan of using descriptive names, like index or counter, but that just might be me.


maxPowerPuffGirlsCreationList.add(quantityOfLabIngredients[i] / quantityOfIngredients[i]);

You want to read up in autoboxing, it might not be important in this case, but it is something you should be aware of.


Overall, if you wanted to have an object-oriented solution to your problem, you want more explicitness, a lot more. That starts by the input, something like this:

Please enter number of ingredients: 4
Please enter needed amount for ingredient #1: 2
Please enter needed amount for ingredient #2: 5
Please enter needed amount for ingredient #3: 6
Please enter needed amount for ingredient #4: 3
Please enter available amount for ingredient #1: 20
Please enter available amount for ingredient #1: 40
Please enter available amount for ingredient #1: 90
Please enter available amount for ingredient #1: 50
You can fabricate 8 items.

You could create a Ingredient class which holds the required amount as well as the available amount and use that throughout your program. Also, you might want to reconsider of having everything static, and you want to extract reading from stdin into its own class, so that you can, for example, easily add reading from a file.

| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ I had expected that you commented on System.exit being called with an index, as that doesn't make sense at all. If you missed that, I can totally understand that, since the code offers so many other things to comment on. :) \$\endgroup\$ – Roland Illig May 30 at 20:25
  • \$\begingroup\$ Actually, I saw it, then forgot to comment on it...thanks for reminding. \$\endgroup\$ – Bobby May 31 at 4:59
  • \$\begingroup\$ Thank Bobby for all the suggestions. \$\endgroup\$ – Zia May 31 at 7:35
  • \$\begingroup\$ Hey Roland, you can also comment on that and let me know the best practice or suggestions. \$\endgroup\$ – Zia May 31 at 7:36
4
\$\begingroup\$

I'm not going to rewrite the code.

There's a simple mathematical way to get the answer to the problem. Using your example,

4
2 5 6 3
20 40 90 50

You can divide the total by the amount used in each recipe. Using integer division:

20 / 2 = 10
40 / 5 = 8
90 / 6 = 15
50 / 3 = 16

The smallest answer is the answer.

8
| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ Hi Gilbert, My code has been already working on the same logic, please see the single SOP statement, where I print the minimum value \$\endgroup\$ – Zia May 31 at 7:23

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.