Khastie

.classpath

@@ -1,9 +1,11 @@
-<?xml version="1.0" encoding="UTF-8"?>
-<classpath>
-	<classpathentry kind="src" path="src"/>
-	<classpathentry kind="src" path="test"/>
-	<classpathentry kind="con" path="org.eclipse.jdt.launching.JRE_CONTAINER/org.eclipse.jdt.internal.debug.ui.launcher.StandardVMType/JavaSE-1.6"/>
-	<classpathentry kind="lib" path="lib/hsqldb.jar"/>
-	<classpathentry kind="con" path="org.eclipse.jdt.junit.JUNIT_CONTAINER/4"/>
-	<classpathentry kind="output" path="bin"/>
-</classpath>
+<?xml version="1.0" encoding="UTF-8"?>
+
+<classpath>
+	<classpathentry kind="src" path="src"/>
+	<classpathentry kind="src" path="test"/>
+	<classpathentry kind="con" path="org.eclipse.jdt.launching.JRE_CONTAINER/org.eclipse.jdt.internal.debug.ui.launcher.StandardVMType/JavaSE-1.6"/>
+	<classpathentry kind="lib" path="lib/hsqldb.jar"/>
+	<classpathentry kind="con" path="org.eclipse.jdt.junit.JUNIT_CONTAINER/4"/>
+	<classpathentry kind="lib" path="lib/log4j-1.2.12.jar"/>
+	<classpathentry kind="output" path="bin"/>
+</classpath>

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 : "")
+				+ "]";
+	}	
+
 }

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<Person> getPeople();
 }

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<Person> people = new ArrayList<Person>();
+
+	// 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<Person> 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
 	}
 }

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;
+
+	}
 }