Skip to main content
Code Review

Return to Answer

replaced http://codereview.stackexchange.com/ with https://codereview.stackexchange.com/
Source Link

Here are a couple more things on top of the other excellent reviews by @ferada @ferada and @rolfl @rolfl. But you should also follow the above suggestions, as they will uncover even more.

Here are a couple more things on top of the other excellent reviews by @ferada and @rolfl. But you should also follow the above suggestions, as they will uncover even more.

Here are a couple more things on top of the other excellent reviews by @ferada and @rolfl. But you should also follow the above suggestions, as they will uncover even more.

added 824 characters in body
Source Link
janos
  • 112.9k
  • 15
  • 154
  • 396

Avoid unused stuff. Every single line of code in a program may potentially introduce a bug, and it inevitably adds to the complexity. Look for unused elements and remove them. Especially for a job interview, there's no place for speculation about potential future uses. Leave in what has purpose, leave out everything that doesn't.

For example in Main, you save args[0] in pluginFolder, but then you never use it. Similarly, you parse args[1], but you never use it.

See, that Main class gave you a lot more problems than it solved. It's kind of crazy, but honestly, this trivial implementation would have been a level of magnitude better:

public class Main {
 public static void main(String args[]) {
 MBeanContainer.startMBeanServer(9999);
 }
}

With so little code, this is a lot harder to pick on. This also effectively eliminates the possibility of another class referencing Main.blah. As a general rule: the less code you have, the better. (Less code meaning less logic, less unused stuff. Not "clever" syntax that hurts readability.)


Avoid unused stuff. Every single line of code in a program may potentially introduce a bug, and it inevitably adds to the complexity. Look for unused elements and remove them. Especially for a job interview, there's no place for speculation about potential future uses. Leave in what has purpose, leave out everything that doesn't.

For example in Main, you save args[0] in pluginFolder, but then you never use it. Similarly, you parse args[1], but you never use it.

See, that Main class gave you a lot more problems than it solved. It's kind of crazy, but honestly, this trivial implementation would have been a level of magnitude better:

public class Main {
 public static void main(String args[]) {
 MBeanContainer.startMBeanServer(9999);
 }
}

With so little code, this is a lot harder to pick on. This also effectively eliminates the possibility of another class referencing Main.blah. As a general rule: the less code you have, the better. (Less code meaning less logic, less unused stuff. Not "clever" syntax that hurts readability.)

Source Link
janos
  • 112.9k
  • 15
  • 154
  • 396

I suppose this was a homework assignment. I do this too when looking for talent. Instead of the extreme pressure of writing code on the spot at the interview, I give the opportunity to do your best in your private time, in your most comfortable environment. And I expect nothing less than a piece of beauty in return.

There are a couple things that you can do almost effortlessly that will make a big difference:

  • Apply standard formatting using the built in functions of your IDE. For example Control-Shift-f in Eclipse, Control-Alt-l in IntelliJ. Apply to all your files.
  • Eliminate all warnings your IDE gives you.
  • Eliminate even more issues: install the FindBugs plugin in Eclipse. In IntelliJ (I'm talking about the free community edition here) the default warnings are already pretty strict.
  • Add a couple of unit tests. And not just any unit tests, make them good.

Except for the unit tests, the above items are actually pretty mechanical: any monkey can do them. Yet, you haven't. This seriously crippled your chances from the start.

Here are a couple more things on top of the other excellent reviews by @ferada and @rolfl. But you should also follow the above suggestions, as they will uncover even more.


private static HashMap<ParamInterface, BaseParam> mbeansList = new HashMap<ParamInterface, BaseParam>();

Use interface types in variable and method parameter declarations whenever possible (most of the time). And, when submitting for a job application, choose a current Java version. Today, that should be at least Java 7, in which case your IDE would tell you to use the diamond operator when creating instances.

private static Map<ParamInterface, BaseParam> mbeansList = new HashMap<>();

Reduce the number of connections between your classes. For example, Main depends on MBeanContainer, but MBeanContainer also depends on Main (for the port setting). In this example Main should have passed all the necessary parameters to MBeanContainer. This is a particularly bad example, because a class like Main is effectively just a throw-away launcher: its sole purpose is to start up the "machinery" of a program / framework, and nothing should depend on it.

lang-java

AltStyle によって変換されたページ (->オリジナル) /