Refactor AWS Directory

* Silence warnings regarding uninitialized instance variable @location
* Tidy up memoization
* Extensive use of getters instead of instance variables

Fixes #884

lib/fog/aws/models/storage/directory.rb

@@ -7,17 +7,23 @@ module Fog
     class AWS
 
       class Directory < Fog::Model
+        VALID_ACLS = ['private', 'public-read', 'public-read-write', 'authenticated-read']
+
+        # See http://docs.amazonwebservices.com/AmazonS3/latest/API/RESTBucketPUT.html
+        INVALID_LOCATIONS = ['us-east-1']
+
+        attr_reader :acl
 
         identity  :key,           :aliases => ['Name', 'name']
 
         attribute :creation_date, :aliases => 'CreationDate'
 
         def acl=(new_acl)
-          valid_acls = ['private', 'public-read', 'public-read-write', 'authenticated-read']
-          unless valid_acls.include?(new_acl)
-            raise ArgumentError.new("acl must be one of [#{valid_acls.join(', ')}]")
+          if VALID_ACLS.include?(new_acl)
+            raise ArgumentError.new("acl must be one of [#{VALID_ACLS.join(', ')}]")
+          else
+            @acl = new_acl
           end
-          @acl = new_acl
         end
 
         def destroy
@@ -30,21 +36,19 @@ module Fog
 
         def location
           requires :key
-          data = connection.get_bucket_location(key)
-          data.body['LocationConstraint']
+          @location || bucket_location || self.connection.region
         end
 
         def location=(new_location)
-          @location = new_location
+          if INVALID_LOCATIONS.include?(new_location)
+            raise ArgumentError, "location must not include any of #{INVALID_LOCATIONS.join(', ')}. See http://docs.amazonwebservices.com/AmazonS3/latest/API/RESTBucketPUT.html"
+          else
+            @location = new_location
+          end
         end
 
         def files
-          @files ||= begin
-            Fog::Storage::AWS::Files.new(
-              :directory    => self,
-              :connection   => connection
-            )
-          end
+          @files ||= Fog::Storage::AWS::Files.new(:directory => self, :connection => connection)
         end
 
         def payer
@@ -71,20 +75,11 @@ module Fog
         end
 
         def versions
-          @versions ||= begin
-            Fog::Storage::AWS::Versions.new(
-                :directory    => self,
-                :connection   => connection
-            )
-          end
+          @versions ||= Fog::Storage::AWS::Versions.new(:directory => self, :connection => connection)
         end
 
         def public=(new_public)
-          if new_public
-            @acl = 'public-read'
-          else
-            @acl = 'private'
-          end
+          self.acl = new_public ? 'public-read' : 'private'
           new_public
         end
 
@@ -103,18 +98,25 @@ module Fog
 
         def save
           requires :key
+
           options = {}
-          if @acl
-            options['x-amz-acl'] = @acl
-          end
-          lc = @location || self.connection.region
-          if lc != 'us-east-1'     # us-east-1 is not valid: See http://docs.amazonwebservices.com/AmazonS3/latest/API/RESTBucketPUT.html
-            options['LocationConstraint'] = lc
-          end
+
+          options['x-amz-acl'] = acl if acl
+
+          options['LocationConstraint'] = location
+
           connection.put_bucket(key, options)
+
           true
         end
 
+        private
+
+        def bucket_location
+          data = connection.get_bucket_location(key)
+          data.body['LocationConstraint']
+        end
+
       end
 
     end