2D vertex of multiple countries

CodeChef - QPOINT


Given a 2D vertex of multiple countries, find which country contains a certain point.


Input format:

"Number of countries"
"Number of vertex" "x1" "y1" "x2" "y2" ...
"Number of vertex" "x1" "y1" "x2" "y2" ...
...
"Number of query"
"x1" "y1"
"x2" "y2"
...


Please give me any advise on code style or design patterns.

`class QPOINT
{

class Point
{
public double X;
public double Y;

public Point()
{
X = 0;
Y = 0;
}

public Point(double x, double y)
{
X = x;
Y = y;
}
}

class Line
{
public Point point1;
public Point point2;

public Line()
{
point1 = new Point();
point2 = new Point();
}

public Line(Point point1, Point point2)
{
this.point1 = point1;
this.point2 = point2;
}
}

class CountryUnit
{
public List vertex = new List();
public List boundaries = new List();
}

List CountryList = new List();

public static void Main(string[] args)
{
QPOINT qpoint = new QPOINT();

qpoint.ReadCountry();
qpoint.ReadQueryAndPrintResult();
}

public void ReadCountry()
{
// First line is the total number of countries
int countryCount = int.Parse(Console.ReadLine());

// Each following N lines contain all vertex of a single country
for (int i = 0; i boundaries = country.boundaries;

// Create a line that extends towards x, starting from the query point
Line infiniteLine = new Line(
query, new Point(1000000, query.Y)
);

// Count the total times of intersection between the infinite line and boundaries
int intersectCount = 0;

foreach (Line boundary in boundaries)
{
// If the point is on the boundary, return true
if (P

Some quick remarks:

• Don't make class names all caps.

• State your access modifiers explicitly.

• public double X;: This is intended as a property and thus should have a getter/setter.

• public Point point1;: This is intended as a property and thus should have a getter/setter. It should also be PascalCase.

• public List vertex = new List();: vertex is a single item, the plural is vertices. Also, this is intended as a property and thus should have a getter/setter. It should also be PascalCase.

• class Point, class Line, class CountryUnit should IMHO not be subclasses of class QPOINT, but should be classes of their own (and separate files).

• Why do for (int i = 0; i

• Your code assumes the input is always in the correct format. You shouldn't anticipate that, and you should check that the received input is in the correct format.

• Point a, Point b, Point c, Point d: these are meaningless names.






• Many of your comments are not necessary, e.g. Else, count intersections.

• Your code contains far too many blank lines between methods.

• GetContainingCountry() returns an int?






• Why assign List boundaries = country.boundaries;? Why not simply use country.boundaries`?

StackExchange Code Review Q#84556, answer score: 3