From 93cc08e32a4c8a659e86a712bcabffeef0f9bc08 Mon Sep 17 00:00:00 2001
From: Wil Cooley <wcooley@nakedape.cc>
Date: Sat, 24 May 2014 11:13:23 -0700
Subject: [PATCH 1/7] Add basic working implementations

---
 cachematrix.R | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/cachematrix.R b/cachematrix.R
index a50be65aa44..01819e36d70 100644
--- a/cachematrix.R
+++ b/cachematrix.R
@@ -4,7 +4,18 @@
 ## Write a short comment describing this function
 
 makeCacheMatrix <- function(x = matrix()) {
+    invmatrix <- NULL
+    set <- function(y) {
+        x <<- y
+        m <<- NULL
+    }
 
+    get <- function() x
+
+    setinverse <- function(inv) invmatrix <<- inv
+    getinverse <- function() invmatrix
+
+    list(set=set, get=get, setinverse=setinverse, getinverse=getinverse)
 }
 
 
@@ -12,4 +23,13 @@ makeCacheMatrix <- function(x = matrix()) {
 
 cacheSolve <- function(x, ...) {
         ## Return a matrix that is the inverse of 'x'
+    m <- x$getinverse()
+    if(!is.null(m)) {
+        message("getting cached data")
+        return(m)
+    }
+    data <- x$get()
+    m <- solve(data, ...)
+    x$setinverse(m)
+    m
 }

From bb048d7c4ea8ccdc8306c473974500f9f2c1a521 Mon Sep 17 00:00:00 2001
From: Wil Cooley <wcooley@nakedape.cc>
Date: Sat, 24 May 2014 22:56:01 -0700
Subject: [PATCH 2/7] Add test driver

---
 test_cachematrix.R | 61 ++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 61 insertions(+)
 create mode 100644 test_cachematrix.R

diff --git a/test_cachematrix.R b/test_cachematrix.R
new file mode 100644
index 00000000000..be4cadb0458
--- /dev/null
+++ b/test_cachematrix.R
@@ -0,0 +1,61 @@
+library(testthat)
+# I don't know how to make proper packages yet, so I'll hack with `source` for
+# now.
+#library(cachematrix)
+source("cachematrix.R")
+
+# Pre-computed results
+setup_fixtures <- function() {
+    xvect <- c(2,3,0,3,0,2,2,1,1)
+    xm <- matrix(xvect,3,3)
+    xvect_inv <- c(2,3,-6,-1,-2,4,-3,-4,9)
+    xm_inv <- matrix(xvect_inv,3,3)
+
+    xvect2 <- xvect
+    xvect2[1] <- 1
+    xm2 <- matrix(xvect2,3,3)
+    xvect2_inv <- c(-2,-3,6,1,1,-2,3,5,-9)
+    xm2_inv <- matrix(xvect2_inv,3,3)
+
+    list(xm=xm, xm_inv=xm_inv, xm2=xm2, xm2_inv=xm2_inv)
+}
+
+test_that("makeCacheMatrix makes a matrix-ish", {
+    fix <- setup_fixtures()
+
+    cm <- makeCacheMatrix(fix$xm)
+
+    expect_that(cm$get(), equals(fix$xm))
+})
+
+test_that("cacheSolve produces inverted matrix", {
+    fix <- setup_fixtures()
+
+    cm <- makeCacheMatrix(fix$xm)
+    cm_inv <- cacheSolve(cm)
+
+    expect_that(cm_inv, equals(fix$xm_inv))
+
+})
+
+test_that("cacheSolve uses cache", {
+    fix <- setup_fixtures()
+
+    cm <- makeCacheMatrix(fix$xm)
+    cm_inv <- cacheSolve(cm)
+
+    expect_that(cacheSolve(cm), shows_message("getting cached data"))
+})
+
+test_that("set invalidates cache", {
+    fix <- setup_fixtures()
+
+    cm <- makeCacheMatrix(fix$xm)
+    cm_inv <- cacheSolve(cm)
+
+    expect_that(cm$set(fix$xm2), shows_message("invalidating cache"))
+    cm_inv2 <- cacheSolve(cm)
+
+    expect_that(cm_inv2, not(equals(cm_inv)))
+    expect_that(cm_inv2, equals(fix$xm2_inv))
+})

From 6a7688ed5adb11d6f5e440ae311e523a05445af5 Mon Sep 17 00:00:00 2001
From: Wil Cooley <wcooley@nakedape.cc>
Date: Sat, 24 May 2014 22:57:41 -0700
Subject: [PATCH 3/7] Add some explanatory comments

---
 cachematrix.R | 19 ++++++++++++++-----
 1 file changed, 14 insertions(+), 5 deletions(-)

diff --git a/cachematrix.R b/cachematrix.R
index 01819e36d70..1839c47a260 100644
--- a/cachematrix.R
+++ b/cachematrix.R
@@ -1,7 +1,14 @@
-## Put comments here that give an overall description of what your
-## functions do
-
-## Write a short comment describing this function
+#
+# Functions `makeCacheMatrix` and `cacheSolve`.
+#
+# These are so close to the examples `makeVector` and `cachemean` that it almost
+# feels like cheating...
+
+# The `makeCacheMatrix` function creates a list of closures, which access a
+# stored matrix and an alternative representation of the matrix.
+# Returns a list of functions which encapsulate the matrix object:
+#  * getter & setter for the matrix
+#  * getter & setter for the alternative representation
 
 makeCacheMatrix <- function(x = matrix()) {
     invmatrix <- NULL
@@ -19,7 +26,9 @@ makeCacheMatrix <- function(x = matrix()) {
 }
 
 
-## Write a short comment describing this function
+# The `cacheSolve` function returns the cached inverse of the matrix inside the
+# scope accessed by the functions in `x`, solving and caching the inverse if it
+# does not already exist.
 
 cacheSolve <- function(x, ...) {
         ## Return a matrix that is the inverse of 'x'

From da022a3496d68e9312b0232f9a18f895a05fbb30 Mon Sep 17 00:00:00 2001
From: Wil Cooley <wcooley@nakedape.cc>
Date: Sat, 24 May 2014 22:58:10 -0700
Subject: [PATCH 4/7] Refactor lots of the internals

* Fix bug that cache wasn't being invalidated due to using `m` instead
  of `invmatrix` (now `altx).

* Reflavor the cache to be about an alternative representation of the
  matrix, rather than necessarily just inversion.

* Add message indicating when cache is being invalidated.
---
 cachematrix.R | 27 +++++++++++++++++++--------
 1 file changed, 19 insertions(+), 8 deletions(-)

diff --git a/cachematrix.R b/cachematrix.R
index 1839c47a260..8cb5dfe503f 100644
--- a/cachematrix.R
+++ b/cachematrix.R
@@ -11,18 +11,28 @@
 #  * getter & setter for the alternative representation
 
 makeCacheMatrix <- function(x = matrix()) {
-    invmatrix <- NULL
+    # Initialize the matrix alternative
+    altx <- NULL
+
+    # setter, which sets the `x` and `altx` in the parent scope
     set <- function(y) {
         x <<- y
-        m <<- NULL
+        # If altx is already set (non-NULL), then we're invalidating the cache;
+        # otherwise we're just initializing it.
+        if (!is.null(altx)) message("invalidating cache")
+        altx <<- NULL
     }
 
+    # getter for `x` in the parent scope
     get <- function() x
 
-    setinverse <- function(inv) invmatrix <<- inv
-    getinverse <- function() invmatrix
+    # setter, which updates `altx` in the parent scope
+    set_alt <- function(alt) altx <<- alt
+
+    # getter for `altx` in the parent scope
+    get_alt <- function() altx
 
-    list(set=set, get=get, setinverse=setinverse, getinverse=getinverse)
+    list(set=set, get=get, set_alt=set_alt, get_alt=get_alt)
 }
 
 
@@ -31,14 +41,15 @@ makeCacheMatrix <- function(x = matrix()) {
 # does not already exist.
 
 cacheSolve <- function(x, ...) {
-        ## Return a matrix that is the inverse of 'x'
-    m <- x$getinverse()
+
+    # `m` is NULL if the cache is empty
+    m <- x$get_alt()
     if(!is.null(m)) {
         message("getting cached data")
         return(m)
     }
     data <- x$get()
     m <- solve(data, ...)
-    x$setinverse(m)
+    x$set_alt(m)
     m
 }

From 887014770177c8d8b86035efe8d96d625c356dc1 Mon Sep 17 00:00:00 2001
From: Wil Cooley <wcooley@nakedape.cc>
Date: Sat, 24 May 2014 23:04:38 -0700
Subject: [PATCH 5/7] Move both branches w/in conditional for symmetry

and to have a single exit from the function.
---
 cachematrix.R | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/cachematrix.R b/cachematrix.R
index 8cb5dfe503f..d7d55074b0a 100644
--- a/cachematrix.R
+++ b/cachematrix.R
@@ -44,12 +44,13 @@ cacheSolve <- function(x, ...) {
 
     # `m` is NULL if the cache is empty
     m <- x$get_alt()
-    if(!is.null(m)) {
+    if (is.null(m)) {
+        data <- x$get()
+        m <- solve(data, ...)
+        x$set_alt(m)
+    }
+    else {
         message("getting cached data")
-        return(m)
     }
-    data <- x$get()
-    m <- solve(data, ...)
-    x$set_alt(m)
     m
 }

From 11a3ba55bb822354d5a62cbadfa911bc77423359 Mon Sep 17 00:00:00 2001
From: Wil Cooley <wcooley@nakedape.cc>
Date: Sat, 24 May 2014 23:09:01 -0700
Subject: [PATCH 6/7] Use set_alt to in set instead of a 2nd assignment

---
 cachematrix.R | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/cachematrix.R b/cachematrix.R
index d7d55074b0a..57a3f7b1a02 100644
--- a/cachematrix.R
+++ b/cachematrix.R
@@ -20,7 +20,7 @@ makeCacheMatrix <- function(x = matrix()) {
         # If altx is already set (non-NULL), then we're invalidating the cache;
         # otherwise we're just initializing it.
         if (!is.null(altx)) message("invalidating cache")
-        altx <<- NULL
+        set_alt(NULL)
     }
 
     # getter for `x` in the parent scope
@@ -35,7 +35,6 @@ makeCacheMatrix <- function(x = matrix()) {
     list(set=set, get=get, set_alt=set_alt, get_alt=get_alt)
 }
 
-
 # The `cacheSolve` function returns the cached inverse of the matrix inside the
 # scope accessed by the functions in `x`, solving and caching the inverse if it
 # does not already exist.

From 7ae61949045de87c715c2d08ba690af0c16e8280 Mon Sep 17 00:00:00 2001
From: Wil Cooley <wcooley@nakedape.cc>
Date: Sat, 24 May 2014 23:11:24 -0700
Subject: [PATCH 7/7] Use multiline block for set_alt

I am not crazy about single-statement-non-block for anything
non-trivial; it makes it to easy to miss things (see the Apple "goto
fail" SSL bug for example).
---
 cachematrix.R | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/cachematrix.R b/cachematrix.R
index 57a3f7b1a02..f4c8de777a6 100644
--- a/cachematrix.R
+++ b/cachematrix.R
@@ -6,6 +6,7 @@
 
 # The `makeCacheMatrix` function creates a list of closures, which access a
 # stored matrix and an alternative representation of the matrix.
+#
 # Returns a list of functions which encapsulate the matrix object:
 #  * getter & setter for the matrix
 #  * getter & setter for the alternative representation
@@ -26,8 +27,10 @@ makeCacheMatrix <- function(x = matrix()) {
     # getter for `x` in the parent scope
     get <- function() x
 
-    # setter, which updates `altx` in the parent scope
-    set_alt <- function(alt) altx <<- alt
+    # setter for `altx` in the parent scope
+    set_alt <- function(alt) {
+        altx <<- alt
+    }
 
     # getter for `altx` in the parent scope
     get_alt <- function() altx
