Java-Gaming.org Hi !
Featured games (83)
games approved by the League of Dukes
Games in Showcase (538)
Games in Android Showcase (132)
games submitted by our members
Games in WIP (601)
games currently in development
News: Read the Java Gaming Resources, or peek at the official Java tutorials
 
    Home     Help   Search   Login   Register   
Pages: [1]
  ignore  |  Print  
  Looking for Code Criticism  (Read 2681 times)
0 Members and 1 Guest are viewing this topic.
Offline 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/AUvX9Gw8

I'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!
Offline HeroesGraveDev

JGO Kernel


Medals: 309
Projects: 11
Exp: 3 years


┬─┬ノ(ಠ_ಠノ)(╯°□°)╯︵ ┻━┻


« 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
stats[]
and have constants called
STR = 0
and
DEX = 1
etc and put the strength variable in
stats[STR]
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.

Offline 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!
Legends of Yore - The Casual Retro Roguelike
Offline HeroesGraveDev

JGO Kernel


Medals: 309
Projects: 11
Exp: 3 years


┬─┬ノ(ಠ_ಠノ)(╯°□°)╯︵ ┻━┻


« 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
java -version
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  Wink) you should be calling
out.flush()
to make sure the data is written to the file.

Offline 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)
Offline 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.
Offline HeroesGraveDev

JGO Kernel


Medals: 309
Projects: 11
Exp: 3 years


┬─┬ノ(ಠ_ಠノ)(╯°□°)╯︵ ┻━┻


« 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);

Offline 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 //Change the return type (void) to something else later.
   {
      String line = "";
      String output = "";
     
      BufferedReader saveFile = new BufferedReader(new FileReader("TextSave.txt"));
     
      while((line = saveFile.readLine()) != null) //Note to Self: Re-read documentation on while loops.
      {
         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 ^.^
Offline sproingie

JGO Kernel


Medals: 202



« 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.

Foo x = new Foo(blah); // creates a new Foo that natter and gromish natter and gromish ...


Anyone who knows what a constructor does knows that statement creates a new Foo

Offline HeroesGraveDev

JGO Kernel


Medals: 309
Projects: 11
Exp: 3 years


┬─┬ノ(ಠ_ಠノ)(╯°□°)╯︵ ┻━┻


« 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 //Change the return type (void) to something else later.
   {
      String line;
      String output;
     
      BufferedReader saveFile = new BufferedReader(new FileReader("TextSave.txt"));
     
      while((line = saveFile.readLine()) != null) //Note to Self: Re-read documentation on while loops.
      {
         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.  Shocked
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. Tongue persecutioncomplex
If the flush is used, it works normally.

Sorry for the 'dirty' metaphor Wink

Games published by our own members! Check 'em out!
Legends of Yore - The Casual Retro Roguelike
Offline Nate

« JGO Bitwise Duke »


Medals: 158
Projects: 4
Exp: 14 years


Esoteric Software


« 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!).

Offline 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.  Shocked
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. Tongue persecutioncomplex
If the flush is used, it works normally.

Sorry for the 'dirty' metaphor Wink

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.  Cool

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.
Offline ctomni231

JGO Wizard


Medals: 99
Projects: 1
Exp: 7 years


Not a glitch. Just have a lil' pixelexia...


« 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.

Offline 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 Tongue

Offline loom_weaver

JGO Coder


Medals: 17



« 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.
Offline masteryoom

JGO Coder


Medals: 5
Projects: 2


If you look closely, you might see it turning...


« 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.  Roll Eyes

Smiley
Offline masteryoom

JGO Coder


Medals: 5
Projects: 2


If you look closely, you might see it turning...


« Reply #16 - Posted 2012-12-14 05:55:53 »

I ran it in eclipse and it didn't do anything. Emo

Smiley
Offline 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. Emo

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.  Undecided

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!
Offline theagentd

« JGO Bitwise Duke »


Medals: 365
Projects: 2
Exp: 8 years



« Reply #18 - Posted 2012-12-14 22:03:26 »

As just a field in a class (
private int x;
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.
Offline tyeeeee1
« Reply #19 - Posted 2012-12-14 22:16:47 »

As just a field in a class (
private int x;
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
Offline matheus23

JGO Kernel


Medals: 114
Projects: 3


You think about my Avatar right now!


« Reply #20 - Posted 2012-12-14 22:19:20 »

As just a field in a class (
private int x;
or something), each variable takes at least 32 bytes

Really?
...
Really?
Can you give me a source please? It's not that I put your skill into question Smiley , 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
private int flags;
to a
private byte flags;
(Dunno about the exact number of instances, but it is my worldOfCube game, so about 10-50 million instances...)

See my:
    My development Blog:     | Or look at my RPG | Or simply my coding
http://matheusdev.tumblr.comRuins of Revenge  |      On Github
Offline sproingie

JGO Kernel


Medals: 202



« 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.
Offline theagentd

« JGO Bitwise Duke »


Medals: 365
Projects: 2
Exp: 8 years



« 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.
Offline 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?
Pages: [1]
  ignore  |  Print  
 
 
You cannot reply to this message, because it is very, very old.

 

Add your game by posting it in the WIP section,
or publish it in Showcase.

The first screenshot will be displayed as a thumbnail.

rwatson462 (30 views)
2014-12-15 09:26:44

Mr.CodeIt (20 views)
2014-12-14 19:50:38

BurntPizza (42 views)
2014-12-09 22:41:13

BurntPizza (76 views)
2014-12-08 04:46:31

JscottyBieshaar (37 views)
2014-12-05 12:39:02

SHC (50 views)
2014-12-03 16:27:13

CopyableCougar4 (48 views)
2014-11-29 21:32:03

toopeicgaming1999 (114 views)
2014-11-26 15:22:04

toopeicgaming1999 (102 views)
2014-11-26 15:20:36

toopeicgaming1999 (30 views)
2014-11-26 15:20:08
Resources for WIP games
by kpars
2014-12-18 10:26:14

Understanding relations between setOrigin, setScale and setPosition in libGdx
by mbabuskov
2014-10-09 22:35:00

Definite guide to supporting multiple device resolutions on Android (2014)
by mbabuskov
2014-10-02 22:36:02

List of Learning Resources
by Longor1996
2014-08-16 10:40:00

List of Learning Resources
by SilverTiger
2014-08-05 19:33:27

Resources for WIP games
by CogWheelz
2014-08-01 16:20:17

Resources for WIP games
by CogWheelz
2014-08-01 16:19:50

List of Learning Resources
by SilverTiger
2014-07-31 16:29:50
java-gaming.org is not responsible for the content posted by its members, including references to external websites, and other references that may or may not have a relation with our primarily gaming and game production oriented community. inquiries and complaints can be sent via email to the info‑account of the company managing the website of java‑gaming.org
Powered by MySQL Powered by PHP Powered by SMF 1.1.18 | SMF © 2013, Simple Machines | Managed by Enhanced Four Valid XHTML 1.0! Valid CSS!