6
\$\begingroup\$

I wrote a random file opener in Python that takes file extensions as a tuple at code level, and directory/amount at console level:

import os
import random
import sys

class RandomFileOpener():

    def __init__(self, positives, file_exts):
        self.file_list_complete = []
        self.amount_to_open = 0 #So it can be checked if there's an existing entry
        self.selected_files = []
        self.positives = positives
        self.file_exts = file_exts
        self.open = False
        self.begin()

    def generate_file_array(self, directory):
        array = []
        for root, directories, files in os.walk(directory):
            for filename in files:
                if filename.lower().endswith(self.file_exts):
                    array.append(os.path.join(root, filename))
        return array            

    def random_choice(self, images, amount):
        return [random.choice(images) for i in range(0, len(images) if len(images) < amount else amount)]

    def open_files(self, selected_files):
        for file in selected_files:
            os.system("open " + file.replace(' ', '\ '))

    def begin(self):
        while True:
            if len(self.file_list_complete) == 0:
                self.file_list_complete = self.generate_file_array(str(input("Enter directory address: ")))
            if self.amount_to_open == 0:
                self.amount_to_open = int(input("Enter the amount of files you want: "))
            if len(self.selected_files) == 0:
                self.selected_files = self.random_choice(self.file_list_complete, self.amount_to_open)          
            if str(input("Do you want to open them? ").lower()) not in positives:
                print([file for file in self.selected_files])
            else:
                self.open_files(self.selected_files)

            if str(input("Do you want to do another? ").lower()) not in positives:
                sys.exit()
            self.begin()

positives = ["y", "yes", "sure"]
image_file_exts = ('.png', '.jpg', '.jpeg', '.gif', '.webm')
image_opener = RandomFileOpener(positives, image_file_exts)
\$\endgroup\$
0
6
\$\begingroup\$

Did you mean to include a call to self.begin at the end of your while loop in self.begin?

def begin(self):
    while True:
        ...
        self.begin() # <-- Ruh-roh shaggy?

The while loop should already repeat the code contained in it, so the call to self.begin would only add more recursion-related issues into the mix.

The call to str here is redundant:

str(input("Do you want to do another? ").lower())

If you're using Python 3.x, as I assumed from looking at your print statements, then you don't need to convert user input to a string. User input is already in a string format when it's obtained. The above code can simply become this:

input("Do you want to do another? ").lower()

In addition, there's no need for the parentheses when declaring a class in Python, as you've done here:

class RandomFileOpener():

It can simply be declared like this:

class RandomFileOpener:

In older versions of Python, the parentheses, plus object inside the parentheses were required though in order to have a new-style class. In newer versions of Python though, all classes inherit from object by default.

\$\endgroup\$
3
  • \$\begingroup\$ Yes, I did mean to add the call at the end. That's mainly why I store the variables. \$\endgroup\$ – Quill Dec 15 '15 at 23:18
  • \$\begingroup\$ @Quill-HATMANIAC Huh? I'm not sure what you're trying to say. I ran your code without the recursive call to begin and it worked fine for me. \$\endgroup\$ – Ethan Bierlein Dec 15 '15 at 23:21
  • \$\begingroup\$ Oh, I see. Right. The while loop does that instead. \$\endgroup\$ – Quill Dec 15 '15 at 23:22
5
\$\begingroup\$

Checking things

Prefer just if lst: to if len(lst) == 0. Prefer if not self.amount_to_open to comparing against 0.

Recursive and Iterative

begin() is both recursive and iterative. Pick one - iterative is good enough. Just have it loop until done:

def begin(self):
    while True:
        # there's a break somewhere in here, but no self.begin()

Naming

begin() doesn't just start doing something. It does everything - which is pretty unintuitive to say the least.

Random choice

This:

return [random.choice(images) for i in range(0, len(images) if len(images) < amount else amount)]

Is the same as:

return [random.choice(images) for i in range(0, min(len(images), amount))]

But could pick the same image lots of times. If you really want to sample, you'll want:

return random.sample(images, min(len(images), amount))

or:

try:
    return random.sample(images, amount)
except ValueError:
    return images
\$\endgroup\$
1
  • \$\begingroup\$ If begin does everything run would probably be a better name. \$\endgroup\$ – SuperBiasedMan Dec 17 '15 at 14:59

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.