Refactor two similar functions into one

Question

I'm trying to refactor this two similar functions that I've extracted from the video-diet code. I think the best thing to do would be to create a File class and put the path, type, codec and file manager inside. I'm having difficulty creating the class. I want to indicate in the class that the manager can be None for the audio files.

How would you refactor the two functions into one?

def process_audio(audio, errors_files):
    if get_codec(str(audio)) == 'hevc':
        typer.secho(f'Skipping: {audio}')
        return
    typer.secho(f'Processing: {audio}')
    new_path = destination(audio, True)
    if new_path.exists():
        os.remove(str(new_path))
    try:
        convert_file(str(audio), str(new_path))
        os.remove(str(audio))
        if audio.suffix == new_path.suffix:
            shutil.move(new_path, str(audio))
    except ffmpeg._run.Error:
        typer.secho(f'ffmpeg could not process this file: {str(audio)}', fg=RED)
        errors_files.append(audio)

def process_video(errors_files, manager, video):
    if get_codec(str(video)) != 'hevc':
        typer.secho(f'Skipping: {video}')
        return
    typer.secho(f'Processing: {video}')
    new_path = destination(video, False)
    if new_path.exists():
        os.remove(str(new_path))
    try:
        convert_video_progress_bar(str(video), str(new_path), manager)
        os.remove(str(video))
        if video.suffix == new_path.suffix:
            shutil.move(new_path, str(video))
    except ffmpeg._run.Error:
        typer.secho(f'ffmpeg could not process: {str(video)}', fg=RED)
        errors_files.append(video)

Comments

I think the question is not specific enough. Do you want a class, or would a single function be OK?

---

I was thinking of using a class, but as long as both functions end up being one, anything goes.

Answer 1

class File(object):
    def __init__(self, path: Path, file_type: str):
        self.path = path
        self.file_type = file_type

def process_file(file, manager, codec, errors_files):
    if file.file_type == 'video' and get_codec(str(file.path)) == codec \
            or file.file_type == 'audio' and get_codec(str(file.path)) == codec:
        typer.secho(f'Skipping: {file.path}')
        return

    typer.secho(f'Processing: {file.path}')
    tmp_path = get_tmp_path(file.path, file.file_type == 'audio')

    if tmp_path.exists():
        os.remove(str(tmp_path))

    try:
        if file.file_type == 'video':
            convert_video_progress_bar(str(file.path), str(
                tmp_path), choose_encoder(codec), manager)
        else:
            convert_audio(str(file.path), str(tmp_path), choose_encoder(codec))

        if tmp_path.exists() and file.path.exists and os.stat(str(tmp_path)).st_size <= os.stat(str(file.path)).st_size:
            os.remove(str(file.path))
            if file.path.suffix == tmp_path.suffix:
                shutil.move(tmp_path, str(file.path))
        else:
            if tmp_path.exists() and file.path.exists:
                os.remove(str(tmp_path))

    except ffmpeg._run.Error:
        typer.secho(f'ffmpeg could not process: {str(file.path)}', fg=RED)
        errors_files.append(file.path)

Comments

You don't need a class for only one property, if then you're comparing file.file_type everywhere like that. Might as well just pass it directly to the function

def process_file(type, path, codec, errors_files, manager=None):
    ...

Answer 2

If you are using classes, I think it would be clearer to use multiple sub-classes instead of a single File class, as in your other answer.

# NOTE: This is untested code. I've merely rearranged the code
#       in your question for the sake of writing an example

class File():
    def _process(self, new_path):
        if new_path.exists():
            os.remove(str(new_path))
        
        try:
            self._convert(new_path)
            os.remove(str(self.media))
            if media.suffix == new_path.suffix:
                shutil.move(new_path, str(media))
        except ffmpeg._run.Error:
            errors_files.append(self.media)

class Audio(File):
    def __init__(self, audio, errors_files):
        self.media = audio
        self.errors_files = errors_files
    
    def _convert(self, new_path):
        convert_audio(str(self.media), str(new_path))
    
    def process(self):
        if get_codec(self.media) == 'hevc':
            return
        
        new_path = destination(self.media, True)
        super()._process(new_path)

class Video(File):
    def __init__(self, video, manager, errors_files):
        self.media = video
        self.manager = manager
        self.errors_files = errors_files
    
    def _convert(self, new_path):
        convert_video(str(self.media), str(new_path), self.manager)
    
    def process(self):
        if get_codec(self.media) != 'hevc':
            return
        
        new_path = destination(self.media, False)
        super()._process(new_path)

def process_file(file):
    file.process()

# Usage
some_file = Audio(...)
process_file(some_file)

# Alternative usage
Audio(...).process()