patternjavaMinor
Is there a better version for verbosing the output of the euclidean method?
Viewed 0 times
themethodversioneuclideanoutputbetterforthereverbosing
Problem
Here is my implementation of the Euclidean Algorithm. My question is how to make it more "professional". It's working right, but isn't this too newbie?
public class Euklideszi {
public int Euklideszi(int a, int b, boolean lepesrolLepesre) {
int r = a % b;
if (a < b) {
r = a;
a = b;
b = r;
}
if (a % b == 0) {
if (lepesrolLepesre) {
System.out.println("A legnagyobb közös osztó: " + b);
}
return b;
} else {
while (r != 0) {
r = a % b;
if (lepesrolLepesre) {
System.out.println(a + "=" + a / b + "*" + b + "+" + r);
}
a = b;
b = r;
}
if (lepesrolLepesre) {
System.out.println("A legnagyobb közös osztó: " + a);
}
return a;
}
}
}Solution
There are a couple of "newbie" things here, yes.
-
Your method has an uppercase first character, it should be lower-case.
-
Your method is not static, which means that an instance of the class has to be created just to use your method. You can make the method static, and add a private constructor to the class so that it can't be initialized from outside.
-
As @skiwi says in a comment, please use English identifier names when you code.
-
-
This code:
is used to switch so that
Note that here, I put the initial calculation of
-
There is no need to use the
Edit:
In the comments below you asked for a cleaner way for the user to see every step, you can accomplish this by using a callback
Then you can modify your code to:
If you're able to use Java 8, you can use a lambda for the callback:
This callback system is more flexible in case you'd want to save each of the steps to a
-
Your method has an uppercase first character, it should be lower-case.
-
Your method is not static, which means that an instance of the class has to be created just to use your method. You can make the method static, and add a private constructor to the class so that it can't be initialized from outside.
-
As @skiwi says in a comment, please use English identifier names when you code.
-
boolean lepesrolLepesre is used to toggle whether or not the method should log anything to console. This is a bad thing. A method like this should not output anything to the console, it should simply return the result.-
This code:
if (a < b) {
r = a;
a = b;
b = r;
}is used to switch so that
a always is the largest variable, and it's using r as a temporary variable. Even though this works, I would use a temp variable here that's only visible in this scope.if (a < b) {
int temp = a;
a = b;
b = temp;
}
int r = a % b;Note that here, I put the initial calculation of
r after this switch has taken place.-
There is no need to use the
else as you return inside the if. You can remove else and reduce the indentation there.Edit:
In the comments below you asked for a cleaner way for the user to see every step, you can accomplish this by using a callback
interface.Then you can modify your code to:
public static interface EuklidesStepCallback {
void onStep(int a, int b, int r);
}
public static int gcd(int a, int b, EuklidesStepCallback callback) {
...
while (r != 0) {
r = a % b;
if (callback != null) {
callback.onStep(a, b, r);
}
a = b;
b = r;
}
return a;
}
public static void main(String[] args) {
EuklidesStepCallback callback = new EuklidesStepCallback() {
@Override
public void onStep(int a, int b, int r) {
System.out.println(a + "=" + a / b + "*" + b + "+" + r);
}
};
System.out.println(gcd(24, 16, callback));
}If you're able to use Java 8, you can use a lambda for the callback:
public static void main(String[] args) {
System.out.println(gcd(24, 16, (a, b, r) -> System.out.println(a + "=" + a / b + "*" + b + "+" + r)));
}This callback system is more flexible in case you'd want to save each of the steps to a
List or similar.Code Snippets
if (a < b) {
r = a;
a = b;
b = r;
}if (a < b) {
int temp = a;
a = b;
b = temp;
}
int r = a % b;public static interface EuklidesStepCallback {
void onStep(int a, int b, int r);
}
public static int gcd(int a, int b, EuklidesStepCallback callback) {
...
while (r != 0) {
r = a % b;
if (callback != null) {
callback.onStep(a, b, r);
}
a = b;
b = r;
}
return a;
}
public static void main(String[] args) {
EuklidesStepCallback callback = new EuklidesStepCallback() {
@Override
public void onStep(int a, int b, int r) {
System.out.println(a + "=" + a / b + "*" + b + "+" + r);
}
};
System.out.println(gcd(24, 16, callback));
}public static void main(String[] args) {
System.out.println(gcd(24, 16, (a, b, r) -> System.out.println(a + "=" + a / b + "*" + b + "+" + r)));
}Context
StackExchange Code Review Q#55262, answer score: 6
Revisions (0)
No revisions yet.