patternjavaMinor
Fibonacci using OOP methods in Java
Viewed 0 times
fibonaccijavamethodsusingoop
Problem
I made a simple program that uses OOP methods. Please tell me if I used the 4 methods right. And if not, please show me how.
class Encap{
private String name;
public void setName(String newValue){
name = "Hello, "+newValue+"!";
}
public String getName(){
return name;
}
}
abstract class Abstraction {
int fibCount, flag=0, y;
String inputName;
String names[] = new String [] {"Anna","Joe","Carl"};
Scanner x = new Scanner(System.in);
abstract void abscompute();
}
class abs2 extends Abstraction{
@Override
void abscompute()
{
System.out.print("Enter number of fibonacci: ");
fibCount = x.nextInt();
int[] fib = new int[fibCount];
fib[0] = 0;
fib[1] = 1;
for(int i=2; i < fibCount; i++){
fib[i] = fib[i-1] + fib[i-2];
}
for(int i=0; i< fibCount; i++){
System.out.print(fib[i] + " ");
}
}
}
public class Fibonacci {
public static void main(String[] args) {
Abstraction abs = new abs2();
Encap encap = new Encap();
System.out.print("Enter your name: ");
abs.inputName = abs.x.next();
for (String name : abs.names) {
if(abs.inputName.equals(name)){
encap.setName(abs.inputName);
System.out.println(""+encap.getName());
abs.flag = 1;
}
}
if(abs.flag!=1){
System.out.println("Name doesn't exist..");
}
else{
abs.abscompute();
}
}
}Solution
class Encap{I find all your class names confusing. You seem to be describing the code, not what it does. Maybe something like
class Greeter {would be more understandable.
private String name;
public void setName(String newValue){
name = "Hello, "+newValue+"!";
}The
name variable isn't a name, but a greeting. Why not call it that? Also,
newValue isn't very descriptive. That actually is a name, so private String greeting;
public void setGreeting(String name) {
greeting = "Hello, " + name + "!";
}Now I can easily see that
setGreeting takes a name as an input. Some people might argue that
setGreeting should only set the greeting, not determine what it is. You could either create a constructor to handle this with an immutable variable, or you could change the name to changeGreeting. Or you can ignore that convention, but you may get negative feedback. abstract class Abstraction {This could be
abstract class Sequence {
private int[] sequence;
Sequence(int length) {
sequence = new int[length];
}
int getLength() {
return sequence.length;
}
abstract void generate();
}This seems a clearer set of names.
You also may want to look into how to use constructors
int fibCount, flag=0, y;It's generally preferable to have one variable per declaration, particularly if you are initializing as well.
int fibCount;
int flag = 0;
int y;Now you can easily see that only
flag is initialized. Why declare
y? You never use it. Why make
fibCount and flag object fields? You only use them in one method. They could be local variables instead. String names[] = new String [] {"Anna","Joe","Carl"};This doesn't seem to have anything to do with the rest of this. Why declare that variable in this class?
Scanner x = new Scanner(System.in);Why name your
Scanner x? Is it a secret that it is a scanner? More common names would be scan or scanner. You could also name it something like input or stdin. I'd move the input and output out of the class and into
main. Then you can make scanner a local variable as well. class abs2 extends Abstraction{Again, this would be clearer as
class FibonacciSequence extends Sequence {Now I know what I expect to find here.
for (String name : abs.names) {
if(abs.inputName.equals(name)){
encap.setName(abs.inputName);
System.out.println(""+encap.getName());
abs.flag = 1;
}
}
if(abs.flag!=1){
System.out.println("Name doesn't exist..");
}
else{
abs.abscompute();
}This would be easier to read as a method call:
if (validName(inputName)) {
greeting.setGreeting(inputName);
sequence.generate();
}
else {
System.out.println("Name doesn't exist..");
}Then instead of
abs.flag = 1;
}
}You could say
return true;
}
}
return false;There are other places with similar feedback, but perhaps you should take a try at it yourself.
Code Snippets
class Encap{class Greeter {private String name;
public void setName(String newValue){
name = "Hello, "+newValue+"!";
}private String greeting;
public void setGreeting(String name) {
greeting = "Hello, " + name + "!";
}abstract class Abstraction {Context
StackExchange Code Review Q#117444, answer score: 3
Revisions (0)
No revisions yet.