Monitoring params via JMX

A task for hiring on a new job (I failed it with reason "very bad code").

Write a program, which will be monitoring the params via JMX.
Param should have name, type and observabale value.

Why my code is bad? And how write this task more eleganty and clear?

main.java
Starts my app

public class Main {
    public static String pluginFolder;
    public static Integer port = 9999;
    public static void main( String args[] )
    {
        //directory with plugins
        if (args.length>0) {
            pluginFolder = args[0];
        } else {
            System.out.println("Please specify folder with plugins! Type path to plugins after space");
            return;
        }

        //port number
        if (args.length>1) {
            try {
                Integer.parseInt(args[1]);
            } catch (NumberFormatException nfe) {
                Utils.showMessage("Wrong port. Using default port: " + port);
            }
        }

        MBeanContainer.startMBeanServer();
    }
}

MBeanContainer.java

There is MBeanServer, where controls observable params. We can start, stop MBeanServer and pass or get params from it.

```
public class MBeanContainer
{
static MBeanServer mBeanServer;
private static HashMap mbeansList = new HashMap();

/**
* Start Mbean server
*/
public static void startMBeanServer() {
mBeanServer = ManagementFactory.getPlatformMBeanServer();
try {
LocateRegistry.createRegistry(Main.port);
JMXServiceURL url = new JMXServiceURL("service:jmx:rmi:///jndi/rmi://localhost:"+Main.port+"/server");
JMXConnectorServer cs = JMXConnectorServerFactory.newJMXConnectorServer(url, null, mBeanServer);
cs.start();
} catch (ExportException ee) { //hide trace exception and continue work when mbean server is already run.
if (ee.getCause().getClass() == BindException.class)
Utils.showMessage("Server is already started. \n " + ee.getCau

Stylistic

• (Always assuming this is not garbled by the pasting) there is

inconsistent indentation in some places, parameter lists sometimes
have spaces, sometimes they don't and the names aren't that great
(e.g. not for notification, o, cs are bad).

• The comments aren't very useful, except for the one in

MBeanContainer, //hide trace..., because it actually clarifies
the intent. I'd also either remove incomplete docstrings, or fill them
in. IMO if they don't add to the understanding of the code, they just
add clutter, but you could argue differently if you actually used the
docstrings in the IDE or docs generation.

• Catching Exception at every turn suggests that the error handling

isn't well structured. If you can't deal with the error, let it
propagate and don't just print a stack trace (or at least re-throw so
the caller can deal with it).

• In MBeanContainer the type check for BindException should probably

be changed to use instanceof. Also, what's with the alternative
case?

• Better use the base interface instead of specific classes when you

write out the types, e.g. Map. Again, I'm
pretty sure you don't rely on the hashing part. With one of the latest
changes in Java you can also omit a lot of the generic arguments.

• If you use final once, either do it always, or not at all. Same for

@Override.

• BaseParam copies the values from ParamInterface and stores the

object itself. Again, choose one or the other.

Structural

• getInterval returns an int, setInterval accepts long. The

parameter is named time in the interface, interval in the
implementation. Renaming to milliseconds would be good, showing off
JodaTime or a cron library would be great for an interview exercise
I guess, although for millisecond resolution it's overkill.

• MBeanContainer is all global and static. Changing that to regular

methods and maybe providing a singleton will make this way more
reusable.

• BaseParam::run does too much, I'd split the notification creation

into a method so the control flow is more obvious.

Okay, maybe others find more. Providing tests would also be something
nice to show off.

Even though this is an interview exercise, I think at least these points
would need to be addressed before considering using it in production and
later maintaining it.

Thread safety is probably okay, but is something to look out for. It
would be nice if stopMonitoring actually waited for threads to
shutdown.

For the design itself, well, without a bit more restrictions it's hard
to see what should be changed; apart from what I wrote it looks okay to
me. JMX probably isn't the best thing to exercise anyway.

StackExchange Code Review Q#67363, answer score: 6