Problem
The questions from hackerearth:
Geeko is in worry now because an exam is coming up and he has to know
what rank he can get on exams. So he goes back into the the school records
and finds the amazing pattern.He finds that if a student is having a current rank n, then his rank on
the final exam will be the count positive numbers between in the range
[1,n] which are relatively prime to n.As being a geek, he became curious now and wants to calculate the rank of
all his classmates on the final exam. But he finds this task a bit hard,
so he asks you programmers to solve this task for him.Input: The first line of each test file contains a integer t denoting
the number of test case. Each test case contains a numbers n
representing the current rank of each studentOutput: for each test case output single integer the rank of student
in final exam in new line.Constraints:
1<=t<=2000
My solution:
import java.io.BufferedReader;
import java.io.IOException;
import java.io.InputStreamReader;
import java.util.ArrayList;
import java.util.Scanner;
class relative {
public static void main(String[] args) throws IOException {
BufferedReader br = new BufferedReader(new InputStreamReader(System.in));
int t = Integer.parseInt(br.readLine());
ArrayList<Integer> input = new ArrayList<Integer>();
ArrayList<Integer> result = new ArrayList<Integer>();
// int input[] = new int[t];
// int result[] = new int[t];
for (int i = 0; i < t; i++) {
int count = 1;
int in;
input.add(in = Integer.parseInt(br.readLine()));
for (int j = 2; j < in; j++) {
if (in % j == 0)
continue;
else if (checkRelativePrime(j, in))
count++;
}
result.add(count);
}
for (int i : result)
System.out.println(i);
}
static boolean checkRelativePrime(int a, int b) {
int i = 2;
while (a > 1) {
if (a % i == 0) {
{
if (b % i == 0)
return false;
}
a = a / i;
} else
i++;
}
return true;
}
}
Solution
in your checkRelativePrime
method you have
static boolean checkRelativePrime(int a, int b) {
int i = 2;
while (a > 1) {
if (a % i == 0) {
{
if (b % i == 0)
return false;
}
a = a / i;
} else
i++;
}
return true;
}
- you have an extra set of brackets
-
you have a statement in purgatory.
a = a / i;
it sits in-between the ending bracket of your if statement and the else statement that is supposed to be attached to the if statement
-
if you use brackets for one part of an if statement, you should use them for the entire if statement. I recommend using brackets even for one-liners no matter what, it's cleaner and leaves less room for mistakes.
I think you wanted to keep that if statement separate from the declaration to the a
variable, but you don't need to do that(especially if you are using brackets properly)
like this
while (a > 1) {
if (a % i == 0) {
if (b % i == 0) {
return false;
}
a = a / i;
} else {
i++;
}
}
if you enter that if statement and return false then a = a /i;
will never happen and your boolean method will return false
Extract More Methods
Right now you have a lot of code in the main method. Ideally, you want to split up your code and have many methods each only doing one thing.
Naming Conventions
Class names should always start with a capital letter! (UpperCamelCase)
Pick more meaningful names. You're using a lot of one-character variable names, try not to do that. (It's okay for loop counters though).
Nested Assignments
You have the following:
input.add(in = Integer.parseInt(br.readLine()));
Don't use these nested assignments. "While admirably terse, such expressions may be confusing, and violate the general design principle that a given construct should do precisely one thing."
Braces
Always use braces for if-statements and loops. It just makes them that much more readable.
Unnecessary code
In your main
method, you have an ArrayList named 'input' that is only written to, but never read from. In other words, you're not using it and you might as well omit it.
You import java.util.Scanner
but it is never used. In the Netbeans IDE you can press ctrl-shift-i
to automatically sort out imports. I'm sure other IDE's have similar features.
Misc
Don't use System.out.println()
if you can avoid it.
Make variables final, unless you really need them not to be.
Code to interface. Save ArrayLists as Lists, for instance. Also, you may omit the diamond brackets as follows:
List<Integer> result = new ArrayList();
instead of:
ArrayList<Integer> result = new ArrayList<Integer>();
- Give meaningful class names to your classes, e.g. "relative" does not seem to describe anything about the data type of the object. A more meaningful name could have been "RankFinder" in the context of your problem.
- Always stick to Hungarian notation while naming your classes and functions. http://en.wikipedia.org/wiki/Hungarian_notation
-
The function
checkRelativePrime
seems wrong to me. Simply make use of an auxiliary function to compute gcd(a, b) and check if it's one. The gcd computation function is pretty standard and can works as follows.function gcd(a, b) while b ≠ 0 t := b; b := a mod b; a := t; return a
- You don't really need the array result. You can simply print the numbers in the first for loop.