HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaMinor

Text menu shopping cart for mobile phone store

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
textstoremenucartformobilephoneshopping

Problem

I made a mobile store, with prices in Indian rupees. Now I went for nested switch over functions, because I wanted to try out something new - biggest mistake in my life.

Since 3 days I've been trying to perfect it, it just won't happen. I have to make sure the menu shows again if negative number or a character is entered. There is try-catch for this, but it's bugging me.

Now my coding is huge and I'm not sure I can post the whole thing here.

I cut out the 4th case in switch (ch) to get some space.

```
import java.io.*;
import java.util.Date;

class Mobile_Store_Project
{
public static void main (String[] args) throws IOException
{
InputStreamReader isr=new InputStreamReader (System.in);
BufferedReader br=new BufferedReader (isr);

int ch=0,a,gt=0;
int pr[]=new int [10];
int qu[]=new int [10];
String x[]=new String[10];
for (a=0;a1)
{
System.out.println("Oops. Lets try this again.");
ch2=0;
continue;
}
break;

case 3: System.out.println ("The Samsung Galaxy Note 2 is the sucessor of the sucessful Galaxy Note. Specifications: \n 1)5.5 inch screen \n 2)Android 4.1 Jelly Bean \n 3)1.5 GhZ Quad Core Processor \n 4)S Pen for super productivity \n The phone is available for Rs.38,656 and only in white option. \n Enter 1 to select.");
int ch3;
ch3=Integer.parseInt(br.readLine());
if (ch31)
{
System.out.println("Oops. Lets try this again.");
ch3=0;
continue;
}
break;
case 4:
break;
default: System.out.println("Oops, lets try th

Solution

Crikey there's alot of duplicated code in there. Before I would even consider reviewing it too closely you might want to think about refactoring all those copy and pasted case statements into one method

-
Remove code that has very likely been "copied and pasted". Oh how I cringe at copy and pasted code:

x[a]="iPhone 5 16GB Black ";
System.out.println("How many do you want to buy?");
qu[a]=Integer.parseInt (br.readLine());
System.out.println(qu[a]+" "+x[a]+"Added to cart.");
pr[a]=45504;


into something like

case 1:
   capturePhoneSelection(a, "iPhone 5 16GB White", 45504);
   a++;
   break;


where the method would be something like (x, qu and pr could be class instance variables):

int capturePhoneSelection(int selection, string phoneTitle, int pr) {
    x[selection]=phoneTitle;

    System.out.println("How many do you want to buy?");
    qu[selection]=Integer.parseInt (br.readLine());

    System.out.println(qu[selection]+" "+x[selection]+"Added to cart.");
    pr[selection]=pr;

    return selection++;
}


-
Change your variable names into something that other people not reading your code can easily understant. i.e what the heck is variable a, pr and qu supposed to represent?

-
Going on from point 1. Break that main up into many smaller functions. By doing this you will notice duplicated code and so eliminate alot

-
And No, a nested switch is probably not a good idea but I haven't reviewed it in enough detail to fully comment that.

These might be a good place to start. After making these changes I'm sure there'll be even more room to refactor and review and in this process it's always good to manage in an iterative process.

Code Snippets

x[a]="iPhone 5 16GB Black ";
System.out.println("How many do you want to buy?");
qu[a]=Integer.parseInt (br.readLine());
System.out.println(qu[a]+" "+x[a]+"Added to cart.");
pr[a]=45504;
case 1:
   capturePhoneSelection(a, "iPhone 5 16GB White", 45504);
   a++;
   break;
int capturePhoneSelection(int selection, string phoneTitle, int pr) {
    x[selection]=phoneTitle;

    System.out.println("How many do you want to buy?");
    qu[selection]=Integer.parseInt (br.readLine());

    System.out.println(qu[selection]+" "+x[selection]+"Added to cart.");
    pr[selection]=pr;

    return selection++;
}

Context

StackExchange Code Review Q#19164, answer score: 5

Revisions (0)

No revisions yet.