From d38456ef7889a82bd307435e7b37bc5584b6f2ea Mon Sep 17 00:00:00 2001
From: Dmitry Petrov <dmitry.petrov@nevesomo.com>
Date: Sat, 25 Mar 2017 17:48:56 -0700
Subject: [PATCH] Refactoring: extract methods

---
 dvc/cmd_data_remove.py | 50 ++++++++++++++++++++++++++----------------
 1 file changed, 31 insertions(+), 19 deletions(-)

diff --git a/dvc/cmd_data_remove.py b/dvc/cmd_data_remove.py
index 02d05f5ec..1f8dc9639 100644
--- a/dvc/cmd_data_remove.py
+++ b/dvc/cmd_data_remove.py
@@ -48,13 +48,7 @@ class CmdDataRemove(CmdBase):
 
         error = False
         for target in self.args.target:
-            try:
-                if os.path.isdir(target):
-                    self.remove_dir(target)
-                else:
-                    self.remove_data_instance(target)
-            except NeatLynxException as ex:
-                Logger.error('Unable to remove data file "{}": {}'.format(target, ex))
+            if not self.remove_target(target):
                 error = True
 
         message = 'DVC data remove: {}'.format(' '.join(self.args.target))
@@ -62,6 +56,17 @@ class CmdDataRemove(CmdBase):
 
         return 0 if error == 0 else 1
 
+    def remove_target(self, target):
+        try:
+            if os.path.isdir(target):
+                self.remove_dir(target)
+            else:
+                self.remove_data_instance(target)
+            return True
+        except NeatLynxException as ex:
+            Logger.error('Unable to remove data file "{}": {}'.format(target, ex))
+            return False
+
     def remove_dir(self, target):
         if not self.args.recursive:
             raise DataRemoveError('Directory "%s" cannot be removed. Use --recurcive flag.' % target)
@@ -83,13 +88,19 @@ class CmdDataRemove(CmdBase):
         # it raises exception if not a symlink is provided
         data_path = self.path_factory.existing_data_path(target)
 
-        if not self.args.keep_in_cache and os.path.isfile(data_path.cache.relative):
-            os.remove(data_path.cache.relative)
-            self.remove_dir_if_empty(data_path.cache.relative)
-        else:
-            if not self.args.keep_in_cache:
-                Logger.warn(u'Unable to find cache file for data instance %s' % data_path.data.relative)
+        self._remove_cache_file(data_path)
+        self._remove_state_file(data_path)
+        self._remove_cloud_cache(data_path)
 
+        os.remove(data_path.data.relative)
+        pass
+
+    def _remove_cloud_cache(self, data_path):
+        if not self.args.keep_in_cloud:
+            aws_key = self.cache_file_aws_key(data_path.cache.dvc)
+            self.remove_from_cloud(aws_key)
+
+    def _remove_state_file(self, data_path):
         if os.path.isfile(data_path.state.relative):
             os.remove(data_path.state.relative)
             self.remove_dir_if_empty(data_path.state.relative)
@@ -97,12 +108,13 @@ class CmdDataRemove(CmdBase):
             Logger.warn(u'State file {} for data instance {} does not exist'.format(
                 data_path.state.relative, data_path.data.relative))
 
-        if not self.args.keep_in_cloud:
-            aws_key = self.cache_file_aws_key(data_path.cache.dvc)
-            self.remove_from_cloud(aws_key)
-
-        os.remove(data_path.data.relative)
-        pass
+    def _remove_cache_file(self, data_path):
+        if not self.args.keep_in_cache and os.path.isfile(data_path.cache.relative):
+            os.remove(data_path.cache.relative)
+            self.remove_dir_if_empty(data_path.cache.relative)
+        else:
+            if not self.args.keep_in_cache:
+                Logger.warn(u'Unable to find cache file for data instance %s' % data_path.data.relative)
 
     def remove_from_cloud(self, aws_file_name):
         conn = S3Connection(self.config.aws_access_key_id, self.config.aws_secret_access_key)