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

BitShift code compacting

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

Problem

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:

  • Let users input the number and degree to be shifted



  • Specify if it is a right or left shift



  • Restart the program if the user requires at the end



  • 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("");

Solution

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.

Context

StackExchange Code Review Q#58472, answer score: 5

Revisions (0)

No revisions yet.