Правильно ли я работаю с ООП?

Question

[Alexander]

Пишу свою библиотеку для работы с изображениями. Пока ещё не все функции дописал.
Хотелось бы услышить критику у проф.разработчиков по своему коду , может где что не так или где что проще сделать можно было бы, так как я новичек
https://gist.github.com/kentuck13/2079114d28fe4019...
Вызов Upload.save_files(image, 'product', new_obj.id)

Answer 1

[JRazor]

Небольшие замечания по коду:
1) path = Upload.get_path(folder_name, id)
Вы немного не так используете вызовы класса внутри его же самого. Почитайте про self:
path = self.get_path(folder_name, id)
2) Зачем вам столько @staticmethod? Убирайте все, кроме get_path, подставляйте везде self и читайте первое замечание.
3) from poliv.settings. А чего не from poliv.nastroiki?
4) get_files, return images. Читайте про yield
5) Используйте хотя бы цифры для format, а лучше именуйте:

"""<img src="{image}" style="max-width: 250px; width:100%" class="{class}">""".format(
    image=images_path + file, class=i_class)

И да, зачем вам тройные кавычки?
6) Если хотите, чтобы ваш класс работал не только на вашей системе используйте os.path.join вместо +

Comments

[Alexander]

Спасибо,
да , я читал про экземпляр класса self но тут штука вот вчем:
допустим есть простой метод , у которой первый обязательный параметр это экземпляр класса - self. Так как я вот так вызываю метод Upload.save_files(image, 'product', new_obj.id) # первый параметр это картинки , второй параметр это название папки где будет храниться картинка , третий это id папки, будет вот так product/1/my_image.png
сам код save_files

@staticmethod
    def save_files(files, folder_name, id):
        id = str(id)
        path = Upload.get_path(folder_name, id)
        if not os.path.exists(path):
            os.makedirs(path)
        for image in files:
            extensions = str(image).split('.')
            millis = str(round(time.time() * 1000))
            def process(f):
                with open(path + '/' + millis + '.' + extensions[-1],
                          'wb+') as destination:
                    for chunk in f.chunks():
                        destination.write(chunk)

            process(image)

из за этого я сделал save_files статичным.

[JRazor]

Alexander: Это не важно. Просто уберите staticmethods и добавьте self. Оно так и должно быть

[JRazor]

Alexander:

def save_files(self, files, folder_name, id):
    id = str(id)
    path = self.get_path(folder_name, id)

[JRazor]

Alexander: extensions = str(image).split('.') можно заменить на _, extension = str(image).split('.') и сразу получать расширение файла

[Alexander]

JRazor:

Вызов метода:
print('///////////////////')
print(new_obj.id) # = 73
print('///////////////////')
Upload.save_files(request.FILES.getlist('image'), 'product', new_obj.id)
сам метод:
def save_files(self, files, folder_name, id):
        id = str(id)
        path = self.get_path(folder_name, id)
.......

Ошибка на сайте save_files() missing 1 required positional argument: 'id'

[JRazor]

Alexander: Прочитайте еще раз второй пункт и замените все Upload внутри класса на self.

[Alexander]

JRazor: вы не совсем правильно поняли меня, я заменил все.
Это вызов метода в admin.py
Upload.save_files(request.FILES.getlist('image'), 'product', new_obj.id)

[Alexander]

admin.py
from poliv.Libary.Upload import Upload
Upload.save_files(request.FILES.getlist('image'), 'product', new_obj.id)
Upload.py

class Upload:
    def save_files(self, files, folder_name, id):
        id = str(id)
        path = self.get_path(folder_name, id)
        if not os.path.exists(path):
            os.makedirs(path)
        for image in files:
            extensions = str(image).split('.')
            millis = str(round(time.time() * 1000))
            with open(os.path.join(path, millis + '.%s' % extensions[-1]),
                      'wb+') as destination:
                for chunk in image.chunks():
                    destination.write(chunk)

[Alexander]

JRazor: а вот ошибка на сайте save_files() missing 1 required positional argument: 'id'

[JRazor]

Alexander: Это все потому, что вы не умеете использовать классы. Используйте так:

upload = Upload(); upload..save_files(request.FILES.getlist('image'), 'product', new_obj.id)

[JRazor]

Alexander: Вместо двух точек - одна, разумеется.

[JRazor]

Alexander: Ну или используйте скобки просто: Upload().save_files(request.FILES.getlist('image'), 'product', new_obj.id). Но лучше первый вариант.

[Alexander]

JRazor: Спасибо , понял.