# How can i improve this code for better metrics?

import java.util.ArrayList;
import java.util.Scanner;

/**
*
* @author cypronmaya
*/
class Point {

int x;
int y;

Point(int x, int y) {
this.x = x;
this.y = y;
}
}

private ArrayList<Point> arrayList;

this.arrayList = new ArrayList<Point>(capacity);
}

public static void main(String args[]) {

Scanner scanner = new Scanner(System.in);
int num_of_points = scanner.nextInt();
for (int i = num_of_points; --i >= 0;) {
int x = scanner.nextInt();
int y = scanner.nextInt();
}
int num_of_queries = scanner.nextInt();
for (int i = num_of_queries; --i >= 0;) {
char c = scanner.next().charAt(0);
quad_query.query(c, scanner.nextInt() - 1, scanner.nextInt() - 1);
}
}

public void add(int x, int y) {
Point point = new Point(x, y);
}

public void query(char c, int i, int j) {

switch (c) {
case 'C':
queryC(i, j);
break;
default:
queryXY(c, i, j);
break;
}
}

public void queryC(int i, int j) {

for (int k = i; k <= j; k++) {
Point pnt = arrayList.get(k);
int pnt_x = pnt.x;
int pnt_y = pnt.y;
if (pnt_x > 0 && pnt_y > 0) {
} else if (pnt_x < 0 && pnt_y > 0) {
} else if (pnt_x < 0 && pnt_y < 0) {
} else if (pnt_x > 0 && pnt_y < 0) {
}
}
}

private void queryXY(char c, int i, int j) {

for (int k = i; k <= j; k++) {
Point pnt = arrayList.get(k);
int pnt_x = pnt.x;
int pnt_y = pnt.y;
if (c == 'X') {
pnt = new Point(pnt_x, -pnt_y);
} else if (c == 'Y') {
pnt = new Point(-pnt_x, pnt_y);
}
arrayList.set(k, pnt);
}
}
}


Description: Input:

4
1 1
-1 1
-1 -1
1 -1
5
C 1 4
X 2 4
C 3 4
Y 1 2
C 1 3


First number indicates Number of points followed by N points (x,y), Next number indicates Q num_queries, followed by Q queries

1. Reflect all points between point i and j both including along the X axis. This query is represented as "X i j"
2. Reflect all points between point i and j both including along the Y axis. This query is represented as "Y i j"
3. Count how many points between point i and j both including lie in each of the 4 quadrants. This query is represented as "C i j"

Sample Output: 1 1 1 1 1 1 0 0 0 2 0 1

-
Can you add the expected output for the input you gave ? As a first quick and useless comment, I would say that variable "num_of_points" is not really useful. –  Josay Jan 2 '12 at 22:33

Instead of initializing your arrayList when you create the instance of the class, do it when you have the num_of_points value from the scanner.

arrayList = new ArrayList<Point>(num_of_points);


This will mean the array list doesn't need to resize itself (potentially slow operation). In the example you have given with only 4 points it won't make any difference, but if there were a larger number of points you may save some time!

Where you get the value from the scanner char c you trim the value, which you shouldn't need to do as the Scanner should stop when it sees a delimiter (in this case, a space).

In the queryC method, you have a for loop...

for (int b = 0; b < len; b++) {
pnt = arrayList.get(b);
}


...that does nothing, so you can remove it.

A few notes on code style...

• In the main method your for loops go from X down to 0, but in the query methods you go from 0 up to X. Just wondering why?
• You also use (0 - pnt.x) where you could use -pnt.x.

Hope this helps.

-
sorry,i forgot to remove that useless loop before only,regarding (o-pnt.x) -- written simply, loops go down in main method from X->0 because i wouldn't use the loop variable in it,the way i've written loops in main method, would optimized one's –  cypronmaya Jan 3 '12 at 3:59
those trim, that i've done just for safety purposes,i know scanner would take exactly what to take, but simply, please look at modified version –  cypronmaya Jan 3 '12 at 4:00
@cypronmaya The only other thing I can see is that you have used variables int pnt_x = pnt.x I wouldn't bother with this variable as the compiler should inline it anyway. It is coded in a pretty readable manner which as @MikeNakis said, is the main thing to worry about. –  luketorjussen Jan 3 '12 at 14:49
Also you say that you write the for loops in the main method that way because you're not using the loop variable inside the loop - why not turn it into a much more readable while loop? –  luketorjussen Jan 3 '12 at 14:51
Ya, u've given lot of usefull inpur, but still i'm not able to achieve the req thing, execution time failed –  cypronmaya Jan 3 '12 at 14:53

for (int i = num_of_queries; --i >=0;) is cute, but it makes one pause for a moment and think whether it is correct or not before moving on. It is better rewritten as for (int i = num_of_queries; i > 0; i--).

Your arraylist is allocated once and never changed, (its contents are ganged, but not the object itself,) so it should be declared as final. Right before you start using it, since you know how many items are going to go into it, do arrayList.ensureCapacity(num_of_points); so as to help it to avoid resizing itself as it is discovering its capacity by itself.

Use better names; quad_qu should be quad_queue or perhaps just queue.

In QueryC:

Get rid of the pnt_x and pnt_y variables and simply use pnt.x and pnt.y instead.

Declare pnt inside the loop, not outside it: Point pnt = arrayList.get(k);

Rename pnt to point, or pt, or just p.

In QueryXY:

Again, declare pnt inside the loop, not outside it.

Declare new_pnt inside the loop, too, and do not initialize it to null. It is bad practice to initialize things when there is no need. It prevents the compiler from giving you useful warnings. When you do this, the compiler will give you a warning that you may be trying to use new_pnt before you have assigned a value to it. That's because your if statement checks for 'X', and then for 'Y', but there is no else clause to handle any other possibility. So, add an else clause and throw an exception: throw new Exception("wtf?")

Get rid of these: int pnt_x = 0, pnt_y = 0; they are not used. Actually, the fact that you have these in your code, and you have not seen a compiler warning about them not being used, tells me that you are not compiling with all warnings enabled. Do yourself a favor and enable all warnings so as to have the compiler help you. That's his job.

Replace this:

arrayList.remove(k);

With simply this: arrayList.set(k, new_pnt);