From 13f943b4a1a2fc5bb40af7f745e4ff09fdf9ae52 Mon Sep 17 00:00:00 2001 From: Florent Rougon Date: Fri, 2 Oct 2020 16:19:18 +0200 Subject: [PATCH] terrasync.py: rename DirIndex attributes and remove accessors In Python, common usage is not to define accessors, but to directly use class or instance attributes (especially when the associated data is constant after instance creation). If it later happens that a given attribute needs getter or setter logic, this can always be done via the @property decorator, and doesn't affect calling code at all. See for instance: https://docs.python.org/3/library/functions.html#property https://mail.python.org/pipermail/tutor/2012-December/thread.html#92990 Apply this to the DirIndex class and rename the following attributes for better readability: f -> files, d -> directories, t -> tarballs. --- .../python/TerraSync/terrasync/dirindex.py | 39 +++++-------------- scripts/python/TerraSync/terrasync/main.py | 8 ++-- .../python/TerraSync/tests/test_dirindex.py | 32 +++------------ 3 files changed, 19 insertions(+), 60 deletions(-) diff --git a/scripts/python/TerraSync/terrasync/dirindex.py b/scripts/python/TerraSync/terrasync/dirindex.py index 4e3d7db28..35c2b85f3 100644 --- a/scripts/python/TerraSync/terrasync/dirindex.py +++ b/scripts/python/TerraSync/terrasync/dirindex.py @@ -28,9 +28,9 @@ class DirIndex: """Parser for .dirindex files.""" def __init__(self, dirIndexFile): - self.d = [] - self.f = [] - self.t = [] + self.directories = [] + self.files = [] + self.tarballs = [] self.version = 0 self.path = None # will be a VirtualPath instance when set @@ -54,24 +54,19 @@ class DirIndex: tokens = line.split(':') if len(tokens) == 0: continue - - if tokens[0] == "version": + elif tokens[0] == "version": self.version = int(tokens[1]) - elif tokens[0] == "path": # This is relative to the repository root self.path = VirtualPath(tokens[1]) - elif tokens[0] == "d": - self.d.append({ 'name': tokens[1], 'hash': tokens[2] }) - + self.directories.append({'name': tokens[1], 'hash': tokens[2]}) elif tokens[0] == "f": - self.f.append({ 'name': tokens[1], - 'hash': tokens[2], 'size': int(tokens[3]) }) - + self.files.append({'name': tokens[1], + 'hash': tokens[2], 'size': int(tokens[3])}) elif tokens[0] == "t": - self.t.append({ 'name': tokens[1], 'hash': tokens[2], - 'size': int(tokens[3]) }) + self.tarballs.append({'name': tokens[1], 'hash': tokens[2], + 'size': int(tokens[3])}) def _sanityCheck(self): if self.path is None: @@ -81,19 +76,3 @@ class DirIndex: raise InvalidDirIndexFile( "no 'path' field found; the first lines of this .dirindex file " "follow:\n\n" + '\n'.join(firstLines)) - - def getVersion(self): - return self.version - - def getPath(self): - return self.path - - def getDirectories(self): - return self.d - - def getTarballs(self): - return self.t - - def getFiles(self): - return self.f - diff --git a/scripts/python/TerraSync/terrasync/main.py b/scripts/python/TerraSync/terrasync/main.py index 2e46ccf30..fe3141960 100755 --- a/scripts/python/TerraSync/terrasync/main.py +++ b/scripts/python/TerraSync/terrasync/main.py @@ -556,26 +556,26 @@ class TerraSync: def handleDirindexFile(self, dirindexFile): dirIndex = dirindex.DirIndex(dirindexFile) - virtualBase = dirIndex.getPath() # VirtualPath instance + virtualBase = dirIndex.path # VirtualPath instance relativeBase = virtualBase.asRelative() # string, doesn't start with '/' serverFiles = [] serverDirs = [] - for file in dirIndex.getFiles(): + for file in dirIndex.files: f = file['name'] self.processFileEntry(virtualBase / f, join(relativeBase, f), file['hash']) serverFiles.append(f) - for subdir in dirIndex.getDirectories(): + for subdir in dirIndex.directories: d = subdir['name'] self.processDirectoryEntry(virtualBase / d, join(relativeBase, d), subdir['hash']) serverDirs.append(d) - for tarball in dirIndex.getTarballs(): + for tarball in dirIndex.tarballs: # Tarballs are handled the same as normal files. f = tarball['name'] self.processFileEntry(virtualBase / f, diff --git a/scripts/python/TerraSync/tests/test_dirindex.py b/scripts/python/TerraSync/tests/test_dirindex.py index 251d2438d..b01f4203b 100644 --- a/scripts/python/TerraSync/tests/test_dirindex.py +++ b/scripts/python/TerraSync/tests/test_dirindex.py @@ -66,30 +66,10 @@ tarballs_in_sample_dirindex_1 = [ class TestDirIndex(unittest.TestCase): """Unit tests for the DirIndex class.""" - def setUp(self): - self.dirindex = DirIndex(testData("sample_dirindex_1")) - def test_readFrom(self): - self.assertEqual(self.dirindex.version, 1) - self.assertEqual(self.dirindex.path, VirtualPath("some/path")) - self.assertEqual(self.dirindex.d, directories_in_sample_dirindex_1) - self.assertEqual(self.dirindex.f, files_in_sample_dirindex_1) - self.assertEqual(self.dirindex.t, tarballs_in_sample_dirindex_1) - - def test_getVersion(self): - self.assertEqual(self.dirindex.getVersion(), 1) - - def test_getPath(self): - self.assertEqual(self.dirindex.getPath(), VirtualPath("some/path")) - - def test_getDirectories(self): - self.assertEqual(self.dirindex.getDirectories(), - directories_in_sample_dirindex_1) - - def test_getTarballs(self): - self.assertEqual(self.dirindex.getTarballs(), - tarballs_in_sample_dirindex_1) - - def test_getFiles(self): - self.assertEqual(self.dirindex.getFiles(), - files_in_sample_dirindex_1) + d = DirIndex(testData("sample_dirindex_1")) + self.assertEqual(d.version, 1) + self.assertEqual(d.path, VirtualPath("some/path")) + self.assertEqual(d.directories, directories_in_sample_dirindex_1) + self.assertEqual(d.files, files_in_sample_dirindex_1) + self.assertEqual(d.tarballs, tarballs_in_sample_dirindex_1)