tyeeeee1
|
 |
«
Posted
2012-12-13 23:03:47 » |
|
Hey, I've been messing around attempting to start making a game for the past few days and I felt as if I should get someone more experienced to do a quick check-over just to tell me what I'm doing wrong, what I could/should change, etc... Just so that I don't go getting into any bad habits or end up doing something that may come back to bite me later on. Code: http://pastebin.com/AUvX9Gw8I'm still actively writing this code so it's already a few lines outdated by the time anyone reads this but any criticism will help me either now or later on. ^.^ Thank's in advance! |
|
|
|
|
|
HeroesGraveDev
|
|
«
Reply #1 - Posted
2012-12-13 23:16:51 » |
|
Removing all the comments will make it easier for most programmers to read and find optimizations. Also, if you are using Java 7, you could use NIO for loading/saving. I can give some help on NIO if you want. Other than that, I can't really see anything that's wrong with all the comments distracting me, but I think there are lots of small things to do the make things easier to code for you that won't affect the final result. Using Arrays of components instead of loads of different components with a differences that could be stored in variables may help. Instead of having "STR", "DEX" etc, you could have and have constants called and etc and put the strength variable in like that. Then you could use arrays to iterate over everything and save lots of time. But then again, depending on the size (size so far and size left to go) of your project, refactoring might not be worth it. Everything is up to you. |
|
|
|
|
tyeeeee1
|
|
«
Reply #2 - Posted
2012-12-13 23:21:42 » |
|
I'm a bit of a nut when it comes to comments. I like to know exactly what everything is doing in-case I go back and can't figure out what I've done; I'll remember to remove them next time I post my code. =)
How would I go about checking if I'm using Java 7 or not, my CS class never covered different versions. I'd definitely like help with the save/loading as I only barley understood how to do it the way I currently have it from the various examples I've seen on google.
|
|
|
|
|
Games published by our own members! Check 'em out!
|
|
|
HeroesGraveDev
|
|
«
Reply #3 - Posted
2012-12-13 23:24:34 » |
|
I'm a bit of a nut when it comes to comments. I like to know exactly what everything is doing in-case I go back and can't figure out what I've done; I'll remember to remove them next time I post my code. =)
How would I go about checking if I'm using Java 7 or not, my CS class never covered different versions. I'd definitely like help with the save/loading as I only barley understood how to do it the way I currently have it from the various examples I've seen on google.
Run in command line: If it says something like 1.7.0_(some other number) then you have Java 7. If it says somthing like 1.6.0_(some other number) then you have Java 6. Oh, and when using BufferedWriter or any other kind of Buffered output stream (including BufferedOutputStream  ) you should be calling to make sure the data is written to the file. |
|
|
|
|
tyeeeee1
|
|
«
Reply #4 - Posted
2012-12-13 23:27:22 » |
|
Looks like I need to update Java, apparently I'm using 1.4.2_19 which I'm guessing must be quite old. (It's the version that my course required)
|
|
|
|
|
|
Sammidysam
|
|
«
Reply #5 - Posted
2012-12-13 23:28:24 » |
|
Looks like I need to update Java, apparently I'm using 1.4.2_19 which I'm guessing must be quite old. (It's the version that my course required)
Man I haven't seen a Java version that old in quite some time. |
|
|
|
|
|
HeroesGraveDev
|
|
«
Reply #6 - Posted
2012-12-13 23:30:27 » |
|
I believe that's Java 5 Also, found something else: When loading your file, you should be iterating over the lines instead of assuming there is at least 2 lines. 1
2
3
4
5
6
7
8
| String line;
String output = "";
while((line = in.readLine()) != null)
{
output = output + line;
}
System.out.println(output); |
|
|
|
|
|
tyeeeee1
|
|
«
Reply #7 - Posted
2012-12-13 23:37:44 » |
|
I believe that's Java 5 Also, found something else: When loading your file, you should be iterating over the lines instead of assuming there is at least 2 lines. 1
2
3
4
5
6
7
8
| String line;
String output = "";
while((line = in.readLine()) != null)
{
output = output + line;
}
System.out.println(output); |
I've just updated my JDK to Java 7. I've also attempted to re-write my LoadingClass with the information you've provided, should it look something like: 1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
| class LoadingClass
{
public static void loadGame() throws Exception
{
String line = "";
String output = "";
BufferedReader saveFile = new BufferedReader(new FileReader("TextSave.txt"));
while((line = saveFile.readLine()) != null)
{
saveFile.flush();
output = output + line;
}
System.out.println(""+output+"");
saveFile.close();
}
} |
I won't be doing too much with the Save/Loading just yet but it's always good to figure things out in advance ^.^ |
|
|
|
|
|
sproingie
|
|
«
Reply #8 - Posted
2012-12-14 00:00:49 » |
|
Okay, there is commenting and there is over-commenting. Comments should explain something that isn't obvious to someone already somewhat familiar with the language and API. Anyone who knows what a constructor does knows that statement creates a new Foo |
|
|
|
|
|
HeroesGraveDev
|
|
«
Reply #9 - Posted
2012-12-14 00:16:03 » |
|
I've just updated my JDK to Java 7. I've also attempted to re-write my LoadingClass with the information you've provided, should it look something like: 1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
| class LoadingClass
{
public static void loadGame() throws Exception
{
String line;
String output;
BufferedReader saveFile = new BufferedReader(new FileReader("TextSave.txt"));
while((line = saveFile.readLine()) != null)
{
saveFile.flush();
output = output + line;
}
System.out.println(""+output+"");
saveFile.close();
}
} |
I won't be doing too much with the Save/Loading just yet but it's always good to figure things out in advance ^.^ You don't flush input streams. It doesn't make any sense. Think of flushing like when you flush a toilet.  If there was no flush, water would be constantly flowing down, which is a waste. If there is a flush, but it isn't used, things get blocked up until the toilet overflows.   If the flush is used, it works normally. Sorry for the 'dirty' metaphor |
|
|
|
Games published by our own members! Check 'em out!
|
|
|
Nate
|
|
«
Reply #10 - Posted
2012-12-14 00:16:25 » |
|
Wow I've never seen that much over commenting. Comments should only be used when the code and variable names are not clear. It might seem like a good idea, but all it does is clutter up the code and actually reduces readability (a lot!).
|
|
|
|
|
tyeeeee1
|
|
«
Reply #11 - Posted
2012-12-14 00:21:04 » |
|
You don't flush input streams. It doesn't make any sense. Think of flushing like when you flush a toilet. If there was no flush, water would be constantly flowing down, which is a waste. If there is a flush, but it isn't used, things get blocked up until the toilet overflows. If the flush is used, it works normally. Sorry for the 'dirty' metaphor The metaphor helped a lot. I'll read up on '.flush' a lot more when I get to the point where I need to write a proper save/loading system.  As for the over-commenting, I half-expected to see something about it, I'll try to do a lot less mass-commenting from now on. |
|
|
|
|
|
ctomni231
|
|
«
Reply #12 - Posted
2012-12-14 03:33:39 » |
|
Well, if it is for your own reference and you believe it'll help you learn a lot more about Java, comment away. As long as you can find where things are within the code for yourself. Just when you are posting it on a forum like this, you just might want to either delete all the comments, or put all comments on a line above (or below) a code line so we can use an IDE to make them disappear.
|
|
|
|
|
ReBirth
|
|
«
Reply #13 - Posted
2012-12-14 04:31:59 » |
|
Usually you don't need someone else to do this, yourself from future will do it |
|
|
|
|
loom_weaver
|
|
«
Reply #14 - Posted
2012-12-14 05:19:27 » |
|
My three cents:
Storing variables in bytes and shorts is unnecessary and a premature optimization.
Naming your classes with verbs e.g. Create* is a bit strange.
Technically, instantiating your main frame should be run on the EDT as well.
|
|
|
|
|
|
masteryoom
|
|
«
Reply #15 - Posted
2012-12-14 05:44:27 » |
|
I have never seen so much commenting in my life over things that everybody should know.  |
|
|
|
|
masteryoom
|
|
«
Reply #16 - Posted
2012-12-14 05:55:53 » |
|
I ran it in eclipse and it didn't do anything.  |
|
|
|
|
tyeeeee1
|
|
«
Reply #17 - Posted
2012-12-14 21:57:18 » |
|
My three cents:
Storing variables in bytes and shorts is unnecessary and a premature optimization.
Naming your classes with verbs e.g. Create* is a bit strange.
Technically, instantiating your main frame should be run on the EDT as well.
I ran it in eclipse and it didn't do anything. Just a few questions based on the above replies: From what I learned in my CS class, don't smaller data types such as bytes and shorts take up a bit less memory than integer? If so, is it alright if I keep using them or would it be a bit better if I just use Ints for now? As for the class names, how come having the word Create in the names is strange? I'm not sure what EDT stands for, could you explain it to me.  The program compiles and runs fine from the cmd line, maybe eclipse works differently or something? (I only use Notepad++ so I don't know too much about eclipse.) Thank's for all the feedback so-far! |
|
|
|
|
|
theagentd
|
|
«
Reply #18 - Posted
2012-12-14 22:03:26 » |
|
As just a field in a class ( or something), each variable takes at least 32 bytes, so there's no need to use anything smaller than an int for them. The only time they consume less space in when used in arrays, in which case a byte[1024] will be a 1/4th the size of an int[1024] (8 vs 32 bits) if we ignore the overhead of the array object. I strongly recommend using Eclipse. It's a lot faster than manually compiling from the command line. Your love / Mokyu |
Myomyomyo.
|
|
|
|
tyeeeee1
|
|
«
Reply #19 - Posted
2012-12-14 22:16:47 » |
|
As just a field in a class ( or something), each variable takes at least 32 bytes, so there's no need to use anything smaller than an int for them. The only time they consume less space in when used in arrays, in which case a byte[1024] will be a 1/4th the size of an int[1024] (8 vs 32 bits) if we ignore the overhead of the array object. I strongly recommend using Eclipse. It's a lot faster than manually compiling from the command line. Your love / Mokyu Ah, well that should make working with all of my numbers a lot easier. Thanks! ~Off to try Eclipse |
|
|
|
|
|
matheus23
|
|
«
Reply #20 - Posted
2012-12-14 22:19:20 » |
|
As just a field in a class ( or something), each variable takes at least 32 bytesReally? ... Really? Can you give me a source please? It's not that I put your skill into question  , but I just can't believe it... If it really is like that, why is that? I've heard about page aligning, but what exactly is it? And why is it needed? EDIT: I'm VERY sure my game once took much less memory (~100 mb), when I memory-optimized a class which had a to a (Dunno about the exact number of instances, but it is my worldOfCube game, so about 10-50 million instances...) |
|
|
|
|
sproingie
|
|
«
Reply #21 - Posted
2012-12-14 22:36:23 » |
|
I'm pretty sure he meant 32 bits. Anyway, Java packs fields aggressively (it sorts them by size, unlike C), but when you're passing a short/char/byte around as an arg or storing it as a local variable, they're going to take at least a word each. The main reason to use a smaller type like short or byte, other than to save a little space in the object, is when you know beforehand that the range of the value will fit and you want to statically guard against narrowing conversions. Most of the time it's just such a pain in the ass (especially byte which is signed, contrary to 99% of use cases) that most people just never bother and stick with int instead. This blog post has a pretty good rundown on the runtime memory layout of java objects. Keep in mind none of this is in the spec, it's implementation-dependent, and different JVMs may work differently, even different versions of the same JVM. |
|
|
|
|
|
theagentd
|
|
«
Reply #22 - Posted
2012-12-14 22:48:54 » |
|
Sorry, my source turned out to be from 2001... And yes, I meant bits, not bytes. Dammit!
|
Myomyomyo.
|
|
|
|
tyeeeee1
|
|
«
Reply #23 - Posted
2012-12-14 22:59:02 » |
|
I'm pretty sure he meant 32 bits. Anyway, Java packs fields aggressively (it sorts them by size, unlike C), but when you're passing a short/char/byte around as an arg or storing it as a local variable, they're going to take at least a word each. The main reason to use a smaller type like short or byte, other than to save a little space in the object, is when you know beforehand that the range of the value will fit and you want to statically guard against narrowing conversions. Most of the time it's just such a pain in the ass (especially byte which is signed, contrary to 99% of use cases) that most people just never bother and stick with int instead. This blog post has a pretty good rundown on the runtime memory layout of java objects. Keep in mind none of this is in the spec, it's implementation-dependent, and different JVMs may work differently, even different versions of the same JVM. Just so that I'm following along correctly... If I use a byte or a short it should only be when the number it's holding will stay within the limits of that data type; But I should just go ahead and use ints instead because it can be a pain to use bytes/shorts? |
|
|
|
|
|
