Page 1 of 2
Coding Guidelines
Posted: Fri Jan 08, 2010 6:27 pm
by RiZe
Hi folks. I would like to discuss about some coding guidelines because I think the source is often a mess. I know that many people are working on the project and it isn't so easy to "switch mode" from style to style but some coding guildelines would be great for the project. Let's see an example:
Code: Select all
public void removeFloodProtection(String ip){ if(!Config.FLOOD_PROTECTION) return; ForeignConnection fConnection = _floodProtection.get(ip); if(fConnection != null) { fConnection.connectionNumber -= 1; if (fConnection.connectionNumber == 0) { _floodProtection.remove(ip); } } else { _log.warning("Removing a flood protection for a GameServer that was not in the connection map??? :"+ip); }}
I have to say: "Wow O_O, what the hell ...". Yes I know the project is still growing and the old code is still here with some additions but look at the example again. It's ... what to say? A mess with capital M, E and both S. There are so many examples like this one but much worse.
I would recommend you to use some existing code guidelines or write your own but please. Avoid to commit code like the code I have posted as example. I know that reformating is slow process but sometimes clean up is done so ... try to look on the style too please.
All the best.
---
Maybe you find it usefull. I know that this is for C# but the code formatting works for Java too.
http://telemetron.googlecode.com/files/ ... elines.pdf
Re: Coding Guidelines
Posted: Fri Jan 08, 2010 6:35 pm
by JIV
whats exactly wrong with that code?
Re: Coding Guidelines
Posted: Fri Jan 08, 2010 7:10 pm
by RiZe
Maybe you will don't agree with me but this is what I don't like in examples
Conditions
Code: Select all
// In most cases there is an space between "if" and condition but not here, why?if(!Config.FLOOD_PROTECTION) return; // Ok, this is better formif (!Config.FLOOD_PROTECTION) return; // This is imo best but you don't have to agree with meif (!Config.FLOOD_PROTECTION){ return;}
Try, catch block
Code: Select all
// Hell, what is that? I saw that today but forget wheretry { ... } catch (Exception e) { } // Much better isn't it?try { ... } catch (Exception e) {}
Line breaking
Code: Select all
// Again, in most cases there is line break after condition but not herefor (L2ShortCut sc : allShortCuts) { if (sc.getId() == id && sc.getType() == L2ShortCut.TYPE_MACRO) _owner.deleteShortCut(sc.getSlot(), sc.getPage());} // Reformattedfor (L2ShortCut sc : allShortCuts){ if (sc.getId() == id && sc.getType() == L2ShortCut.TYPE_MACRO) { _owner.deleteShortCut(sc.getSlot(), sc.getPage()); }}
Just few lines lower...
Code: Select all
// Some line breaking would be great_revision++;L2Macro[] all = getAllMacroses();if (all.length == 0) { _owner.sendPacket(new SendMacroList(_revision, all.length, null));} else { for (L2Macro m : all) { _owner.sendPacket(new SendMacroList(_revision, all.length, m)); }} // Like this_revision++;L2Macro[] all = getAllMacroses(); if (all.length == 0) { _owner.sendPacket(new SendMacroList(_revision, all.length, null));} else { for (L2Macro m : all) { _owner.sendPacket(new SendMacroList(_revision, all.length, m)); }}
I know that this is an old code but sometimes it appears in the timeline (I mean not so old code) too. You are doing great work at all and I don't want to hurt you but think about it.
Re: Coding Guidelines
Posted: Fri Jan 08, 2010 7:40 pm
by Probe
try making huge switch cases and try inside try with such code format, tell me how soon you get lost
Re: Coding Guidelines
Posted: Fri Jan 08, 2010 7:46 pm
by RiZe
Then ask yourself. Is switch right choice in your case? I'm talking about some standards which are missing in some files or file parts.
Re: Coding Guidelines
Posted: Fri Jan 08, 2010 7:51 pm
by Probe
switch cases are much faster than 70 if clauses . performance is also of an issue here
Re: Coding Guidelines
Posted: Fri Jan 08, 2010 9:18 pm
by Deadmeat
Like you said there is a lot of people here posting fixes and the L2J team has to look over and test them as well as work on there own projects and they don't have the time to go over every line of code and correct it, so they or other people like you post clean up scripts when they have the free time.
Myself and I'm sure some others here never went to school for programing or took any classes in it and are just learning on the fly how to program by looking at code here and other websites on the Internet and spending hours to do something it would take you seconds to do, and I do try and kept the code clean but when it starts getting late the code starts to go it's own way and I'm sure if you were to look at some of the stuff I've done it would make you sick, but the way I look at it why add more bytes to my files with spaces, returns, and braces.
Re: Coding Guidelines
Posted: Sat Jan 09, 2010 5:40 am
by ZaKaX
Lol... you do realize you can reformat whole core in few seconds using Eclipse? Instead of making that space/lines break corrections manually like a newb
Right click on the project > Source > Format. w00t! You just saved 465465hours of useless shitty work. Much better, isn't it?
P.S: Making brackets when you only have one line after a condition is useless,
and ugly.
Re: Coding Guidelines
Posted: Sat Jan 09, 2010 12:49 pm
by Vapulabe
In C, you've R&K and Ansi style for braces... I think that R&K still has some point...
R&K :
Code: Select all
int func(void) { if (cond) { inst; } else { inst; } }
Ansi :
Code: Select all
int func(void){ if (cond) { inst; } else { inst; } }
First has the advantage of giving smaller (in number of line) code. This allow you to see more code in your editor window and make the reading easier
I'd say that in while/for/if/..., braces should ALWAYS be used even if there is only ONE line.
- no doubt about the range of code when there are lots of for/if/... imbricated
- when you need to add one line, no risk of forgetting to add the braces
- when you've multiple IF and an ELSE statement, make code much clearer
- code reformatting works better when braces are present
- braces also act as a syntax check (you need to have opening and closing braces in pair)
I'd add that some JAVADOC could be very useful...
Re: Coding Guidelines
Posted: Sat Jan 09, 2010 1:48 pm
by ZaKaX
I find "Ansi" easier to read...o0 the other one is ugly.
Re: Coding Guidelines
Posted: Sat Jan 09, 2010 3:16 pm
by janiii
from work and from java tutorials on web, i am more used to r&k than ansi. but because l2j is in ansi, i have to use here ansi althaught i hate it
too much lines for nothing.. ^^
Re: Coding Guidelines
Posted: Sat Jan 09, 2010 5:02 pm
by jurchiks
I would agree to ZakaX, ANSI looks much better, and about the primary topic - backets do make it much simpler to understand the code
Re: Coding Guidelines
Posted: Sat Jan 09, 2010 5:30 pm
by Probe
they do?
please scroll down the file L2AttackableAI.java
tell me how much of it you understand with your brackets
I agree that Ansi is more "readable" and nicer for the eye if the code isn't that complex, I use it myself for the simpler stuff.. but imo for the more complex files, such as AI's, and large objects (L2Character, L2PcInstance for eg.) R&K would be the preferable choice..
Re: Coding Guidelines
Posted: Sat Jan 09, 2010 5:49 pm
by jurchiks
meh, that's what you think, and everyone has his own opinion, the main idea here is to keep the code clean, not messy, easier to understand
Re: Coding Guidelines
Posted: Sat Jan 09, 2010 6:06 pm
by Probe
tell me if you think I'm not right after looking at that file