BitShift code compacting

Question

I'm new to Java programming and am really enjoying it. I made a bitshift program, but I think it's too long.

I expect the program to:

1. Let users input the number and degree to be shifted
2. Specify if it is a right or left shift
3. Restart the program if the user requires at the end
4. Ensure exceptions don't crash the program

import java.util.Scanner;
class BitShift{
    int a,b;
    String c;


    static int shiftNo(){
        Scanner sc=new Scanner(System.in);
        int x=sc.nextInt();
        return x;
    }

    static String shiftDir(){
        Scanner sc=new Scanner(System.in);
        String x=sc.nextLine();
        return x;
    }   
    }

    class BitShiftObjects1 extends BitShift{

    public static void main(String []args){ 

        int answer;
        BitShift ob1=new BitShift();

    boolean isValue=false;
    while(isValue==false){
        try{
            System.out.println("Please enter number to be shifted");
            ob1.a=shiftNo();
            System.out.println("Please enter degree to be shifted");
            ob1.b=shiftNo();
            isValue=true;
        }   
        catch(java.util.InputMismatchException e){
            System.out.println("ERROR:**Please Input Integer Number**");
            System.out.println("**Please enter from begining**");
            System.out.println("");
        }

    }

    isValue=false;
    while(isValue==false){

            System.out.println("Please enter r or l");
            ob1.c=shiftDir();

            if(ob1.c.equals("r") || ob1.c.equals("l"))
                isValue=true;
            else
                System.out.println("ERROR:**Please enter LOWERCASE r/l**");

        }

        if(ob1.c.equals("r")){
            answer=ob1.a >> ob1.b;
            System.out.println("Your Answer is : "+answer); 
                }
        else{
            answer=ob1.a << ob1.b;
            System.out.println("Your Answer is : "+answer); 
                    }

        System.out.println("");
        System.out.println("Do you want to enter a new data set??");
        System.out.println("Press y or n");


        isValue=false;
        while(isValue==false){
        ob1.c=shiftDir();

        if(ob1.c.equals("y")){
            System.out.println("**Reinitializing program**");
            isValue=true;
            main(args);
        }
        else if(ob1.c.equals("n"))
            isValue=true;   
        else
            System.out.println("ERROR:**Please enter lowercase y or n**");
        }
    }
}

Answer 1

Formating

Correct indentation and spaces make the code a lot easier to read. Just paste your code in any decent IDE and it should manage this for you automatically.

• Spaces before curly brackets
• Spaces before and after equals sign
• Indentation: closing brackets should be on the same level as the beginning of the opening statement
• Indentation: Elements on the same level should be indented the same amount

Variable names

• a, b, c, x, sc, ob1, those are all bad variable names. The bigger the scope of the variable, the more important it gets that it has a good name. So while sc for scanner is probably still alright, a, b, and c are definitely not.
• shiftNo and shiftDir should better be named shiftAmount and shiftDirection. Or, to make them more reusable: getInputString and getInputInteger.
• isValue sounds confusing. Maybe recievedInput or something like it. And then, don't do this: while (recievedInput == false), but this: while (!recievedInput)

Comments

Your code is missing comments. If it is self-documenting code this might be alright. But in this case: What are a, b, and c? Does shiftDir shift something? (Maybe a directory?) No, it gets input. The method name and/or the comment should make this clear.

Class Structure

see the answer of Puneet_Techie

In addition to that, you might also want to extract the actual shifting to their own methods. It makes the code cleaner, and it also makes it easier to handle corner cases and to write unit tests.

Corner Cases / Input validation

What if I want to shift 3 by 100? Is the output still correct? Or if I want to shift -4 by 5? Or if I want to shift 3 by -4?

Make sure that you only allow input that you actually want. And that the result is always what you expect it to be.

Answer 2

Much of what I would comment on has already been covered.... but, I don't see any mention of the main method recursion... which, frankly, is a bug.

Why? Well, how many times will the user want to process some input? 5 times, 10 times, 100 times, 1000 times? Somewhere between about 100 and 1000 the program will crash with a StackOverflowException. Each time the user says 'y', you add a new level to the stack.

The right solution is a simple do-while loop:

boolean repeat = false;
do {
    ... process your value ...

    System.out.println("Do you want to enter a new data set??");
    repeat = "y".equals(....);
} while (repeat);

For the record, everything inside that do-while loop that is represented with ... process your value ... should probably be extracted to a function.

Answer 3

Apart from suggestions made by Tim I would like to suggest the following:

1. Choose a proper className. BitShiftObjects1 is not a good name.

2. While defining instance methods and instance variables, please use access specifiers.

3. Your code can be completed in Only One While Loop instead of 3 while loops

4. Each class should own only one responsibility.

5. Usage of isValue flag is confusing. I think user3889963 wants client to input values until its a valid One(Numeric / String) ... But name isValue doesn't reflects that ,instead if it would have been isValidInput it would have been informative. So pseudo code while(isValidInput){ ask user to enter data ... }, would be self explanatory.

With said above I have written below Program. Please let me know if it serve your functionality

package com.study.bit;

import java.util.Scanner;

class BitShift{

    public int bitShifting(int number,int degreeToBeShifted ,char direction){
        if(direction=='r'){
            return number >> degreeToBeShifted;
        } else if(direction=='l'){
            return number << degreeToBeShifted;
        }
        else{
            System.out.println("ERROR:** Shift direction is invalid only LOWERCASE r/l  are accepted ");//I think developer wants to restart the 
            System.out.println("Assuming the Right shift  ... ");
            return number >> degreeToBeShifted;
        }

    }
}

public class BitShiftUtil  {
    private static int  readNumeric(){
        Scanner sc=new Scanner(System.in);
        int x=sc.nextInt();
        return x;
    }

    private static char readCharacter(){
        Scanner sc=new Scanner(System.in);
        String line=sc.nextLine();
        return (null!= line && line.trim().length()>0 ? line.charAt(0): ' ') ;
    }

    public static void main(String []args){
        BitShift shifter = new BitShift();

        int number,degreeToBeShifted;
        char direction;
        char shouldContinue ='y';
        do{
            try{
                System.out.println("Please enter number to be shifted");
                number= readNumeric();
                System.out.println("Please enter degree to be shifted");
                degreeToBeShifted= readNumeric();
                System.out.println("Please enter r or l");
                direction=  readCharacter();
                System.out.println("Your Answer is : "+shifter.bitShifting(number, degreeToBeShifted, direction));
                System.out.println("Do you want to enter number new data set?? Press Y to continue.. Press any key to exit");
                shouldContinue=  readCharacter(); 
            }   
            catch(java.util.InputMismatchException e){
                System.out.println("ERROR:**Please Input Integer for number and Degree to be shifted and Character for Direction and to restart program**");
            }             
        }while( shouldContinue=='y' || shouldContinue =='Y' );

        System.out.println("Exiting the program......");
    }
}

Answer 4

I think you can do away with the Class BitShift and write your functionality in one class only, unless you can give me some specific reason for using inheritance. Also, i think instead of calling the method and taking the input, you can just ask the user to give the option of his choice and then use switch case to call the particular operation, something like -:

System.out.println("Enter the choice of operation 1 - xoperation , 2-yoperation");
int choice = user input value
switch(choice)

All the exception handling for a particular case can be done in methods.