diff --git a/.classpath b/.classpath index 3300db2..edea692 100644 --- a/.classpath +++ b/.classpath @@ -1,9 +1,11 @@ - - - - - - - - - + + + + + + + + + + + diff --git a/lib/log4j-1.2.12.jar b/lib/log4j-1.2.12.jar new file mode 100755 index 0000000..9b5a720 Binary files /dev/null and b/lib/log4j-1.2.12.jar differ diff --git a/src/com/proquest/interview/phonebook/Person.java b/src/com/proquest/interview/phonebook/Person.java index 96ccaa5..1eb29d3 100644 --- a/src/com/proquest/interview/phonebook/Person.java +++ b/src/com/proquest/interview/phonebook/Person.java @@ -1,7 +1,64 @@ package com.proquest.interview.phonebook; public class Person { - public String name; - public String phoneNumber; - public String address; + + private String name; + private String phoneNumber; + private String address; + + /** + * @return the name + */ + public String getName() { + return name; + } + + /** + * @param name the name to set + */ + public void setName(String name) { + this.name = name; + } + + /** + * @return the phoneNumber + */ + public String getPhoneNumber() { + return phoneNumber; + } + + /** + * @param phoneNumber the phoneNumber to set + */ + public void setPhoneNumber(String phoneNumber) { + this.phoneNumber = phoneNumber; + } + + /** + * @return the address + */ + public String getAddress() { + return address; + } + + /** + * @param address the address to set + */ + public void setAddress(String address) { + this.address = address; + } + + /* + * @see java.lang.Object#toString() + */ + @Override + public String toString() { + + return "Person [" + + (name != null ? "name=" + name + ", " : "") + + (phoneNumber != null ? "phoneNumber=" + phoneNumber + ", " + : "") + (address != null ? "address=" + address : "") + + "]"; + } + } diff --git a/src/com/proquest/interview/phonebook/PhoneBook.java b/src/com/proquest/interview/phonebook/PhoneBook.java index 47627fd..dea431f 100644 --- a/src/com/proquest/interview/phonebook/PhoneBook.java +++ b/src/com/proquest/interview/phonebook/PhoneBook.java @@ -1,6 +1,9 @@ package com.proquest.interview.phonebook; +import java.util.List; + public interface PhoneBook { public Person findPerson(String firstName, String lastName); public void addPerson(Person newPerson); + public List getPeople(); } diff --git a/src/com/proquest/interview/phonebook/PhoneBookImpl.java b/src/com/proquest/interview/phonebook/PhoneBookImpl.java index d7eb6fe..58e9056 100644 --- a/src/com/proquest/interview/phonebook/PhoneBookImpl.java +++ b/src/com/proquest/interview/phonebook/PhoneBookImpl.java @@ -1,20 +1,114 @@ package com.proquest.interview.phonebook; -import java.util.List; - import com.proquest.interview.util.DatabaseUtil; +import com.proquest.interview.util.ErrorLogger; +import org.apache.log4j.Logger; + +import java.sql.Connection; +import java.sql.PreparedStatement; +import java.sql.ResultSet; +import java.sql.Statement; +import java.util.ArrayList; +import java.util.List; public class PhoneBookImpl implements PhoneBook { - public List people; + + static Logger logger = Logger.getLogger(PhoneBookImpl.class); + + private List people = new ArrayList(); + + // NOTE: All of this DB stuff should probably be moved into a DAO layer @Override public void addPerson(Person newPerson) { - //TODO: write this method + + people.add(newPerson); + + Connection conn = null; + PreparedStatement ps = null; + // NOTE: getSqlValue will escape values properly, so we can find names like D'Angelo, etc... + // Adding single quotes manually is for sissies. That's what computers are for. + // Should probably put the column names into an enum or at least private final static Strings + String query = "INSERT INTO phonebook (name, phoneNumber, address) VALUES (?, ?, ?)"; + + // NOTE: we should use a real profiling lib like log4jdbc + // Allow for trace-/debug-level logging of queries + logger.debug("Executing query: " + query); + + try { + + conn = DatabaseUtil.getConnection(); + ps = conn.prepareStatement(query); + + ps.setString(1, newPerson.getName()); + ps.setString(2, newPerson.getPhoneNumber()); + ps.setString(3, newPerson.getAddress()); + + ps.executeUpdate(); + + } catch (Exception ex) { + logger.error("Problem finding person in DB. Query:" + query); + ErrorLogger.filterStackTrace(ex); // NOTE: I don't need to see this entire exception every time. Just enough to trace it in our domain code. + + } finally { + DatabaseUtil.closeAll(ps, conn, query); + } + } + /** + * NOTE: This method assumes that only the first person found will be returned. + */ @Override - public Person findPerson() { + public Person findPerson(String firstName, String lastName) { //TODO: write this method + + Person person = new Person(); + + Connection conn = null; + Statement stmt = null; + ResultSet rs = null; + + String query = "SELECT name, phonenumber, address FROM phonebook WHERE name LIKE " + + DatabaseUtil.getSqlValue(firstName + " " + lastName) + + " LIMIT 1"; + + logger.debug("Executing query: " + query); + + try { + conn = DatabaseUtil.getConnection(); + stmt = conn.createStatement(); + rs = stmt.executeQuery(query); + + while (rs.next()) { + // NOTE: small efficiency gains by using column indices instead of names are usually trumped by readability/maintainability + // Optimize when you need to. + person.setName(rs.getString("name")); + person.setPhoneNumber(rs.getString("phonenumber")); + person.setAddress(rs.getString("address")); + logger.debug("Finding person: " + person); + } + + } catch (Exception ex) { + + logger.error("Problem finding person in DB. Query:" + query); + ErrorLogger.filterStackTrace(ex); + + } finally { + DatabaseUtil.closeAll(rs, stmt, conn, query); + } + + return person; + + } + + /** + * getter + setter = better (than leaving _people_ publically accessible) + * Also + */ + @Override + public List getPeople() { + return people; } public static void main(String[] args) { @@ -24,8 +118,34 @@ public static void main(String[] args) { * John Smith, (248) 123-4567, 1234 Sand Hill Dr, Royal Oak, MI * Cynthia Smith, (824) 128-8758, 875 Main St, Ann Arbor, MI */ + + PhoneBook phoneBook = new PhoneBookImpl(); + Person p1 = new Person(), p2 = new Person(); + + p1.setName("John Smith"); + p1.setAddress("1234 Sand Hill Dr, Royal Oak, MI"); + p1.setPhoneNumber("(248) 123-4567"); + + p2.setName("Cynthia Smith"); + p2.setAddress("875 Main St, Ann Arbor, MI"); + p2.setPhoneNumber("(824) 128-8758"); + + System.out.println("\n1. Adding people..."); + phoneBook.addPerson(p1); + phoneBook.addPerson(p2); + + System.out.println("\n2. Printing phone book..."); // TODO: print the phone book out to System.out + for (Person person : phoneBook.getPeople()) { + System.out.println(person); + } + // TODO: find Cynthia Smith and print out just her entry + + System.out.println("\n3. Finding a person..."); + System.out.println(phoneBook.findPerson("Cynthia", "Smith")); + // TODO: insert the new person objects into the database + // done above } } diff --git a/src/com/proquest/interview/util/DatabaseUtil.java b/src/com/proquest/interview/util/DatabaseUtil.java index a23342d..201ba27 100644 --- a/src/com/proquest/interview/util/DatabaseUtil.java +++ b/src/com/proquest/interview/util/DatabaseUtil.java @@ -1,31 +1,133 @@ package com.proquest.interview.util; -import java.sql.Connection; -import java.sql.DriverManager; -import java.sql.SQLException; -import java.sql.Statement; +import org.apache.log4j.Logger; + +import java.sql.*; /** * This class is just a utility class, you should not have to change anything here * @author rconklin */ public class DatabaseUtil { + + static Logger logger = Logger.getLogger(DatabaseUtil.class); + public static void initDB() { + Connection cn = null; + Statement stmt = null; try { - Connection cn = getConnection(); - Statement stmt = cn.createStatement(); + cn = getConnection(); + stmt = cn.createStatement(); + // NOTE: This table could use an indexed primary key column (PERSON_ID) + // stmt.execute("DROP TABLE PHONEBOOK"); stmt.execute("CREATE TABLE PHONEBOOK (NAME varchar(255), PHONENUMBER varchar(255), ADDRESS varchar(255))"); stmt.execute("INSERT INTO PHONEBOOK (NAME, PHONENUMBER, ADDRESS) VALUES('Chris Johnson','(321) 231-7876', '452 Freeman Drive, Algonac, MI')"); stmt.execute("INSERT INTO PHONEBOOK (NAME, PHONENUMBER, ADDRESS) VALUES('Dave Williams','(231) 502-1236', '285 Huron St, Port Austin, MI')"); cn.commit(); cn.close(); } catch (Exception ex) { - ex.printStackTrace(); + logger.warn("Failed to init DB."); + ErrorLogger.filterStackTrace(ex); + } finally { + closeAll(stmt, cn); } } + public static Connection getConnection() throws SQLException, ClassNotFoundException { Class.forName("org.hsqldb.jdbcDriver"); return DriverManager.getConnection("jdbc:hsqldb:mem", "sa", ""); } + + /** + * Closes the result set, statement & connection. + * + * @param rs + * @param stmt + * @param conn + * @param query + */ + public static void closeAll (ResultSet rs, Statement stmt, Connection conn, String query) { + + try { + if (rs != null) { + rs.close(); + } + + if (stmt != null ) { + stmt.close(); + } + + if (conn != null) { + conn.close(); + } + } catch (Exception e) { + logger.warn("Unable to close rs/stmt/conn. Query: " + query, e); + } + } + + /** + * Closes the statement & connection. {@link DatabaseUtil#closeAll(Statement, Connection, String)} is preferred. + * + * @param stmt + * @param conn + */ + public static void closeAll (Statement stmt, Connection conn) { + try { + if (stmt != null) { + stmt.close(); + } + + if (conn != null) { + conn.close(); + } + } catch (Exception e) { + logger.warn("Unable to close stmt/conn.", e); + } + } + + /** + * Closes the statement & connection. + * + * @param stmt + * @param conn + * @param query + */ + public static void closeAll (Statement stmt, Connection conn, String query) { + try { + if (stmt != null) { + stmt.close(); + } + + if (conn != null) { + conn.close(); + } + } catch (Exception e) { + logger.warn("Unable to close stmt/conn. Query: " + query, e); + } + } + + + /** + * Helper method to convert strings to a sql-ready state. Probably don't want to pass pre-escaped SQL to this. Could + * add functionality to handle converting nulls to empty strings or vice versa, etc... if desired + * + * @param str The String to be converted. Should not already be escaped. + * + * @return The string, escaped for SQL and wrapped in single quotes when necessary (which is always in this case + * unless the string is null). Empty strings are wrapped in single quotes. Don't ever wrap the string returned by this method in single quotes. + */ + public static String getSqlValue(String str) { + + // Could be optimized + if (str != null) { + str = str.replace("'", "''"); // escape single quotes (could use StringEscapeUtils#escapeSql instead) + StringBuilder sb = new StringBuilder(str.length() + 2); + sb.append("'").append(str).append("'"); + return sb.toString(); + } + + return str; + + } } diff --git a/src/com/proquest/interview/util/ErrorLogger.java b/src/com/proquest/interview/util/ErrorLogger.java new file mode 100755 index 0000000..9e79402 --- /dev/null +++ b/src/com/proquest/interview/util/ErrorLogger.java @@ -0,0 +1,50 @@ +package com.proquest.interview.util; + +import org.apache.log4j.Logger; + +public class ErrorLogger { + + static Logger logger = Logger.getLogger(ErrorLogger.class); + + /** + * Print a pared down stacktrace when reporting handled exceptions + */ + static public void filterStackTrace(Exception e) { + + StackTraceElement[] stackTraceElements = e.getStackTrace(); + + logger.error("\n" + e); + + for (int i = 0; i < stackTraceElements.length; i++) { + + StackTraceElement ste = stackTraceElements[i]; + + if (i == 0 || + ste.getClassName().contains("com.proquest") || + ste.getClassName().contains("org.apache.jsp")) { + + System.err.println("\tat " + + ste.getClassName() + "." + + ste.getMethodName() + "(" + + ste.getFileName() + ":" + + ste.getLineNumber() + ")" + ); + + } + } + + // print the path to the first referenced compiled jsp + for (StackTraceElement ste : stackTraceElements) { + + if (ste.getClassName().contains("org.apache.jsp")) { + + // Determine path to jsp source file from compiled jsp class name + System.err.println("WPath: " + ste.getClassName().replace(".", "\\") + "\\" + ste.getFileName() + ":" + ste.getLineNumber()); + System.err.println("LPath: /var/tomcat/webapps/ROOT/" + ste.getClassName().replace(".", "/") + "/" + ste.getFileName() + ":" + ste.getLineNumber()); + break; + + } + } + } + +} diff --git a/src/log4j.properties b/src/log4j.properties new file mode 100755 index 0000000..a25b5cb --- /dev/null +++ b/src/log4j.properties @@ -0,0 +1,13 @@ +! turn off the internal log4j debugging flag so we can see what it is doing +log4j.debug=false + +# Set root logger level to INFO and its only appender to CONSOLE. +log4j.rootLogger=INFO,CONSOLE +log4j.logger.com.proquest.interview.phonebook=INFO + +# +# CONSOLE uses PatternLayout. +# +log4j.appender.CONSOLE=org.apache.log4j.ConsoleAppender +log4j.appender.CONSOLE.layout=org.apache.log4j.PatternLayout +log4j.appender.CONSOLE.layout.ConversionPattern=%-4r [%t] %-5p %c %x %M - %m%n