Rate my code.

[Foodle]

I usually don't do this, but i want other programmers of this site to criticize my code and tell me where I could improve. I want no flame wars(just try to keep out of insulting as in ~ "omfg are you dumb? x should be y"), I only want to expand my KNOWLEDGE.

Anyway, hack and slash:

https://github.com/FoodleProgedy/StaffModeUtil

 

[LCastr0]

1. Indentation
   1. Your code doesn't seem to be correctly indented. Some of the lines are off to the left. (StaffUtility#22 for example)
2. You don't need a whole method to check if something is null. It may look cleaner, but it's just taking more time to execute the code
3. StaffItem seems to have methods that don't do anything?
4. Code style
   1. The style of your code doesn't seem to follow a "layout", for example, in some parts of the code you open the brackets for an if statement that has another if statement inside of it, and other parts you don't.
   2. You're leaving too many blank lines, and that makes me kinda confused, instead of keeping the code organized, it makes it look weird.

Anyway, hope the best for you.

 

[Jdsgames]

1.) Lacks comments, If you were to release this as Open Source a developer would have to go through all the code check it over to see what it does. Commenting makes it easier say if you haven't touched this code in a month to come back and quickly adapt into it.

2.) Indentation standards not met
public void temp(Temp temp){

}

^ Does not follow standards

public void temp(Temp temp)
{

}

^ Follows standards

I would also stay away from doing this:

^The if with no brackets surrounding an if.
It is mainly used for small minor things; counters, 1 variable changes, etc. not sections of code.

Standards for blank lines in code:
One blank line between different sections
(Like the example below)

// Input
<do input stuff>

// Calculations
<do calculation stuff>

// Output
<do output>[DOUBLEPOST=1497103161][/DOUBLEPOST]

You don't need a whole method to check if something is null. It may look cleaner, but it's just taking more time to execute the code

This may take longer to execute however, it is better than writing it a dozen times in the code.

 

[Foodle]

1.) Lacks comments, If you were to release this as Open Source a developer would have to go through all the code check it over to see what it does. Commenting makes it easier say if you haven't touched this code in a month to come back and quickly adapt into it.

2.) Indentation standards not met
public void temp(Temp temp){

}

^ Does not follow standards

public void temp(Temp temp)
{

}

^ Follows standards

I would also stay away from doing this:
View attachment 65261

^The if with no brackets surrounding an if.
It is mainly used for small minor things; counters, 1 variables changes, not sections of code.

Thank you for your responses!

I think that it's because i use tabs instead of spaces, indeed a VERY bad habit, but I can't seem to get it out of my head.
Also, for the "standards" part - I am using the google style, google it(ha i did that on purpose)
As for the comments, that completely skipped over my head as I don't normally publish projects in github,

Thank you so much!

Now, to answer LCastr0, you are very right indeed. My spacing is weird, I think this is also part of github's fault, but that's just my guess.

Also, can you show me the "you leave too many blank lines" in code? I'd like to know where so I could fix it !

 

[Jdsgames]

Thank you for your responses!

I think that it's because i use tabs instead of spaces, indeed a VERY bad habit, but I can't seem to get it out of my head.
Also, for the "standards" part - I am using the google style, google it(ha i did that on purpose)
As for the comments, that completely skipped over my head as I don't normally publish projects in github,

Thank you so much!

Now, to answer LCastr0, you are very right indeed. My spacing is weird, I think this is also part of github's fault, but that's just my guess.

Also, can you show me the "you leave too many blank lines" in code? I'd like to know where so I could fix it !

One example

 

[LCastr0]

Thank you for your responses!

I think that it's because i use tabs instead of spaces, indeed a VERY bad habit, but I can't seem to get it out of my head.
Also, for the "standards" part - I am using the google style, google it(ha i did that on purpose)
As for the comments, that completely skipped over my head as I don't normally publish projects in github,

Thank you so much!

Now, to answer LCastr0, you are very right indeed. My spacing is weird, I think this is also part of github's fault, but that's just my guess.

Also, can you show me the "you leave too many blank lines" in code? I'd like to know where so I could fix it !

Do you use any IDE? IDEs should fix your indentation.
Lines 20 and 24 of StaffUtility.java are blank, there is no need for that. And in StaffItemManager.java, lines 22 and 23 are blank.
Those are just examples, there are a lot more.

 

[Foodle]

Thanks guys! I will fix the spacing, these are all weird habits.


(but as i was saying, i would like some functionality issues, not just conventions)

also: forgot to mention, the multiple functions there are made so that they can be overriden when needed, for example, a random teleport item would only need an interact event.

 

[Jdsgames]

http://www.oracle.com/technetwork/java/javase/documentation/codeconventions-139411.html

^ Java code conventions from Oracle
Although, they do keep the {
}

Thing I have seen that changed in most business and ignored mainly for organizational purpose

 

[LCastr0]

Thanks guys! I will fix the spacing, these are all weird habits.


(but as i was saying, i would like some functionality issues, not just conventions)

The blank methods in StaffItem. They don't do anything, yet they are there. Unless you removed that before sending to GitHub.

 

[Foodle]

The blank methods in StaffItem. They don't do anything, yet they are there. Unless you removed that before sending to GitHub.

Read the edited comment.

 

[LCastr0]

http://www.oracle.com/technetwork/java/javase/documentation/codeconventions-139411.html

^ Java code conventions from Oracle
Although, they do keep the {
}

Thing I have seen that changed in most business and ignored mainly for organizational purpose

A lot of companies still use the Google style, that really depends on the company/person, and I don't think that affects anything, as long as you keep all the code in one style. If you use two styles in the same code, then it's obviously bad.

 

[Jdsgames]

Thanks guys! I will fix the spacing, these are all weird habits.

(but as i was saying, i would like some functionality issues, not just conventions)

I do not see functionality problems again you would have to actually
TEST it out, but nothing that sticks out to me.

 

[Foodle]

I do not see functionality problems

Just wondering, I know that I have a bunch of convention problems(and that's why I made this thread) but I also want to know some cases where performance can be hindered due to something I have done.

 

[LCastr0]

Read the edited comment.

If they are in an abstract class and they don't have any body, then they should be declared as abstract methods

 

[Foodle]

If they are in an abstract class and they don't have any body, then they should be declared as abstract methods

I don't want to have empty methods in a class that extends StaffItem, that looks bad. Better sacrifice one class than sacrifice three.

 

[Jdsgames]

Just wondering, I know that I have a bunch of convention problems(and that's why I made this thread) but I also want to know some cases where performance can be hindered due to something I have done.

Eh, its normally a fight between performance and code organization
Like your null check it is nicer to read and go over, but as LCastr0 mentioned it does slow it down that fraction of a fraction of a second.

 

[LCastr0]

I don't want to have empty methods in a class that extends StaffItem, that looks bad. Better sacrifice one class than sacrifice three.

The StaffItem class doesn't seem to be extended in other classes, so why would you need empty methods in other classes?

Edit: Just saw the example. The thing is, it's better to have them to be abstract. That is one of the points of abstraction. If another developer needs to work with your code, and the methods are not abstract, the dev might not know that they have to implement that part of the code.

 

[Foodle]

Eh, its normally a fight between performance and code organization
Like your null check it is nicer to read and go over, but as LCastr0 mentioned it does slow it down that fraction of a fraction of a second.

It's more of a shortcut, and as you said it does make the process cleaner and faster. It is true it slows it down, duh, but that fraction of a fraction of a fraction of a fraction of a fraction of a fraction of a fraction of a second, but that won't have any repercussions in the end.


Check the examples package, LCastr0.

 

[Jdsgames]

The StaffItem class doesn't seem to be extended in other classes, so why would you need empty methods in other classes?

Edit: Just saw the example. The thing is, it's better to have them to be abstract. That is one of the points of abstraction. If another developer needs to work with your code, and the methods are not abstract, the dev might not know that they have to implement that part of the code.

Example: RandomTeleport.java extends it

 

[LCastr0]

Example: RandomTeleport.java extends it

Yeah I edited the post