What's with unorganized code?

General Discussions
#1

I’ve never had this problem unless I was running a very deep algorithm. My code looks godly.

package Core;

import java.awt.Color;
import java.awt.Graphics2D;
import java.awt.Point;
import java.util.ArrayList;

public class HexGrid {

	int size;
	ArrayList<Point> points = new ArrayList<Point>();
	ArrayList<Troint> troints = new ArrayList<Troint>();
	Point[][] pointA;

	public HexGrid(int size){
		this.size = size;
		setupGrid(size);
	}
	
	public void update() {
		
		
	}

	public void draw(Graphics2D g) {
		
		int offset = size*24;
		int spacing = 20;
		for(int c = 0; c < points.size(); c++){
			
			g.setColor(Color.green);
			
			Point pFill = points.get(c);
			Troint tFill = troints.get(c);
			
			if(tFill.n == 0 && tFill.v == 0){
				g.setColor(Color.red);
			}
			
			if(tFill.n > 0){
				g.fillOval(tFill.n*(spacing)+offset, tFill.v*(spacing)+offset-(size*(spacing/2))+((size-tFill.n)*(spacing/2)), (spacing), (spacing));
			}else
			if(tFill.n <= 0){
			g.fillOval(tFill.n*(spacing)+offset, tFill.v*(spacing)+offset+((tFill.n)*(spacing/2)), (spacing), (spacing));
			}
		}		
	}
	int numKeep = 0;
	public void setupGrid(int size){
		
		pointA = new Point[size*2+1][size*2+1];

		
		int count = 1;
		for(int o = 0; o < size; o++){
			
			for(int i = 0; i <  size+1+o; i++){
				System.out.println((o-size)+":"+(i-o)+":"+(Math.abs(o-size)-(i-o)));
				points.add(new Point(o-size,i-o));
				troints.add(new Troint(o-size,Math.abs(o-size)-(i-o),i-o));
				count++;
			}			
		}
		
		for(int o = 0; o < size*2+1; o++){
			System.out.println(0+":"+(o-size)+":"+-(o-size));
			points.add(new Point(0,o-size));
			troints.add(new Troint(0,-(o-size),o-size));
			count++;
		}
		
		for(int o = 0; o < size; o++){
			
			for(int i = 0; i <  Math.abs(o-size*2); i++){
				System.out.println((o+1)+":"+(i-o-1)+":n, "+(-(o+1)+Math.abs(i-o-1)));
				points.add(new Point(o+1,i+o-size+1));
				troints.add(new Troint(o+1,-((o+1)+Math.abs(i-o-1)),i+o-size+1));
				count++;
			}			
		}
	}
}

And you may be asking yourself, wtf is a TROINT? It’s just a Point with 3 coordinates instead of 2 like a normal point. Dealing with Hexagons that is…

package Core;

public class Troint {

	//n = negative line coordinate;
	//p = positive line coordinate;
	//v = vertical line coordinate;
	
	public int n;
	public int p;
	public int v;
	
	public Troint(int n, int p, int v){		
		this.n = n;
		this.p = p;
		this.v = v;		
	}
	
	public int getN(){
		return n;
	}
	public int getP(){
		return p;
	}
	public int getV(){
		return v;
	}
	
}
#2

WtF IS TROINT? Isn’t it just vec3i ?

#3

You forgot to place an enter between the two methods :open_mouth:

#4

Ah…oh nevermind.

#5

So, uh, the point of this post is to brag about your code?

The code you posted is rather trivial, so it shouldn’t be too hard to keep it “organized”. And organization is subjective: I personally wouldn’t have numKeep declared in the middle of the class, especially since you don’t actually use it. I could further nitpick by pointing out that you violated standard naming conventions by capitalizing your package, plus you only have a single comment (we shouldn’t have to guess at what a “Troint” is), as well as an empty function. You also have public variables as well as getters which is redundant. You’ve also coded to the class instead of the interface in a few places.

It becomes more difficult to keep your code “organized” when you’re working on a larger project. It becomes impossible to keep your code “organized” when you’re working with more than just yourself.

Work on a project of 100,000 lines of code split between 5 people, then brag about your code! :stuck_out_tongue:

(I am not trying to start a fight or be overly critical, just trying to maybe provide some perspective.)

#6

On a partially related topic
How do you all do your curly braces? I prefer

for (int i = 0; i < 10; i ++)
{
}

but some people have the { brace afterwards:

for (int i = 0; i < 10; i ++){

}

#7

The official Java naming conventions say that the opening brace goes on the same line as the statement. Recommended reading: http://www.oracle.com/technetwork/java/codeconventions-150003.pdf

However, what you’re touching on is one of those pointlessly debated conventions that don’t actually matter:




[text: "braces go on the next line you tremendous fuckass", photograph of clear, still lake with snow capped mountains in the background and smooth stone beach in the foreground] [HD Version]

#8

^I know, I’m aware it’s more widely done on the same line, was really just wondering why.

It’s interesting to read how many code conventions there are though, it’s quite surprising

#9

While you’re prettifying your code, you might as well put spaces between operands and have consistent indentation in your if else blocks.
Or use a formatter. :point:

#10

Not to mention the use of the String append operator inside of a loop. That should be a StringBuilder.

If this code is “godly”, I wonder what it’s the god of…? :stuck_out_tongue:

#11

And really, one could say that most of that code could probably be reduced to only a few lines with some lambdas and map stuff.

#12

I’m getting such an ignorant vibe right now…

#13

Also, in your ‘troint’ class, why is there public member variables but also functions for encapsulation? Isn’t the point of encapsulation to make sure that values are only set in a certain way? Maybe I’m wrong.

#14

numKeep was there to determine how my forloop was iterating, I would ++ it in the nested forloop to see how many iterations it would do in code that no longer exists in there.

#15

I don’t care for encapsulation if I’m not sharing the code for someone else to work on. I just set everything to public.

#16

So, basically you are lazy and go against good programming practices just because you aren’t sharing your code? Sorry, but that is just silly. Your code shows that you really don’t have a care for many standards or conventions and your code is suffering because of it. Learn to write the best code you can write all the time, not just when other people are looking at it.

And honestly, Troint? Really? Your class is simply a container object for three integers, just call it a vector. A three dimensional vector. Or something along those lines. Don’t use names that aren’t common knowledge, that’s another sort of convention. Code readability is super important.

#17

Hi

It’s funny because you made this post just to show off your ‘godly’ programming and you’ve so far shown that your code is far from godly :). Not only do you make variables public and then add getters for them, but your indentation is less than godly and your code sample is not nearly complex enough to uncover true coding practices :slight_smile:

Just in case anybody is curious my post was meant to summarize the flaws in your ‘godly’ code and to share my views :slight_smile:

CopyableCougar4

#18

So many serious responses… lol.

I’m not lazy, using the public modifier isn’t a bad practice, I just use that for everything. It can be changed later.

I use setters and getters so that I can go back and make changes.

#19

[quote=“micecd,post:18,topic:49674”]
This is pretty much the definition of bad practice. I can tell you’ve never worked on a team before! That’s not necessarily a bad thing, but you might want to look at real code samples before calling your code “godly”.

The public attributes are nowhere near the only thing wrong with your code.

#20

Perhaps this was a bad example as the first snippet was something I was in the middle of debugging…