Refactoring to Strategy. My example. Part 1
I am writing about refactoring again. I found a class that has a code which is not so clean as I want. I researched it and understood that I could make it better. It’s a good chance to demonstrate how to refactor code using pattern Strategy. Let’s go.
We have following code:
public class ConnectionManager {
private static Logger logger = Logger.getLogger(ConnectionManager.class);
public static final String CONTEXT_PROPERTIES_DEFAULT = "context";
public static final String COMMON_PROPERTIES_DEFAULT = "common";
private static final String DS_JNDI_NAME = "connection.jndi_name";
private DataSource dataSource = null;
private Context context = null;
private boolean isDataSourceUsed = false;
private boolean isPropertyInit = false;
private String url = null;
private String password = null;
private String login = null;
private String commonProperties;
private String contextProperties;
private boolean autoChoose;
private static final String CONNECTION_AUTO_CHOOSE = "connection.auto_choose";
private static final String CONNECTION_USE_DATASOURCE = "connection.use_datasource";
private static final String CONNECTION_DRIVER_CLASS_NAME = "connection.driver_class_name";
private static final String CONNECTION_DATABASE_URL = "connection.database_url";
private static final String CONNECTION_DATABASE_USER = "connection.database_user";
private static final String CONNECTION_DATABASE_PASSWORD = "connection.database_password";
public ConnectionManager() {
this(COMMON_PROPERTIES_DEFAULT, CONTEXT_PROPERTIES_DEFAULT);
}
public ConnectionManager(String commonProperties, String contextProperties) {
this.commonProperties = commonProperties;
this.contextProperties = contextProperties;
}
private DataSource createDataSource() throws NamingException {
ResourceBundle contextRes = ResourceBundle.getBundle(commonProperties);
String strJNDI = contextRes.getString(DS_JNDI_NAME);
if (context == null) {
context = getRootContext();
}
dataSource = (DataSource)context.lookup(strJNDI);
return dataSource;
}
private Context getRootContext() throws NamingException {
// create property for initial context
Properties properties = new Properties();
//read property from resurce bundle
ResourceBundle contextRes = ResourceBundle.getBundle(contextProperties);
Enumeration keys = contextRes.getKeys();
if (keys != null) {
while(keys.hasMoreElements()) {
String key = (String)keys.nextElement();
String value = contextRes.getString(key);
// add property
properties.put(key, value);
}
}
context = new InitialContext(properties);
return context;
}
private DataSource getDataSource() throws NamingException {
if (dataSource == null) {
createDataSource();
}
return dataSource;
}
public Connection getConnection() throws Exception {
Connection conn = null;
try {
if (!isPropertyInit) {
ResourceBundle contextRes = ResourceBundle.getBundle(commonProperties);
autoChoose = new Boolean(contextRes.getString(CONNECTION_AUTO_CHOOSE)).booleanValue();
isDataSourceUsed = new Boolean(contextRes.getString(CONNECTION_USE_DATASOURCE)).booleanValue();
try {
Class.forName(contextRes.getString(CONNECTION_DRIVER_CLASS_NAME));
url = contextRes.getString(CONNECTION_DATABASE_URL);
login = contextRes.getString(CONNECTION_DATABASE_USER);
password = contextRes.getString(CONNECTION_DATABASE_PASSWORD);
} catch (Exception ex) {
logger.error("exception while getting datasource", ex);
}
}
isPropertyInit = true;
if (isDataSourceUsed) {
try {
conn = getDataSource().getConnection();
} catch (Exception ex) {
logger.error("while getting datasource", ex);
}
if (autoChoose && conn == null) {
logger.info("Can't find a datasource. Using direct connection");
conn = getConnectionFromURL();
}
} else {
conn = getConnectionFromURL();
}
} catch (Exception ex) {
logger.error(ex, ex);
throw ex;
}
return conn;
}
private Connection getConnectionFromURL() throws SQLException {
Connection conn = DriverManager.getConnection(url, login, password);
return conn;
}
}
private static Logger logger = Logger.getLogger(ConnectionManager.class);
public static final String CONTEXT_PROPERTIES_DEFAULT = "context";
public static final String COMMON_PROPERTIES_DEFAULT = "common";
private static final String DS_JNDI_NAME = "connection.jndi_name";
private DataSource dataSource = null;
private Context context = null;
private boolean isDataSourceUsed = false;
private boolean isPropertyInit = false;
private String url = null;
private String password = null;
private String login = null;
private String commonProperties;
private String contextProperties;
private boolean autoChoose;
private static final String CONNECTION_AUTO_CHOOSE = "connection.auto_choose";
private static final String CONNECTION_USE_DATASOURCE = "connection.use_datasource";
private static final String CONNECTION_DRIVER_CLASS_NAME = "connection.driver_class_name";
private static final String CONNECTION_DATABASE_URL = "connection.database_url";
private static final String CONNECTION_DATABASE_USER = "connection.database_user";
private static final String CONNECTION_DATABASE_PASSWORD = "connection.database_password";
public ConnectionManager() {
this(COMMON_PROPERTIES_DEFAULT, CONTEXT_PROPERTIES_DEFAULT);
}
public ConnectionManager(String commonProperties, String contextProperties) {
this.commonProperties = commonProperties;
this.contextProperties = contextProperties;
}
private DataSource createDataSource() throws NamingException {
ResourceBundle contextRes = ResourceBundle.getBundle(commonProperties);
String strJNDI = contextRes.getString(DS_JNDI_NAME);
if (context == null) {
context = getRootContext();
}
dataSource = (DataSource)context.lookup(strJNDI);
return dataSource;
}
private Context getRootContext() throws NamingException {
// create property for initial context
Properties properties = new Properties();
//read property from resurce bundle
ResourceBundle contextRes = ResourceBundle.getBundle(contextProperties);
Enumeration keys = contextRes.getKeys();
if (keys != null) {
while(keys.hasMoreElements()) {
String key = (String)keys.nextElement();
String value = contextRes.getString(key);
// add property
properties.put(key, value);
}
}
context = new InitialContext(properties);
return context;
}
private DataSource getDataSource() throws NamingException {
if (dataSource == null) {
createDataSource();
}
return dataSource;
}
public Connection getConnection() throws Exception {
Connection conn = null;
try {
if (!isPropertyInit) {
ResourceBundle contextRes = ResourceBundle.getBundle(commonProperties);
autoChoose = new Boolean(contextRes.getString(CONNECTION_AUTO_CHOOSE)).booleanValue();
isDataSourceUsed = new Boolean(contextRes.getString(CONNECTION_USE_DATASOURCE)).booleanValue();
try {
Class.forName(contextRes.getString(CONNECTION_DRIVER_CLASS_NAME));
url = contextRes.getString(CONNECTION_DATABASE_URL);
login = contextRes.getString(CONNECTION_DATABASE_USER);
password = contextRes.getString(CONNECTION_DATABASE_PASSWORD);
} catch (Exception ex) {
logger.error("exception while getting datasource", ex);
}
}
isPropertyInit = true;
if (isDataSourceUsed) {
try {
conn = getDataSource().getConnection();
} catch (Exception ex) {
logger.error("while getting datasource", ex);
}
if (autoChoose && conn == null) {
logger.info("Can't find a datasource. Using direct connection");
conn = getConnectionFromURL();
}
} else {
conn = getConnectionFromURL();
}
} catch (Exception ex) {
logger.error(ex, ex);
throw ex;
}
return conn;
}
private Connection getConnectionFromURL() throws SQLException {
Connection conn = DriverManager.getConnection(url, login, password);
return conn;
}
}
Where are the problems? First of all, method getConnection has to many conditions and contains initialization logic. In this article I will prepare a code for extracting different strategies.
Step 1. Remove duplications.The first duplication is:
File with properties has been loaded twice (getConnection, createDataSource), and contextRes variable has been used for different purposes. It’s wrong. Variable for commonProperties should be renamed to commonRes. I will move resources to class fields and load them in the constructor only once. So,
public class ConnectionManager{
//....
private ResourceBundle commonRes;
private ResourceBundle contextRes;
public ConnectionManager() {
this(COMMON_PROPERTIES_DEFAULT, CONTEXT_PROPERTIES_DEFAULT);
}
public ConnectionManager(String commonProperties, String contextProperties) {
this.commonProperties = commonProperties;
this.contextProperties = contextProperties;
this.commonRes = ResourceBundle.getBundle(commonProperties);
this.contextRes = ResourceBundle.getBundle(contextProperties);
}
//...
}
//....
private ResourceBundle commonRes;
private ResourceBundle contextRes;
public ConnectionManager() {
this(COMMON_PROPERTIES_DEFAULT, CONTEXT_PROPERTIES_DEFAULT);
}
public ConnectionManager(String commonProperties, String contextProperties) {
this.commonProperties = commonProperties;
this.contextProperties = contextProperties;
this.commonRes = ResourceBundle.getBundle(commonProperties);
this.contextRes = ResourceBundle.getBundle(contextProperties);
}
//...
}
Step 2. Change signature of createDataSource. Returned value is not using anywhere. I will change method’s name:
private void initDataSource() throws NamingException {
String strJNDI = commonRes.getString(DS_JNDI_NAME);
if (context == null) {
context = getRootContext();
}
dataSource = (DataSource) context.lookup(strJNDI);
}
String strJNDI = commonRes.getString(DS_JNDI_NAME);
if (context == null) {
context = getRootContext();
}
dataSource = (DataSource) context.lookup(strJNDI);
}
Step 3. Let’s review method getConnection. I think it’s a good idea to use ‘extract method’ refactoring template in this case:
try {
Class.forName(commonRes.getString(CONNECTION_DRIVER_CLASS_NAME));
url = commonRes.getString(CONNECTION_DATABASE_URL);
login = commonRes.getString(CONNECTION_DATABASE_USER);
password = commonRes.getString(CONNECTION_DATABASE_PASSWORD);
} catch (Exception ex) {
logger.error("exception while getting datasource", ex);
}
Class.forName(commonRes.getString(CONNECTION_DRIVER_CLASS_NAME));
url = commonRes.getString(CONNECTION_DATABASE_URL);
login = commonRes.getString(CONNECTION_DATABASE_USER);
password = commonRes.getString(CONNECTION_DATABASE_PASSWORD);
} catch (Exception ex) {
logger.error("exception while getting datasource", ex);
}
initDirectConnectionParams will be a good name for this method.
Step 4. Variable isPropertyInit is redundancy and I want to remove it. Following lines will be moved to constructor:
Step 5. I’m using extract method again for getDataSource().getConnection(). New method will be getDataSourceConnection.
Step 6. I want to remove getDataSource method. I will move initDataSource() to the constructor and replace getDataSource with Inline Method.
public ConnectionManager(String commonProperties, String contextProperties) {
this.commonProperties = commonProperties;
this.contextProperties = contextProperties;
this.commonRes = ResourceBundle.getBundle(commonProperties);
this.contextRes = ResourceBundle.getBundle(contextProperties);
autoChoose = new Boolean(commonRes.getString(CONNECTION_AUTO_CHOOSE)).booleanValue();
isDataSourceUsed = new Boolean(commonRes.getString(CONNECTION_USE_DATASOURCE)).booleanValue();
initDirectConnectionParams();
if (isDataSourceUsed){
try {
initDataSource();
} catch (NamingException e) {
throw new RuntimeException(e);
}
}
}
private Connection getDataSourceConnection() throws SQLException, NamingException{
return dataSource.getConnection();
}
this.commonProperties = commonProperties;
this.contextProperties = contextProperties;
this.commonRes = ResourceBundle.getBundle(commonProperties);
this.contextRes = ResourceBundle.getBundle(contextProperties);
autoChoose = new Boolean(commonRes.getString(CONNECTION_AUTO_CHOOSE)).booleanValue();
isDataSourceUsed = new Boolean(commonRes.getString(CONNECTION_USE_DATASOURCE)).booleanValue();
initDirectConnectionParams();
if (isDataSourceUsed){
try {
initDataSource();
} catch (NamingException e) {
throw new RuntimeException(e);
}
}
}
private Connection getDataSourceConnection() throws SQLException, NamingException{
return dataSource.getConnection();
}
So, what do we have now?
public class ConnectionManager {
private static Logger logger = Logger.getLogger(ConnectionManager.class);
public static final String CONTEXT_PROPERTIES_DEFAULT = "context";
public static final String COMMON_PROPERTIES_DEFAULT = "common";
private static final String DS_JNDI_NAME = "connection.jndi_name";
private DataSource dataSource = null;
private Context context = null;
private boolean isDataSourceUsed = false;
private String url = null;
private String password = null;
private String login = null;
private String commonProperties;
private String contextProperties;
private boolean autoChoose;
private static final String CONNECTION_AUTO_CHOOSE = "connection.auto_choose";
private static final String CONNECTION_USE_DATASOURCE = "connection.use_datasource";
private static final String CONNECTION_DRIVER_CLASS_NAME = "connection.driver_class_name";
private static final String CONNECTION_DATABASE_URL = "connection.database_url";
private static final String CONNECTION_DATABASE_USER = "connection.database_user";
private static final String CONNECTION_DATABASE_PASSWORD = "connection.database_password";
private ResourceBundle commonRes;
private ResourceBundle contextRes;
public ConnectionManager() {
this(COMMON_PROPERTIES_DEFAULT, CONTEXT_PROPERTIES_DEFAULT);
}
public ConnectionManager(String commonProperties, String contextProperties) {
this.commonProperties = commonProperties;
this.contextProperties = contextProperties;
this.commonRes = ResourceBundle.getBundle(commonProperties);
this.contextRes = ResourceBundle.getBundle(contextProperties);
autoChoose = new Boolean(commonRes.getString(CONNECTION_AUTO_CHOOSE)).booleanValue();
isDataSourceUsed = new Boolean(commonRes.getString(CONNECTION_USE_DATASOURCE)).booleanValue();
initDirectConnectionParams();
if (isDataSourceUsed) {
try {
initDataSource();
} catch (NamingException e) {
throw new RuntimeException(e);
}
}
}
private void initDataSource() throws NamingException {
// get JNDI name from properies
String strJNDI = commonRes.getString(DS_JNDI_NAME);
if (context == null) {
context = getRootContext();
}
dataSource = (DataSource) context.lookup(strJNDI);
}
private Context getRootContext() throws NamingException {
// create property for initial context
Properties properties = new Properties();
Enumeration keys = contextRes.getKeys();
if (keys != null) {
while (keys.hasMoreElements()) {
String key = (String) keys.nextElement();
String value = contextRes.getString(key);
// add property
properties.put(key, value);
}
}
context = new InitialContext(properties);
return context;
}
public Connection getConnection() throws Exception {
Connection conn = null;
try {
if (isDataSourceUsed) {
try {
conn = getDataSourceConnection();
} catch (Exception ex) {
logger.error("while getting datasource", ex);
}
if (autoChoose && conn == null) {
logger.info("Can't find a datasource. Using direct connection");
conn = getConnectionFromURL();
}
} else {
conn = getConnectionFromURL();
}
} catch (Exception ex) {
logger.error(ex, ex);
throw ex;
}
return conn;
}
private Connection getDataSourceConnection() throws SQLException, NamingException {
return dataSource.getConnection();
}
private void initDirectConnectionParams() {
try {
Class.forName(commonRes.getString(CONNECTION_DRIVER_CLASS_NAME));
url = commonRes.getString(CONNECTION_DATABASE_URL);
login = commonRes.getString(CONNECTION_DATABASE_USER);
password = commonRes.getString(CONNECTION_DATABASE_PASSWORD);
} catch (Exception ex) {
logger.error("exception while getting datasource", ex);
}
}
private Connection getConnectionFromURL() throws SQLException {
Connection conn = DriverManager.getConnection(url, login, password);
return conn;
}
}
private static Logger logger = Logger.getLogger(ConnectionManager.class);
public static final String CONTEXT_PROPERTIES_DEFAULT = "context";
public static final String COMMON_PROPERTIES_DEFAULT = "common";
private static final String DS_JNDI_NAME = "connection.jndi_name";
private DataSource dataSource = null;
private Context context = null;
private boolean isDataSourceUsed = false;
private String url = null;
private String password = null;
private String login = null;
private String commonProperties;
private String contextProperties;
private boolean autoChoose;
private static final String CONNECTION_AUTO_CHOOSE = "connection.auto_choose";
private static final String CONNECTION_USE_DATASOURCE = "connection.use_datasource";
private static final String CONNECTION_DRIVER_CLASS_NAME = "connection.driver_class_name";
private static final String CONNECTION_DATABASE_URL = "connection.database_url";
private static final String CONNECTION_DATABASE_USER = "connection.database_user";
private static final String CONNECTION_DATABASE_PASSWORD = "connection.database_password";
private ResourceBundle commonRes;
private ResourceBundle contextRes;
public ConnectionManager() {
this(COMMON_PROPERTIES_DEFAULT, CONTEXT_PROPERTIES_DEFAULT);
}
public ConnectionManager(String commonProperties, String contextProperties) {
this.commonProperties = commonProperties;
this.contextProperties = contextProperties;
this.commonRes = ResourceBundle.getBundle(commonProperties);
this.contextRes = ResourceBundle.getBundle(contextProperties);
autoChoose = new Boolean(commonRes.getString(CONNECTION_AUTO_CHOOSE)).booleanValue();
isDataSourceUsed = new Boolean(commonRes.getString(CONNECTION_USE_DATASOURCE)).booleanValue();
initDirectConnectionParams();
if (isDataSourceUsed) {
try {
initDataSource();
} catch (NamingException e) {
throw new RuntimeException(e);
}
}
}
private void initDataSource() throws NamingException {
// get JNDI name from properies
String strJNDI = commonRes.getString(DS_JNDI_NAME);
if (context == null) {
context = getRootContext();
}
dataSource = (DataSource) context.lookup(strJNDI);
}
private Context getRootContext() throws NamingException {
// create property for initial context
Properties properties = new Properties();
Enumeration keys = contextRes.getKeys();
if (keys != null) {
while (keys.hasMoreElements()) {
String key = (String) keys.nextElement();
String value = contextRes.getString(key);
// add property
properties.put(key, value);
}
}
context = new InitialContext(properties);
return context;
}
public Connection getConnection() throws Exception {
Connection conn = null;
try {
if (isDataSourceUsed) {
try {
conn = getDataSourceConnection();
} catch (Exception ex) {
logger.error("while getting datasource", ex);
}
if (autoChoose && conn == null) {
logger.info("Can't find a datasource. Using direct connection");
conn = getConnectionFromURL();
}
} else {
conn = getConnectionFromURL();
}
} catch (Exception ex) {
logger.error(ex, ex);
throw ex;
}
return conn;
}
private Connection getDataSourceConnection() throws SQLException, NamingException {
return dataSource.getConnection();
}
private void initDirectConnectionParams() {
try {
Class.forName(commonRes.getString(CONNECTION_DRIVER_CLASS_NAME));
url = commonRes.getString(CONNECTION_DATABASE_URL);
login = commonRes.getString(CONNECTION_DATABASE_USER);
password = commonRes.getString(CONNECTION_DATABASE_PASSWORD);
} catch (Exception ex) {
logger.error("exception while getting datasource", ex);
}
}
private Connection getConnectionFromURL() throws SQLException {
Connection conn = DriverManager.getConnection(url, login, password);
return conn;
}
}
This class still has a lot of places for the refactoring. For example,
I would be very happy if you would find such places and post examples in comments. In the next post I planned to continue refactoring this class using Strategy pattern.
hmmm…. these code frames makes the post unreadable.
i vote for a “stretched” design!!!
this post should be named not “part 1″ but “cleaning up - preparations to real refactoring”.
so waiting for nicer design as well as for part 2.
2007-01-22 at 4.59 pm
hm. very nice… but why the hell these code frames remain fixed-width?!?
2007-01-23 at 1.27 pm
The css style is cached by your browser
2007-01-23 at 9.37 pm
actually you could just not let my previous post go here. this is why you are called moderator here
2007-01-24 at 5.00 pm
I have received the same question from other readers. I have decided to write the answer here.
2007-01-24 at 5.21 pm
[...] So… It’s time to finish refactoring the example. In part 1 I have prepared a class for extracting strategies. Let’s look at our code:[...]
2007-01-30 at 8.23 pm