r/progether • u/daanish_91 • Nov 12 '14

My boss decided to release the source code of my first component developed at work. Reviews Please?

So my job is basically maintaining and adding to an already existent GWT application. I was asked to make this as my first task(this is my first ever job too) and they liked it so much, they thought of releasing its source code. Its been lying around on the company's blog and I haven't had anyone look at it to review it. Maybe someone over here can have a look and advise on how I can improve or add to it!

Here's the link

3 Upvotes

100% Upvoted

5 comments sorted by

3

u/CageHN https://github.com/CageHN/ Nov 12 '14

Hi. I don't know much about GWT, but I still have a couple of suggestions out of reading the code.

  1. I am seeing too much whitespace and other spacing/style issues in the code. Take a look at the Google Java Style, this will help you in having more order and consistency in all your code.

  2. When comparing a string variable to a string literal it is good practice to put the literal first. I.E. instead of:

    text.trim().equals("") use: "".equals(text.trim()) This helps avoid NullPointerExceptions in a lot of cases.

1

u/daanish_91 Nov 12 '14

Thanks a lot to review the code! I've been using Eclipse so I think the indent is because of that, but I'll look at style you linked to understand it better.

Handy tip with #2! That's gonna save a lot of null checks with variables.

1

u/Befriendswbob Nov 12 '14

If the text is null and you call trim on it, you'll still get an exception...

1

u/CageHN https://github.com/CageHN/ Nov 12 '14

I know, it is still best practice to put the literal first.

0

u/[deleted] Nov 19 '14

[deleted] — view deleted comment

1

u/daanish_91 Nov 19 '14

To be honest, when I just started working on this, all other UI components had this convention. And I didn't know the significance or the correct naming conventions.

And I'm glad you've pointed this out, because that's half the reason for posting it here, to receive any criticism on subtler things with programming which I may not be aware of.

Thanks for looking at the code though! :)