ソースコードに書いてあると嬉しい情報 (PythonのFAT32実装を例に)

ソースコードに記述されていると嬉しい情報をまとめてみます😊
リファクタリングというより公開/送付前の情報追加的なお話です。
ロジックを変更しなくても読み手に優しいコードになるよ！　というのが趣旨です。

サンプルはPythonでファイルシステム(FAT32^[1])の一部機能を実装しているClassとします。

実装機能^[2]

• ファイル作成
• ディレクトリ作成
• ディレクトリ内エントリ表示
• ディレクトリ移動

class FAT32FileSystem
    def __init__(self, total_clusters=4096):
        (省略)

    def allocate_cluster(self):
        (省略)

    def create_file(self, name, size=0):
        (省略)
       
    def create_directory(self, name):
        (省略)

    def list_directory(self):
        (省略)

    def change_directory(self, name):
        (省略)

型情報 (or docstring)

ダックタイピングの手軽さがなくなる！と、型情報を入れることに異を唱える方もいらっしゃると思います。しかし、集団での開発効率向上のためには型情報はあったほうがいいです😊

• 変数名だけでは伝わらない変数の情報を伝えられる。
  例えば、starting_cluster という変数名だけでは、初期クラスタのインデックスを格納する数値型なのか、クラスタ全般の情報を格納するクラスインスタンスなのかが分かりません。
• IDEによる自動補完の恩恵を受けられる。
  
• 関数シグネチャだけで関数のI/Oを把握できる。

    def create_file(self, name: str, size: int = 0) -> None:
  

  これは、docstringでもいいですね。

誤った型付けは可読性をむしろ悪化させてしまいます。Pythonの型は実行時には作用しないため、誤った型付けをしても動作には影響しません。mypyなどによる型チェックが必須です。

Before

    def create_file(self, name, size=0):
        if name in self.current_dir['entries']:
            print(f"Error: File '{name}' already exists.")
            return
        starting_cluster = self.allocate_cluster()
        file_entry = {
            'name': name,
            'size': size,
            'is_directory': False,
            'starting_cluster': starting_cluster,
            'parent': self.current_dir
        }
        self.current_dir['entries'][name] = file_entry
        print(f"File '{name}' created with starting cluster {starting_cluster}")

After

    def create_file(self, name: str, size: int = 0) -> None:
        if name in self.current_dir['entries']:
            print(f"Error: File '{name}' already exists.")
            return
        starting_cluster: int = self.allocate_cluster()
        file_entry: FileEntry = {
            'name': name,
            'size': size,
            'is_directory': False,
            'starting_cluster': starting_cluster,
            'parent': self.current_dir
        }
        self.current_dir['entries'][name] = file_entry
        print(f"File '{name}' created with starting cluster {starting_cluster}")

関数呼び出し階層

以下が関数呼び出し階層です。例えば、allocate_clusterという関数はcreate_fileとcreate_directoryの２つの関数から呼び出されいることが分かります。

この情報があることによって、関数の粒度や影響範囲が分かりやすくなります😊。また、開発者側視点としては、例えば、allocate_clusterをprivateメンバ__allocate_clusterに変更し忘れていたということに気付けるかもしれません。

class FAT32FileSystem:
    '''
    A call hierarchy of the methods in the FAT32FileSystem class:
   
    FAT32FileSystem
    ├── __init__(self, total_clusters: int = 4096) -> None
    ├── create_file(self, name: str, size: int = 0) -> None
    │   └── allocate_cluster(self) -> int
    ├── create_directory(self, name: str) -> None
    │   └── allocate_cluster(self) -> int
    ├── list_directory(self) -> None
    ├── change_directory(self, name: str) -> None
    └── allocate_cluster(self) -> int
    '''

    def __init__(self, total_clusters: int = 4096) -> None:
        (省略)

VSCodeのOutlineでも関数/変数の概観は確認できます。呼び出し階層まで手軽に見れるといいんですけどね。

class内のサンプル実行エントリ ( or テストコード )

Pythonのような動的型付け言語では、class実装だけでは使い方が伝わりにくいためclass内のサンプル実行用のmain関数があるのが望ましいです😊
class毎に記述するのが最良ですが、時間との相談ですね。

型情報がある場合であっても、サンプル実行エントリは欲しいです。
例えば、classの利用者がプログラムの暗黙的な実行制約を知らずに異常終了に遭遇するとします。その場合に、サンプル実行エントリがあれば、その制約を推察することができます。
例えば、numpy.arrayのshapeなどの型記述が難しいものが実行制限のトリガーになっている場合があげられるでしょう。

class内のサンプル実行エントリ

class FAT32FileSystem:
        (省略)

# ↓　これ
if __name__ == "__main__":
    fs = FAT32FileSystem()
    fs.create_file('file1.txt')
    fs.create_directory('dir1')
    fs.list_directory()
    fs.change_directory('dir1')
    fs.create_file('file_in_dir.txt')
    fs.list_directory()
    fs.change_directory('..')
    fs.list_directory()

普通じゃない場所へのコメント

コードをご覧ください。

Before

    def allocate_cluster(self) -> int:
        for i in range(2, self.total_clusters):
            if self.fat[i] == 0x00000000:
                self.fat[i] = 0x0FFFFFFF
                self.clusters[i] = []
                return i
        raise Exception("No free clusters available")

何が「普通じゃない」なのかは個人の感覚によるところが大きいと思います。私の場合ですが、 for i in range(2, self.total_clusters): が 2 から開始する点が普通じゃないと思いますので、ここにコメントを打ちたくなります。

After

    def allocate_cluster(self) -> int:
        # First cluster is reserved for root directory. So start from 2.
        for i in range(2, self.total_clusters):
            if self.fat[i] == 0x00000000:
                self.fat[i] = 0x0FFFFFFF
                self.clusters[i] = []
                return i
        raise Exception("No free clusters available")

最後に

これら情報はcopilotなどを使うとロジックに変更を加えずに短時間で記述できるかと思います。
設計が上出来じゃないなぁ。。というときでも、まだ諦めてはいけません。
適切な情報を加筆して、読み手に優しいコードを！

補遺

Before　コード全体

class FAT32FileSystem:
    def __init__(self, total_clusters=4096):
        self.total_clusters = total_clusters
        self.fat = [0x00000000] * self.total_clusters
        self.clusters = [None] * self.total_clusters  
        self.root_dir = {'name': '/', 'entries': {}, 'is_directory': True, 'starting_cluster': None, 'parent': None}
        self.current_dir = self.root_dir

    def allocate_cluster(self):
        for i in range(2, self.total_clusters):  
            if self.fat[i] == 0x00000000:
                self.fat[i] = 0x0FFFFFFF
                self.clusters[i] = []
                return i
        raise Exception("No free clusters available")

    def create_file(self, name, size=0):
        if name in self.current_dir['entries']:
            print(f"Error: File '{name}' already exists.")
            return
        starting_cluster = self.allocate_cluster()
        file_entry = {
            'name': name,
            'size': size,
            'is_directory': False,
            'starting_cluster': starting_cluster,
            'parent': self.current_dir
        }
        self.current_dir['entries'][name] = file_entry
        print(f"File '{name}' created with starting cluster {starting_cluster}")

    def create_directory(self, name):
        if name in self.current_dir['entries']:
            print(f"Error: Directory '{name}' already exists.")
            return
        starting_cluster = self.allocate_cluster()
        dir_entry = {
            'name': name,
            'entries': {},
            'is_directory': True,
            'starting_cluster': starting_cluster,
            'parent': self.current_dir
        }
        self.current_dir['entries'][name] = dir_entry
        print(f"Directory '{name}' created with starting cluster {starting_cluster}")

    def list_directory(self):
        entries = self.current_dir['entries']
        for name, entry in entries.items():
            type_str = 'DIR ' if entry['is_directory'] else 'FILE'
            print(f"{type_str:<5} {name}")

    def change_directory(self, name):
        if name == '..':
            if self.current_dir['parent'] is not None:
                self.current_dir = self.current_dir['parent']
                print(f"Changed directory to '{self.current_dir['name']}'")
            else:
                print("Already at root directory")
            return
        entries = self.current_dir['entries']
        if name in entries and entries[name]['is_directory']:
            self.current_dir = entries[name]
            print(f"Changed directory to '{name}'")
        else:
            print(f"No such directory: '{name}'")

After　コード全体

from __future__ import annotations
from typing import List, Dict, Optional, Union, Any, TypedDict, cast

class FileEntry(TypedDict):
    name: str
    size: int
    is_directory: bool
    starting_cluster: int
    parent: DirectoryEntry

class DirectoryEntry(TypedDict):
    name: str
    entries: Dict[str, Union[FileEntry, DirectoryEntry]]
    is_directory: bool
    starting_cluster: Optional[int]
    parent: Optional[DirectoryEntry]

class FAT32FileSystem:
    '''
    A call hierarchy of the methods in the FAT32FileSystem class:
   
    FAT32FileSystem
    ├── __init__(self, total_clusters: int = 4096) -> None
    ├── create_file(self, name: str, size: int = 0) -> None
    │   └── allocate_cluster(self) -> int
    ├── create_directory(self, name: str) -> None
    │   └── allocate_cluster(self) -> int
    ├── list_directory(self) -> None
    ├── change_directory(self, name: str) -> None
    └── allocate_cluster(self) -> int
    '''

    def __init__(self, total_clusters: int = 4096) -> None:
        self.total_clusters: int = total_clusters
        self.fat: List[int] = [0x00000000] * self.total_clusters
        self.clusters: List[Optional[List[Any]]] = [None] * self.total_clusters
        self.root_dir: DirectoryEntry = {
            'name': '/',
            'entries': {},
            'is_directory': True,
            'starting_cluster': None,
            'parent': None
        }
        self.current_dir: DirectoryEntry = self.root_dir

    def allocate_cluster(self) -> int:
        # First cluster is reserved for root directory. So start from 2.
        for i in range(2, self.total_clusters):
            if self.fat[i] == 0x00000000:
                self.fat[i] = 0x0FFFFFFF
                self.clusters[i] = []
                return i
        raise Exception("No free clusters available")

    def create_file(self, name: str, size: int = 0) -> None:
        if name in self.current_dir['entries']:
            print(f"Error: File '{name}' already exists.")
            return
        starting_cluster: int = self.allocate_cluster()
        file_entry: FileEntry = {
            'name': name,
            'size': size,
            'is_directory': False,
            'starting_cluster': starting_cluster,
            'parent': self.current_dir
        }
        self.current_dir['entries'][name] = file_entry
        print(f"File '{name}' created with starting cluster {starting_cluster}")

    def create_directory(self, name: str) -> None:
        if name in self.current_dir['entries']:
            print(f"Error: Directory '{name}' already exists.")
            return
        starting_cluster: int = self.allocate_cluster()
        dir_entry: DirectoryEntry = {
            'name': name,
            'entries': {},
            'is_directory': True,
            'starting_cluster': starting_cluster,
            'parent': self.current_dir
        }
        self.current_dir['entries'][name] = dir_entry
        print(f"Directory '{name}' created with starting cluster {starting_cluster}")

    def list_directory(self) -> None:
        entries: Dict[str, Union[FileEntry, DirectoryEntry]] = self.current_dir['entries']
        for name, entry in entries.items():
            type_str: str = 'DIR ' if entry['is_directory'] else 'FILE'
            print(f"{type_str:<5} {name}")

    def change_directory(self, name: str) -> None:
        if name == '..':
            if self.current_dir['parent'] is not None:
                self.current_dir = self.current_dir['parent']
                print(f"Changed directory to '{self.current_dir['name']}'")
            else:
                print("Already at root directory")
            return
        entries: Dict[str, Union[FileEntry, DirectoryEntry]] = self.current_dir['entries']
        if name in entries:
            entry = entries[name]
            if entry['is_directory']:
                self.current_dir = cast(DirectoryEntry, entry)
                print(f"Changed directory to '{name}'")
            else:
                print(f"No such directory: '{name}'")
        else:
            print(f"No such directory: '{name}'")


if __name__ == "__main__":
    fs = FAT32FileSystem()
    fs.create_file('file1.txt')
    fs.create_directory('dir1')
    fs.list_directory()
    fs.change_directory('dir1')
    fs.create_file('file_in_dir.txt')
    fs.list_directory()
    fs.change_directory('..')
    fs.list_directory()

脚注

1. FAT32は、世界で最も広く使用されているファイルシステムの一つです。一般にファイルシステムでは、1)物理的なメモリブロックとファイルの対応、2)ファイルとディレクトリの階層構造を管理します。FAT32では、物理的なメモリブロックと同等の単位として「クラスター」という言葉を導入しています。このクラスターをファイルアロケーションテーブル（FAT）というテーブルに記録・追跡することによって、先ほどの２つ関係の管理を実現しています。詳細はこちらが詳しいです。

2. 本実装はイメージ理解用の超簡易実装です。例えば、ファイルサイズを0としています。実際には複数のクラスタにまたがるファイルをテーブルを利用して記録できるようにする必要があります。