Create a java program to find the cheapest plan that have the services required

Question

I am a beginner who just picked up Java (2 months ago) and I am solving some questions. For this question, the overall problem wants me to choose the cheapest price plan, so I listed out the possibilities of every plan and 'hardcoded' the logic by inputting the number of services, as the services is more important than the plan. However, I would like to improve the algorithm of this method and as a beginner my foundation is very weak.

Is there any other ways where I can further improve this method or is there any other solutions? This is because if I need to implement a new service, my entire solution will be flawed. Hence, I am looking at constructors/interfaces and arrays. However, it gets very confusing when there is a plan and services being involved and then to get the cheapest price as well.

Therefore, I would appreciate any help or advise to improve on this method of solving. Please pardon me for my weak explanation and lack of concept in Java as this is my first post.

Problem statement

Create a java program

The seller from corporation Test provides the following plans and its price:

• PLAN1: (voice, email), 100ドル per year
• PLAN2: (email, database, admin), 150ドル per year
• PLAN3: (voice, admin), 125ドル per year
• PLAN4: (database, admin), 135ドル per year

The customer wants (email, voice, admin) service. He has the following choices to cover his need:

• (PLAN1, PLAN2): 100 + 150 = 250ドル (PLAN1, PLAN3): 100 + 125 = 225ドル
• (PLAN2, PLAN3): 150 + 125 = 270ドル (PLAN1, PLAN4): 100 + 135 = 235ドル
• (PLAN2, PLAN3, PLAN4) = 150 +たす 125 +たす 135 =わ 410ドル
• ...// other possible plan choices omitted

And the minimum price is 225,ドル PLAN1 + PLAN3.

My solution

package practice;
import java.util.Scanner;
public class Test {
 private static Scanner input;
 public static void main(String[] args) {
 input = new Scanner(System.in);
 System.out.println("We have the following services:");
 System.out.println("Voice, Database, Admin, Email");
 System.out.println("How many services would you like?");
 // input number of services
 int choice = input.nextInt();
 // +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 // Logic starts here
 // when number of services required is 1
 switch (choice) {
 case 1:
 Scanner input1 = new Scanner(System.in);
 System.out.println("What type of services would you like?");
 System.out.println("1. Voice");
 System.out.println("2. Database");
 System.out.println("3. Admin");
 System.out.println("4. Email");
 // input 1 option
 int choice1 = input1.nextInt();
 switch (choice1) {
 case 1:
 System.out.println("100ドル");
 break;
 case 2:
 System.out.println("135ドル");
 break;
 case 3:
 System.out.println("125ドル");
 break;
 case 4:
 System.out.println("100ドル");
 break;
 default:
 System.out.println("Invalid option. Please try again");
 }
 break;
 // when number of services required is 2
 case 2:
 Scanner input2 = new Scanner(System.in);
 System.out.println("What type of services would you like?");
 System.out.println("1. Voice");
 System.out.println("2. Database");
 System.out.println("3. Admin");
 System.out.println("4. Email");
 // input 2 options
 int choice2 = input2.nextInt();
 int choice3 = input2.nextInt();
 // plan(1,4) -> 100ドル
 if (choice2 == 1 && choice3 == 4) {
 System.out.println("100ドル");
 // plan(1,3) -> 125ドル
 } else if (choice2 == 1 && choice3 == 3) {
 System.out.println("125ドル");
 // plan(2,3) & (3,4) -> 125ドル
 } else if ((choice2 == 2 && choice3 == 3) || (choice2 == 3 && choice3 == 4)) {
 System.out.println("150ドル");
 } else
 System.out.println("Invalid option. Please try again");
 break;
 // when number of services required is 3
 case 3:
 Scanner input3 = new Scanner(System.in);
 System.out.println("What type of services would you like?");
 System.out.println("1. Voice");
 System.out.println("2. Database");
 System.out.println("3. Admin");
 System.out.println("4. Email");
 // input 3 options
 int choice4 = input3.nextInt();
 int choice5 = input3.nextInt();
 int choice6 = input3.nextInt();
 // plan(1,3,4) -> 225ドル
 if (choice4 == 1 && choice5 == 3 && choice6 == 4) {
 System.out.println("225ドル");
 // plan(1,2,3) -> 235ドル
 } else if (choice4 == 1 && choice5 == 2 && choice6 == 3) {
 System.out.println("325ドル");
 // plan(2,3,4) -> 150ドル
 } else if (choice4 == 2 && choice5 == 3 && choice6 == 4) {
 System.out.println("150ドル");
 } else
 System.out.println("Invalid option. Please try again");
 break;
 // when number of services required is all 4
 case 4:
 System.out.println("All 4 services will cost 250ドル");
 break;
 default:
 System.out.println("Invalid input.");
 }
 }
}

Output

We have the following services:
Voice, Database, Admin, Email
How many services would you like? 
3 
What type of services would you like?
1. Voice
2. Database
3. Admin
4. Email 
1 
3 
4 
225ドル

Answer 1

Thanks for sharing your code.

your solution is straight procedural. There is nothing wrong with procedural solutions as such but Java is an object oriented programming language and therefore you should learn to find OO-ish solutions to problems.
However: this is just an advice, no judgement.

Here is what I consider problematic in your code:

General approach

no calculation

Your code hides the concept of the Service Plan from the problem statement by cascading decisions. This leads to the "spaghetti code" we see and what you consider problematic yourself.

On top of that your code fails if the user enters the service IDs in an unexpected order, eg.: 4 3 1.

A proper solution would try to calculate the best plan combination, so that adding new plans and/or services would just be a configuration.

no reuse of variables

At every assignment you create a new variable.

You declare multiple variables input with numbers. There should be only one variable input since System.in is a singelton and cannot be accessed concurrently anyway. On top of that you declared input (without number) as a class variable. Since you only use this variable at only one method in our code which is the same where you assign it its value this is not needed.

The variables choice1, choice2 and choice4 hold the same value regarding the business case: the first selected service ID. There should be only one variable firstSelectedServiceID to be used in all cases of the outer switch. Similar is true for the other numbered choice variables.

Naming

Finding good names is the hardest part in programming. So always take your time to think carefully of your identifier names.

Choose your names from the problem domain

You have some identifiers which are named after their technical implementation: the same name with numbers. This makes your code hard to read and hard to understand. Your variables should better be named like this:

• choice: should be numberOfServices
  instead of writing a comment you can name the variable accordingly
• choice1, choice2 and choice4 as mentioned before
• choice3 and choice5 should be secondSelectedServiceID
• choice6 should be thirdSelectedServiceID

• Alan Chu: Hi Timothy, thanks for taking the time out to comment on my codes. I am deeply humble by the advice and will looked to revise on my foundations level. It is my first time learning programming on my own and java is my first language. I really appreciate your advice and will looked to implement them in the near future. Cheers! :)

Answer 2

I'm going to code review from the top down. Don't be discouraged, but I have a lot of suggestions:

Change name of class 'Test'

Lots of projects put classes outside of the production build based on the ClassName starting or ending with Test. Of course it doesn't matter when you're just messing around, but it's good practice to avoid such a name for a class that is not an Automated test.

Another problem arises when you create a test for Test, if you follow the common naming pattern you're left with TestTest, which is an awkward name.

Change name of 'input'. Set it as Final.

input is a poor name. Normally input would refer to input from somewhere, such as the user. Not the object used to get the input. I suggest renaming it scanner. You can also declare it as final since it never needs to change.

Use methods & don't repeat yourself.

Whenever you copy & paste some code in more than 1 place, you should ask yourself if it could be a method. This holds true even when you make a slight change to the pasted code.

I work with a fairly large code base and I've only seen a maximum of 1-2 lines of code repeated. Even that could be reduced to 0. In other words, you should never repeat yourself.

Even if you forget about re-usability, You should get into the habit of creating methods wherever they make sense to help readability of your code.

This could be a method:

Scanner input1 = new Scanner(System.in);
System.out.println("What type of services would you like?");
System.out.println("1. Voice");
System.out.println("2. Database");
System.out.println("3. Admin");
System.out.println("4. Email");
int choice1 = input1.nextInt();

But we already have an input, so we don't need to instantiate another Scanner object. You can either take it as an parameter, or just reference it directly if it's kept as a class variable.

So a better method would look like:

private static int promptUserForBeginningServices(Scanner scanner)
{
 System.out.println("We have the following services:");
 System.out.println("Voice, Database, Admin, Email");
 System.out.println("How many services would you like?");
 return scanner.nextInt();
}

..However you may notice this method is doing 2 things: Prompting the user, & getting a result for number of services. Methods should really only be doing 1 thing. But this is our first iteration of refactoring. So we will leave it for now.

Change the names of 'choice'

A better name would be choiceEnteredByUser, or typeOfServiceSelected.

"choice" is ambiguous. Actually, it's meaningless. You could have named it x & it would be as descriptive.

Commenting

Commenting where the logic start is not helpful. Use comments to describe what code does, when it's not clear. Actually, your first approach is to use descriptive methods & descriptive method names. If it's still not 100% clear, use a comment.

ENUMS

Whenever You have a switch statement, you should ask yourself if you should be using an ENUM. That's not to say every switch statement should be an enum, but a lot of them should be. This is one of those times.

"1" is meaningless. You should never have meaningless code.

...That being said, you don't need a switch statement at all, you should be using a Map or parallel arrays.

Magic Numbers

Magic numbers / magic strings are those literal values you have hanging around. The numbers in your switch statement, the strings you output to the user ("What type of services would you like?") these should be declare as final static variables at the top of your class. This will help maintenance & readability.

End notes

I normally post the finished product when doing code reviews, but I think in this case it would do more damage than good. Don't try to move from 0 - 10 in one round of refactoring. You'll learn a LOT from simply adding methods. Your code will become 10x easier to read, you'll be able to see all the useless duplication & be able to make changes a lot easier.

After you've added methods, adding an ENUM will be another 'aha moment' or epiphany (for lack of better term).

Once you have an ENUM & methods, see if you can remove all of your magic strings.

Once you have all your String messages at the top, you'll probably notice you can group them... See if you can put them into Arrays. Afterwards, your code has literally been cut in half (by line number, assuming you can print those array messages in a loop).

• Alan Chu: Hi sir, I have not tried using ENUMS but I will look to implement it in the near future. Thanks for the advice, really appreciate you taking the time out to help me out. :)