refactoring java code

Rohit Bajaj
Rohit Bajaj used Ask the Experts™
on
Hi,
Following is my main function :
 public static void main(String[] args) throws Exception {
        String env = System.getProperty("FLOCK_APPS_CONFIG");
        int port=0;
        try {
            InputStream in = new FileInputStream(env);
            Properties props = new Properties();
            props.load(in);
            PropertyConfigurator.configure(props);
            port = Integer.parseInt(props.getProperty("port"));
        }
        catch (Exception e)
        {
            logger.log(Level.SEVERE,"Error accessing config File : "+env,e);
            System.exit(1);
        }
        AppStart appStart = new AppStart(port);
        try {
            appStart.start();
            appStart.waitForInterrupt();
        } finally {
            appStart.stop();
        }
        System.exit(1);
    }

Open in new window


I want to know experts comment on this. If i can improve it in some way..
Some things that probably can be changed are :
1) I am assigning int port = 0
which actually does not make any sense as i want the port to be read from a file and if i cannot read it
applications stops.
2) there is one try catch block and one try finally

Is this code fine or there is some improvement scope in it ?

Thanks
Comment
Watch Question

Do more with

Expert Office
EXPERT OFFICE® is a registered trademark of EXPERTS EXCHANGE®
Top Expert 2016
Commented:
props.load(in);

Open in new window

That stream should be closed again

logger.log(Level.SEVERE,"Error accessing config File : "+env,e);

Open in new window

That's assumptive. Your general Exception could have been thrown for several reasons

You should log.debug the port you read

Why do you use exit code 1 finally?

If you structure your code and its exception handling you won't have to use that first System.exit as a kludgy GOTO

Author

Commented:
Hi,
reason for using Exit code 1 is, In one of the cases i got the  following exception and the application shutdown :

2015-11-26 13:33:30 ERROR main org.directi.code.dao.SnippetDaoImpl Error querying Database after Initialization :  
org.springframework.jdbc.CannotGetJdbcConnectionException: Could not get JDBC Connection; nested exception is org.apache.commons.dbcp.SQLNestedException: Cannot create PoolableConnectionFactory (Communications link failure

The last packet sent successfully to the server was 0 milliseconds ago. The driver has not received any packets from the server.)
	at org.springframework.jdbc.datasource.DataSourceUtils.getConnection(DataSourceUtils.java:80)
	at org.springframework.jdbc.core.JdbcTemplate.execute(JdbcTemplate.java:390)
	at org.springframework.jdbc.core.JdbcTemplate.query(JdbcTemplate.java:470)
	at org.springframework.jdbc.core.JdbcTemplate.query(JdbcTemplate.java:480)
	at org.springframework.jdbc.core.JdbcTemplate.queryForObject(JdbcTemplate.java:490)
	at org.springframework.jdbc.core.JdbcTemplate.queryForObject(JdbcTemplate.java:496)
	at org.directi.code.dao.SnippetDaoImpl.afterPropertiesSet(SnippetDaoImpl.java:55)
	at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.invokeInitMethods(AbstractAutowireCapableBeanFactory.java:1637)
	at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.initializeBean(AbstractAutowireCapableBeanFactory.java:1574)
	at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.doCreateBean(AbstractAutowireCapableBeanFactory.java:545)
	at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.createBean(AbstractAutowireCapableBeanFactory.java:482)
	at org.springframework.beans.factory.support.AbstractBeanFactory$1.getObject(AbstractBeanFactory.java:305)
	at org.springframework.beans.factory.support.DefaultSingletonBeanRegistry.getSingleton(DefaultSingletonBeanRegistry.java:230)
	at org.springframework.beans.factory.support.AbstractBeanFactory.doGetBean(AbstractBeanFactory.java:301)
	at org.springframework.beans.factory.support.AbstractBeanFactory.getBean(AbstractBeanFactory.java:196)
	at org.springframework.beans.factory.support.DefaultListableBeanFactory.findAutowireCandidates(DefaultListableBeanFactory.java:1145)
	at org.springframework.beans.factory.support.DefaultListableBeanFactory.doResolveDependency(DefaultListableBeanFactory.java:1069)
	at org.springframework.beans.factory.support.DefaultListableBeanFactory.resolveDependency(DefaultListableBeanFactory.java:967)
	at org.springframework.beans.factory.annotation.AutowiredAnnotationBeanPostProcessor$AutowiredFieldElement.inject(AutowiredAnnotationBeanPostProcessor.java:543)
	at org.springframework.beans.factory.annotation.InjectionMetadata.inject(InjectionMetadata.java:88)
	at org.springframework.beans.factory.annotation.AutowiredAnnotationBeanPostProcessor.postProcessPropertyValues(AutowiredAnnotationBeanPostProcessor.java:331)
	at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.populateBean(AbstractAutowireCapableBeanFactory.java:1214)
	at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.doCreateBean(AbstractAutowireCapableBeanFactory.java:543)
	at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.createBean(AbstractAutowireCapableBeanFactory.java:482)
	at org.springframework.beans.factory.support.AbstractBeanFactory$1.getObject(AbstractBeanFactory.java:305)
	at org.springframework.beans.factory.support.DefaultSingletonBeanRegistry.getSingleton(DefaultSingletonBeanRegistry.java:230)
	at org.springframework.beans.factory.support.AbstractBeanFactory.doGetBean(AbstractBeanFactory.java:301)
	at org.springframework.beans.factory.support.AbstractBeanFactory.getBean(AbstractBeanFactory.java:196)
	at org.springframework.beans.factory.support.DefaultListableBeanFactory.preInstantiateSingletons(DefaultListableBeanFactory.java:772)
	at org.springframework.context.support.AbstractApplicationContext.finishBeanFactoryInitialization(AbstractApplicationContext.java:834)
	at org.springframework.context.support.AbstractApplicationContext.refresh(AbstractApplicationContext.java:537)
	at org.springframework.web.servlet.FrameworkServlet.configureAndRefreshWebApplicationContext(FrameworkServlet.java:667)
	at org.springframework.web.servlet.FrameworkServlet.createWebApplicationContext(FrameworkServlet.java:633)
	at org.springframework.web.servlet.FrameworkServlet.createWebApplicationContext(FrameworkServlet.java:681)
	at org.springframework.web.servlet.FrameworkServlet.initWebApplicationContext(FrameworkServlet.java:552)
	at org.springframework.web.servlet.FrameworkServlet.initServletBean(FrameworkServlet.java:493)
	at org.springframework.web.servlet.HttpServletBean.init(HttpServletBean.java:136)
	at javax.servlet.GenericServlet.init(GenericServlet.java:244)
	at org.eclipse.jetty.servlet.ServletHolder.initServlet(ServletHolder.java:626)
	at org.eclipse.jetty.servlet.ServletHolder.initialize(ServletHolder.java:405)
	at org.eclipse.jetty.servlet.ServletHandler.initialize(ServletHandler.java:875)
	at org.eclipse.jetty.servlet.ServletContextHandler.startContext(ServletContextHandler.java:346)
	at org.eclipse.jetty.webapp.WebAppContext.startWebapp(WebAppContext.java:1380)
	at org.eclipse.jetty.webapp.WebAppContext.startContext(WebAppContext.java:1342)
	at org.eclipse.jetty.server.handler.ContextHandler.doStart(ContextHandler.java:772)
	at org.eclipse.jetty.servlet.ServletContextHandler.doStart(ServletContextHandler.java:259)
	at org.eclipse.jetty.webapp.WebAppContext.doStart(WebAppContext.java:518)
	at org.eclipse.jetty.util.component.AbstractLifeCycle.start(AbstractLifeCycle.java:68)
	at org.eclipse.jetty.util.component.ContainerLifeCycle.start(ContainerLifeCycle.java:132)
	at org.eclipse.jetty.server.Server.start(Server.java:405)
	at org.eclipse.jetty.util.component.ContainerLifeCycle.doStart(ContainerLifeCycle.java:106)
	at org.eclipse.jetty.server.handler.AbstractHandler.doStart(AbstractHandler.java:61)
	at org.eclipse.jetty.server.Server.doStart(Server.java:372)
	at org.eclipse.jetty.util.component.AbstractLifeCycle.start(AbstractLifeCycle.java:68)
	at org.directi.code.AppStart.start(AppStart.java:70)
	at org.directi.code.AppStart.main(AppStart.java:47)
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.lang.reflect.Method.invoke(Method.java:497)
	at com.intellij.rt.execution.application.AppMain.main(AppMain.java:140)
Caused by: org.apache.commons.dbcp.SQLNestedException: Cannot create PoolableConnectionFactory (Communications link failure

The last packet sent successfully to the server was 0 milliseconds ago. The driver has not received any packets from the server.)
	at org.apache.commons.dbcp.BasicDataSource.createPoolableConnectionFactory(BasicDataSource.java:1549)
	at org.apache.commons.dbcp.BasicDataSource.createDataSource(BasicDataSource.java:1388)
	at org.apache.commons.dbcp.BasicDataSource.getConnection(BasicDataSource.java:1044)
	at org.springframework.jdbc.datasource.DataSourceUtils.doGetConnection(DataSourceUtils.java:111)
	at org.springframework.jdbc.datasource.DataSourceUtils.getConnection(DataSourceUtils.java:77)
	... 60 more
Caused by: com.mysql.jdbc.exceptions.jdbc4.CommunicationsException: Communications link failure

The last packet sent successfully to the server was 0 milliseconds ago. The driver has not received any packets from the server.
	at sun.reflect.NativeConstructorAccessorImpl.newInstance0(Native Method)
	at sun.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:62)
	at sun.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:45)
	at java.lang.reflect.Constructor.newInstance(Constructor.java:422)
	at com.mysql.jdbc.Util.handleNewInstance(Util.java:400)
	at com.mysql.jdbc.SQLError.createCommunicationsException(SQLError.java:1038)
	at com.mysql.jdbc.MysqlIO.<init>(MysqlIO.java:339)
	at com.mysql.jdbc.ConnectionImpl.coreConnect(ConnectionImpl.java:2247)
	at com.mysql.jdbc.ConnectionImpl.connectOneTryOnly(ConnectionImpl.java:2280)
	at com.mysql.jdbc.ConnectionImpl.createNewIO(ConnectionImpl.java:2079)
	at com.mysql.jdbc.ConnectionImpl.<init>(ConnectionImpl.java:794)
	at com.mysql.jdbc.JDBC4Connection.<init>(JDBC4Connection.java:44)
	at sun.reflect.NativeConstructorAccessorImpl.newInstance0(Native Method)
	at sun.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:62)
	at sun.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:45)
	at java.lang.reflect.Constructor.newInstance(Constructor.java:422)
	at com.mysql.jdbc.Util.handleNewInstance(Util.java:400)
	at com.mysql.jdbc.ConnectionImpl.getInstance(ConnectionImpl.java:399)
	at com.mysql.jdbc.NonRegisteringDriver.connect(NonRegisteringDriver.java:325)
	at org.apache.commons.dbcp.DriverConnectionFactory.createConnection(DriverConnectionFactory.java:38)
	at org.apache.commons.dbcp.PoolableConnectionFactory.makeObject(PoolableConnectionFactory.java:582)
	at org.apache.commons.dbcp.BasicDataSource.validateConnectionFactory(BasicDataSource.java:1556)
	at org.apache.commons.dbcp.BasicDataSource.createPoolableConnectionFactory(BasicDataSource.java:1545)
	... 64 more
Caused by: java.net.ConnectException: Connection refused
	at java.net.PlainSocketImpl.socketConnect(Native Method)
	at java.net.AbstractPlainSocketImpl.doConnect(AbstractPlainSocketImpl.java:350)
	at java.net.AbstractPlainSocketImpl.connectToAddress(AbstractPlainSocketImpl.java:206)
	at java.net.AbstractPlainSocketImpl.connect(AbstractPlainSocketImpl.java:188)
	at java.net.SocksSocketImpl.connect(SocksSocketImpl.java:392)
	at java.net.Socket.connect(Socket.java:589)
	at com.mysql.jdbc.StandardSocketFactory.connect(StandardSocketFactory.java:214)
	at com.mysql.jdbc.MysqlIO.<init>(MysqlIO.java:298)
	... 80 more

Open in new window


And i got the following output in console of Intellij Idea :
Process finished with exit code 0

Which i think is incorrect as the application shutdown abruptly due to some database error. So i am returning exit code 1 as to signify that there was something wrong.

Please let me know your views on this.
Top Expert 2016

Commented:
But normal return of main will return 1 in your code. That's wrong
Why Diversity in Tech Matters

Kesha Williams, certified professional and software developer, explores the imbalance of diversity in the world of technology -- especially when it comes to hiring women. She showcases ways she's making a difference through the Colors of STEM program.

Author

Commented:
ALso you mentioned :
If you structure your code and its exception handling you won't have to use that first System.exit as a kludgy GOTO
You mean i can avoid the first System.exit in my code  ?
Please give some more pointers...
As there are two things going on here :
1) reading a config file -- which can break due to wrong parameters in it/ the file location itself is wrong
2) server startup embedded jetty --- this can go wrong due to wrong database configuration, port number busy etc..
Top Expert 2016
Commented:
The point is that you need to inspect each line of your code for possible errors, the exceptions than can be thrown and suitable ways of logging them and maybe dealing with them.

e.g if someone put in the value "80x" for key "port" in your code,  your log file would tell them that there was a problem accessing the config file. Wrong!
Top Expert 2016

Commented:
:)

Do more with

Expert Office
Submit tech questions to Ask the Experts™ at any time to receive solutions, advice, and new ideas from leading industry professionals.

Start 7-Day Free Trial