patternjavaMinor
Program for finding Fibonacci primes
Viewed 0 times
primesfibonacciprogramforfinding
Problem
I think it could have been designed with more object orientation. I don't like how one of my methods calls another from within the method, but I wasn't sure how to return the result because it is a loop.
Also is it ok to have all methods static, or should I be instantiating the classes?
If there is any other improvements please let me know, but these were my main concerns.
App.java:
Fibonacci.class:
Primes.java:
Also is it ok to have all methods static, or should I be instantiating the classes?
If there is any other improvements please let me know, but these were my main concerns.
App.java:
public class App {
public static void main(String[] args) {
long startTime = System.currentTimeMillis();
Fibonacci.fibonacci();
long endTime = System.currentTimeMillis();
long totalTime = endTime - startTime;
System.out.println("****************************************\n");
System.out.println("Total Running Time: " + totalTime + "ms");
}
}Fibonacci.class:
public class Fibonacci {
static void fibonacci() {
long i = 0;
long j = 1;
long k = 0;
while (i < 1E17) {
k = i + j;
i = j;
j = k;
Prime.prime(k);
}
}
}Primes.java:
public class Prime {
static void prime(long number) {
boolean isPrime = false;
long end = (long) (Math.sqrt(number) + 1);
for (long i = 2; i <= end; i++) {
if (number % i == 0) {
isPrime = false;
break;
} else {
isPrime = true;
}
}
if (isPrime) {
System.out.println(number);
}
}
}Solution
Object Oriented
I wouldn't call your code object oriented. And yes, using
But your program is so small and specific that this isn't really a bad thing. If you actually have some extension in mind, a different approach might be better, but right now, I would leave it as it is.
But for example, lets say you plan to write a program in the future which prints every odd Fibonacci number, or every Fibonacci number dividable by 3. With your code, this might be harder to do.
If your approach was like this:
It would be easily extendable. But as I said, if you are not planning on extending your code, you don't really need OOP in this case. I wouldn't even create the
Naming
Other
I would pass an argument to
I wouldn't call your code object oriented. And yes, using
static in too many places can be a hint that you are not using OOP correctly.But your program is so small and specific that this isn't really a bad thing. If you actually have some extension in mind, a different approach might be better, but right now, I would leave it as it is.
But for example, lets say you plan to write a program in the future which prints every odd Fibonacci number, or every Fibonacci number dividable by 3. With your code, this might be harder to do.
If your approach was like this:
public class Fibonacci {
static void fibonacci(NumberCheck numberCheck) {
[...]
// instead of Prime.prime(k); call:
if (numberCheck.meetsContition(k)) {print(k)}
}
}
interface NumberCheck {
boolean meetsContition(int number);
}
public class Prime implements NumberCheck {
@Override
boolean meetsContition(int number) {
// check if number is prime, return true or false
}
}
public class Odd implements NumberCheck {
@Override
boolean meetsContition(int number) {
// check if number is odd, return true or false
}
}It would be easily extendable. But as I said, if you are not planning on extending your code, you don't really need OOP in this case. I wouldn't even create the
Fibonacci and Prime classes, they are more confusing than they are helpful, just put the methods in App.Naming
prime should better be called isPrime, and I would find it easier to read if it then returned true or false (move the printing to fibonacci).Other
I would pass an argument to
fibonacci for up to which number it should run. Just hardcoding 1E17 is bad style.Code Snippets
public class Fibonacci {
static void fibonacci(NumberCheck numberCheck) {
[...]
// instead of Prime.prime(k); call:
if (numberCheck.meetsContition(k)) {print(k)}
}
}
interface NumberCheck {
boolean meetsContition(int number);
}
public class Prime implements NumberCheck {
@Override
boolean meetsContition(int number) {
// check if number is prime, return true or false
}
}
public class Odd implements NumberCheck {
@Override
boolean meetsContition(int number) {
// check if number is odd, return true or false
}
}Context
StackExchange Code Review Q#60425, answer score: 7
Revisions (0)
No revisions yet.