patternjavaMinor
BitShift code compacting
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:
```
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("");
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.
Variable names
Comments
Your code is missing comments. If it is self-documenting code this might be alright. But in this case: What are
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.
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 whilescfor scanner is probably still alright,a,b, andcare definitely not.
shiftNoandshiftDirshould better be namedshiftAmountandshiftDirection. Or, to make them more reusable:getInputStringandgetInputInteger.
isValuesounds confusing. MayberecievedInputor 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.