310 likes | 595 Views
From Bad Practices to Best Practices. A refactoring case study Randy Ballew Student Information Systems University of California. Overview . Introduction/Background Metamorphosis -- Seconds per transaction -> milliseconds RMI -> EJB -> Web Service Test-driven development
E N D
From Bad Practices to Best Practices A refactoring case study Randy Ballew Student Information Systems University of California
Overview • Introduction/Background • Metamorphosis -- • Seconds per transaction -> milliseconds • RMI -> EJB -> Web Service • Test-driven development • Design patterns • Open-source tools • Demo that doesn’t work
What is Refactoring? • Refactoring - Improving the Design of Existing Code • Martin Fowler with Kent Beck and others • “Controlled and Efficient” redesign of code • Extract Method • Extract Class • Risky -- test as you go • Junit framework makes it safer
When should I refactor? • Early and often • Especially if you IDE supports it • Performance problems may force your hand • As in today’s case study…
Chart of Accounts Validation • Validates financial account strings • Combination of codes (business unit, account, fund, organization) • Must be valid atomically • Must be valid in combinations • Standalone RMI server using JDBC • Severe performance problems (multi-second per transaction) • Development was out-sourced
First Things First • “The basic environment for good programming” • Automated build process • Automated testing • Coding standards • Version Control Tool • Tom and Mary Poppendieck, Lean Software Development
First Things First • “The basic environment for good programming” • Automated build process • Ant • Automated testing • Junit/Ant • Coding standards • Packages • Jalopy Eclipse plugin • Version Control Tool • CVS integrated with Eclipse
Refactoring Phase 1 • Import into CVS • Check out as Eclipse Java Project • Code review • Instrument implementation • Trace execution logic -- UML sequence • Capture generated SQL queries • Timing
Bad Smells in Code (Review) • Procedural rather than object-oriented • A single RMI interface definition - COA.java • Implemented in a single java class with no inner classes -- COAImpl.java • Specification and legacy system partly to blame • But not totally - no class that encapsulates the fundamental object (ChartString) in the system
Bad Smells (continued) • Overlong methods • Unnamed magic constants • String concatenation using += operator • try/catch(SQLException) without finally
Instrumentation results • A very large number (21) JDBC round trips per validation • Half of these perform query generation and produce identical queries • Revisit of code notes that all queries of the form java.sql.Statement.executeQuery() • One-time generation of java.sql.PreparedStatement should help performance
Instrumentation Results (cont) • Timing • Query generation, while inelegantly implemented, is not the bottleneck • Actual validation queries account for 90% of execution time • Query plan analysis reveals that all queries require table scan
Pair Programming • Need to uncover and understand the business logic • Work side-by-side with domain expert • PeopleSoft Financials “set id” • Table-driven “combination rules” partitioned by “Business Unit” • Refactored Design • One-time generation of sql code at initialization time • “Query Maps” keyed by “Business Unit”
21 Round Trips - 1 Round Trip • Combination rule • If • Any combination exists for this “anchor” • Then • This combination must be present
21 Round Trips - 1 Round Trip • Procedural - (foreach rule) // do we need to check combo? boolean checkCombo = false; ResultSet rs = stmt.executeQuery(); while (rs.next()) { checkCombo = true; } if (checkCombo) { boolean comboOK = false; ResultSet combos = comboStatement.executeQuery(); while (combos.next()) { comboOK = true; }
21 Round Trips - 1 Round Trip • If the validation failed, execute another round trip to retrieve error message. • Yikes! Let’s cache the error text at initialization… • There’s something wrong with that result processing… • Wouldn’t if (rs.next()) be a little more efficient? • Wait a second. If the last thing we do is fetch error message, what we’re really looking for is…
21 Round Trips - 1 Round Trip • SPECIFICATION (Evans and Fowler, “Specifications” Plop97 and Evans, Domain-Driven Design) • A SPECIFICATION is a predicate that determines if an object does or does not satisfy some criteria • “Lucky for us, SQL is a very natural way to write SPECIFICATIONS.” -- Eric Evans • So is java.util.regexp (the object being examined is a java.lang.String)
21 Round Trips - 1 Round Trip • Malformed (business unit either “1” or “J”, account is 5 numeric chars, etc) • Use regular expressions • Atomically invalid SELECT “AccountInvalid” WHERE NOT EXISTS (SELECT 1 FROM ACCOUNT WHERE ACCOUNT_ID = ? AND SET_ID = ?)
21 Round Trips - 1 Round Trip • Combination rule • If • Any combination exists for this “anchor” • Then • This combination must be present
21 Round Trips - 1 Round Trip • Combination rule as SQL SELECT ‘5FieldEdit’ WHERE EXISTS (SELECT 1 FROM VALID_COMBINATION WHERE COMBINATION = ‘5FieldEdit’ AND SET_ID = ? AND ORG_CODE = ?) AND NOT EXISTS (SELECT 1 FROM VALID_COMBINATION WHERE COMBINATION = ‘5FieldEdit’ AND SET_ID = ? AND ORG_CODE = ? AND ACCOUNT = ? AND ACCOUNT = ? AND FUND = ? …)
21 Round Trips - 1 Round Trip • Use SQL union operator to send all SPECIFICATION queries in a single RPC • If no results, valid • Otherwise, we have a list of rule names that should be reported (we’re planning on maintaining a cache for this) • Presto, 21 down to 1
Cover Query With Index • Now that we’ve got RPC count down to one, we can deal with performance at the database level • Create indexes that “cover” each query so that we never hit data pages • Average time per validation: 80 ms. • Your DBA is your friend • If she isn’t, take her to lunch tomorrow…
Layers (from mud to structure) • So far, we’ve done a great deal of SQL work, but not much architecture • No, we we’ve been dealing with the innermost LAYER (Buschman, et al, POSA). • Think of layers as an onion, and work from the inside out. • Test as you go!
Layers and POJO • Plain Old Java Objects • Separation of concerns • Facilitates unit testing • Enables reuse and repurposing • Constants class • Effective Java -- Joshua Block • ChartStringTokenizer • Encapsulates all string manipulation
Layers and POJO • Data Access Object (Core J2EE) • Encapsulates all JDBC • SqlGenerator • Refactoring -- extracted from first DAO impl • Single Responsibility Principle • Validator • Business logic (applies SPECIFICATIONS) • ChartStringBean • Simple VALUE OBJECT • RMI implementation • Delegates to Validator (GoF DECORATOR)
RMI • Junit tests validate performance • Junit tests validate identical results from original implementation and new implementation • Junit++ • Multi-threaded test runner • Reveals concurrency problem • Single-threaded, no connection pool
J2EE Container • Use the services the platform provides • Refactor DAO to use connection pool • javax.sql.Datasource • Replace RMI layer with EJB (concurrency) • Xdoclet • Javadoc API and ant tasks • Generates Home, Remote, Local, ejb-jar.xml, and vendor-specific deployment descriptors
Web Services • Why deploy as a web service? • Frankly, as a test case • Need to provide web services based access to Student Information Systems data for Course Management and Learning Management
Web Services • Apache Axis / Jboss-net • XML deployment descriptor publishes Session Bean as Web Service • WSDL2Java • Generates client stub, locator scaffolding • And a simple Junit test case that pings the service
Rich Swing Client • Break out of the browser prison • Inspired by Sun’s Jamazon demo at JavaOne 2003 • Currently being tested by selected departments