• Post Reply Bookmark Topic Watch Topic
  • New Topic
programming forums Java Mobile Certification Databases Caching Books Engineering Micro Controllers OS Languages Paradigms IDEs Build Tools Frameworks Application Servers Open Source This Site Careers Other Pie Elite all forums
this forum made possible by our volunteer staff, including ...
Marshals:
  • Campbell Ritchie
  • Jeanne Boyarsky
  • Ron McLeod
  • Paul Clapham
  • Liutauras Vilda
Sheriffs:
  • paul wheaton
  • Rob Spoor
  • Devaka Cooray
Saloon Keepers:
  • Stephan van Hulst
  • Tim Holloway
  • Carey Brown
  • Frits Walraven
  • Tim Moores
Bartenders:
  • Mikalai Zaikin

Rate my code

 
Greenhorn
Posts: 4
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
How would you rate my code?
Pastebin

Thanks!
 
Marshal
Posts: 79153
377
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Welcome to the Ranch

Please post the code here with code tags because mny people are reluctant to follow links to find code.
 
Alexandro Rossi
Greenhorn
Posts: 4
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
It's pastebin, almost every coder uses it
 
Bartender
Posts: 10780
71
Hibernate Eclipse IDE Ubuntu
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

Pjotr Inno wrote:How would you rate my code?


Well, it's nicely formatted, and you appear to be using proper Java naming conventions; so well done on that score.

However:
1. I'd suggest adding Javadoc headers for all your public methods.

2. This is a personal preference, but I'd use switch statements rather than those huge if...else blocks. These days you can use them for Strings as well as numeric values.
Either way, having lots of them suggests that you're doing a lot of "dispatching", so it might be worth considering subtyping/polymorphism, or possibly enums, to hide some of that logic, or make it more readable.

3. Your handleBanCommands() method looks very "monolithic", so I'd try to break it up a bit ... actually, a LOT.  
My general rule of thumb is that methods should rarely be longer than a "viewable page", or about 20 lines of code - and 10 is even better.

HIH

Winston
 
Saloon Keeper
Posts: 15490
363
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

Pjotr Inno wrote:It's pastebin, almost every coder uses it


I really don't think so.
 
Stephan van Hulst
Saloon Keeper
Posts: 15490
363
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
You have a lot of hard-coded text. I would move these to resource bundles.
 
Sheriff
Posts: 17644
300
Mac Android IntelliJ IDE Eclipse IDE Spring Debian Java Ubuntu Linux
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Methods are too long and incoherent. One in particular, handleBanCommands(), is horrific and obscenely long. In some places, writing code like this can actually be cause for termination. Honestly.

BTW, I agree with Stephan about PasteBin. A lot of programmers I know don't use it, myself included.
 
author
Posts: 23951
142
jQuery Eclipse IDE Firefox Browser VI Editor C++ Chrome Java Linux Windows
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

Stephan van Hulst wrote:

Pjotr Inno wrote:It's pastebin, almost every coder uses it


I really don't think so.



Agree with Stephan. Most developers use a source code control system as a repository for code. And even then, it runs the gamut. The reason for that is source code control is not a developer's decision, but a corporate decision. The repository is used by everyone on the team, perhaps, even across teams (company wide), and in many cases, integrated with the build system and the bug tracking system.

Henry
 
Alexandro Rossi
Greenhorn
Posts: 4
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Thank you so much for the comments!

And to the pastebin comment: Yes, I use GitHub myself.
I just mean to share quick snippets, pastebin is widely used and there's nothing "unsafe" about the site that's what I meant
 
Alexandro Rossi
Greenhorn
Posts: 4
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

Winston Gutkowski wrote:

Pjotr Inno wrote:How would you rate my code?


Well, it's nicely formatted, and you appear to be using proper Java naming conventions; so well done on that score.

However:
1. I'd suggest adding Javadoc headers for all your public methods.

2. This is a personal preference, but I'd use switch statements rather than those huge if...else blocks. These days you can use them for Strings as well as numeric values.
Either way, having lots of them suggests that you're doing a lot of "dispatching", so it might be worth considering subtyping/polymorphism, or possibly enums, to hide some of that logic, or make it more readable.

3. Your handleBanCommands() method looks very "monolithic", so I'd try to break it up a bit ... actually, a LOT.  
My general rule of thumb is that methods should rarely be longer than a "viewable page", or about 20 lines of code - and 10 is even better.

HIH

Winston


Thanks
I'll definitely improve the handleBanCommands method, since it's massive
 
lowercase baba
Posts: 13089
67
Chrome Java Linux
  • Likes 2
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

Pjotr Inno wrote:It's pastebin, almost every coder uses it


I always figure, if I'm asking someone else for a favor, it's my job to make it as easy as possible for them to do so. And which is easier:

Asking someone to read something on the screen they are already looking at and reply right there

- or -

Asking them to go somewhere else, find whatever it is you are talking about, maybe cut-n-paste bits and move that back to the original website...

just something to consider.
 
Winston Gutkowski
Bartender
Posts: 10780
71
Hibernate Eclipse IDE Ubuntu
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

Pjotr Inno wrote:I'll definitely improve the handleBanCommands method, since it's massive


One of the chaps around here is fond of saying that good code should tell a story; and I completely agree with him. But to do that, you have to think about what your code is doing, rather than just how it's going to do it.

For example, if I was writing a chess program, I might start with a method like this:The above is purely for illustration and there are probably many better ways to do it; but I'd say that it describes a game of chess pretty well.

And most importantly, I don't have to know anything about how those methods work (and they could be very complex indeed), or exactly what a Board or Player or Result object looks like; I'm simply describing the process of a game of chess in "Java-ese".

I hope you can also see that choosing good, descriptive names for your methods and variables can really help other people to understand your code; but keeping methods short is probably the most important thing you can do to aid readability.

HIH

Winston
 
Sheriff
Posts: 5555
326
IntelliJ IDE Python Java Linux
  • Likes 1
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

Pjotr Inno wrote:It's pastebin, almost every coder uses it


As a purely practical consideration, I am unable to participate in this discussion because pastebin is not accessible from my office network. Therefore I cannot see your code at all.

Also, think about the long term usefulness of this topic. There has been, and may continue to be, lots of really good advice about general programming techniques and syntax specific preferences. But, where is the context? At best the future reader has to navigate off to some other site to see your code which is an inconvenience. At worst, the code being discussed is gone forever because you decided to delete it from pastebin, or maybe even pastebin closed down, in which case this topic is now useless.

My recommendation for posting questions about code on CodeRanch is to always post the code you want to discuss directly on CodeRanch. You preserve the usefulness of the thread for future generations, and you make it much easier for the reader to engage with your question and thus are more likely receive the help you seek.
 
Don't get me started about those stupid light bulbs.
reply
    Bookmark Topic Watch Topic
  • New Topic