1) Написано на бидоне.

Пожалуйста, ну не надо писать на бидоне гуй для линукса. Он в нем и так глючит и тормозит(нет, правда).

Продолжим

Вместо непосредственного разбора опций командной строки рекомендую воспользоваться модулем optparse из стандартной библиотеки. С его помощью легко получишь длинные и короткие опции, информацию о версии и красивый help.

Вместо создания симлинков в bin рекомендовал бы вынести всю логику в пакет и создать коротенький скрипт для запуска как я это сделал в pybackdrop. Логика тут проста: импортируемые библиотеки автоматически компилируются в пайтоновский байткод, а скрипты нет.

Пример тормозных приложений пожалуйста!

Троллюшко ты наш.

по делу есть что сказать? уже в новости высказались чуть ли не все что питон (с ГТК вместе) толст тормознут и не нужен. Но меня он устраивает и я хочу довести код до ума.

Он в нем и так глючит и тормозит(нет, правда)

да ну? почему-то я не замечаю что snapfly тормозит на отрисовке или в работе (правда-правда, не разыгрываю). Так и напрашивается коронный вопрос - что я делаю не так?

Я незнаю насколько будут мои малюсенькие замечания уместны, вопрос скорей стиля.

1. Пожалуйста, заставьте развидеть строки вида (84):
print(«Error in config file: Unknown parameter (»+line+").")

2. Насколько я понял функция IsInt вызывается только из одного места один раз. Зачем выносить тогда в функцию?

3. Уберите это:
config_file.write(data)
config_file.write(data)
config_file.write(data)
config_file.write(data)
config_file.write(data)

и usermenu.write тоже)
лучше уж определите целиком data и запишите в файл (экономьте время):
open(FILE, MODE).write(DATA)

4. множественные print идущих друг за другом тоже уберите)

>1. Пожалуйста, заставьте развидеть строки вида (84):

А чего не так то?

>Так и напрашивается коронный вопрос - что я делаю не так?
Купил себе шестиядерный зион?

Да, тебя он может и устраивает. И, может, его тормознутость не заметна когда работает только 1-2 аппликухи, и на хай-енд системах последних поколений. А глюки заметны только после долгого использования.

И если ты все это делашь «просто так» - т.е. от нефиг делать, или для обучения питону - это пожалуйста, конечно.

Но, блин, серьезно - линуксовый десктоп и так уже невозможно засран разнообразными тормознутыми и глючными говноподелиями на питоне.


А сказать - ну что сказать? Кодревью ты просишь? Зачем? В такого рода программах кода немного, он тупой, и он сводится к дерганью различных фреймворков.

>В такого рода программах кода немного, он тупой, и он сводится к дерганью различных фреймворков.
Следовательно - какая разница, как его писать.

>Следовательно - какая разница, как его писать.

Вот из-за таких как ты и возникают тормоза! А если к делу подходить с толком, то получаются хорошие приложения вроде quodlibet или sk1.

>А если к делу подходить с толком, то получаются хорошие приложения вроде ... sk1.

http://www.linux.org.ru/forum/development/5737396

банально не красиво xD

print("Error in config file: Unknown parameter ("+line+").")

заменить на

print "Error in config file: Unknown parameter (\"%s\")" % line

и вы уж определитесь, пишете на третьем питоне или втором? :)

print() или print?

кстати еще уродства вроде:

line = line.strip('\n')
line = line.strip(' ')
line = line.split('=')

можно заменить на

line = line.strip('\n').strip(' ').split('=')

И дальше что?

Ну решили переписать старый код skencil'а? Они же сами там говорят, что даже сейчас оно работает на уровне (а иногда и быстрее) inkscape.

Ну, еще можно сказать, что это парсер ини файла, который уже есть встроенный. :)

Ну, тащить модуль, да еще и монструозный в маленькую утилиту не стоило бы) Хотелось бы чтоб у авторов не начался ынтэрпрайз головного мозга и они не будут делать из простой утилиты чудовище какое-то :)

Ты думаешь что configparser не потащится самим pygtk или питоном (это стандартный модуль)?
Да и он не такой уж и монструозный, что бы делать самому парсинг.

Мне больше не нравятся вот такие конструкции

    r, g, b = hexcolor[:2], hexcolor[2:4], hexcolor[4:]
    r, g, b = [int(n, 16) for n in (r, g, b)]
    return (float(r)/256, float(g)/256, float(b)/256)

>Ты думаешь что configparser не потащится самим pygtk или питоном (это стандартный модуль)?

считаю что надо экономить там где можно :) Тащить ConfigParser чтоб заменить 6 пусть и велосипедных строчек? Зачем? Чтоб кошерней было? Использовать ConfigParser, чтоб он использовал кучу if,else, регулярные выражения и прочее? Вместо 6 строчек?))) Не смешно :D

>Мне больше не нравятся вот такие конструкции

ой я туда еще не лез)))

Кстати, авторы, почему вы тратите время на двойное присваивание?

    hexcolor = color_hex.strip()
    hexcolor = hexcolor[1:]

    for size in sizelist:
        for extension in extensionlist:
            for category in categorylist:
                if themes:
                    for iconbasecat in themes:
                        iconfile = iconpath+"/"+iconbasecat+'/'+size+'/'+category+'/'+icon+'.'+extension
                        iconimagelist.append(iconfile)
                for iconbasecat in iconbase:
                    iconfile = iconpath+"/"+iconbasecat+'/'+size+'/'+category+'/'+icon+'.'+extension
                    iconimagelist.append(iconfile)

Экономия она разная бывает... особенно когда не экономное решение сделано по стандарту. Боюсь, что мультилайн или что-то подобное в файле (типа rem в качестве комента) сделает работу программы совсем экономной - крах при старте. :)

посмотрел функцию info_desktop из __init__.py

try:
    for
        if
        if
           for
               if
        elif
        elif
            if
            if
                if not
            if
            if
                for
                    for
                        if
                    if
    if
        if
        else
    for
        if
        if
    for
        if
    if
except

Боже, самое слабое место питона: циклы. А тут циклы в циклах в циклах циклов)) Да еще с if'aми и els'ами, и внутри try-except)

>сделает работу программы совсем экономной - крах при старте. :)

Должен быть разумный компромисс. Внятный отчет об ошибке с отсылкой к шаблону позволит не тянуть в систему комбайн ;)

То есть ты предпочитаешь, что ради пары тиков и нескольких строк кода валидный формат превращается в невалидный? Конечному пользователю будет не очень приятно это видеть, даже красиво оформленное. :)

Хотя, каждый хозяин барин. Не хочу доказывать.

line = line.strip('\n ').split('=')

конечному пользователю, лезущему в конфиг будет не особо принципиально) Он и исправить может. :)

Я не ради пары тиков, просто подходить к коду надо верно. Тогда и утилита быстрей будет (в целом). Но если конечно подтирать за пользователем попу, с чересчур многими проверками, разветвлениями, то пожалуй лучше выбрать другой язык. ;)

line = line.strip('\n ').split('=')

спасибо))

>то пожалуй лучше выбрать другой язык.

Какой именно?

>Вместо непосредственного разбора опций командной строки рекомендую воспользоваться модулем optparse из стандартной библиотеки
Deprecated since version 2.7

> Deprecated since version 2.7

development will continue with the argparse module. :)

>Deprecated since version 2.7

Чиорд! Как же меня забавляют эти велосипедисты!

//Я то до сих пор со своей сверхстабильной гентой на 2.6 сижу и не в курсе событий…

Какой именно?

CL наверное...

python-pip + virtualenv помогут :)

>> print(«Error in config file: Unknown parameter (»+line+").")

заменить на

print «Error in config file: Unknown parameter (\»%s\")" % line

А еще лучше на
print(«Error in config file: Unknown parameter (\»{0}\")".format(line))
тем более что скобочки в принте нам как бы намекают на третью версию.

> Боже, самое слабое место питона: циклы. А тут циклы в циклах в циклах циклов)) Да еще с if'aми и els'ами, и внутри try-except)

А что, есть вариант получше? Кстати насчет «циклы в циклах» есть пруф почитать?

>тем более что скобочки в принте нам как бы намекают на третью версию.

а в других местах только print bla-bla-bla намекают на вторую :)

А что, есть вариант получше?

угу, функциональные циклы.

Кстати насчет «циклы в циклах» есть пруф почитать?

что именно хотите почитать? :)

>угу, функциональные циклы.

и пересмотр алгоритма) Другого пути не знаю)

> не надо писать на бидоне гуй для линукса

Писать можно, но «легким» от этого будет только процесс написания, а не сама программа.

>и лаконичней и быстрее работает)

Быстрее, лаконичнее, но менее наглядно. С таким подходом - прямая дорога в Ruby :)

>> угу, функциональные циклы.

и пересмотр алгоритма) Другого пути не знаю)

Насчет пересмотра алгоритмов не спорю, а вот насчет вложенных функциональных циклов в цикле плюс ифы - что-то я сомневаюсь, что будет быстрее. Тем более что кроме автора уже вряд ли кто сможет распарсить код.

ну нет, эти две вещи (функциональные циклы + пересмотр алгоритма) надо в комплексе делать) Одними функциями сильно не ускоришь, но пересмотр алгоритма и замена на функц.циклы по всему коду даст заметный прирост скорости.

Быстрее, лаконичнее, но менее наглядно. С таким подходом - прямая дорога в Ruby :)

не менее :)

«string».method1().method2() - очень наглядно) У руби перед питоном плюсов нету, как в прочем и у питона) Дело в стиле, а тут уж кому как :)

    def process_IN_CREATE(self, event):
        global g_menu
        g_menu.update_menu()
    def process_IN_DELETE(self, event):
        global g_menu
        g_menu.update_menu()

event вообще используется? Надеюсь функцию IsInt вы удалили? А то там тоже переменная number торчит от балды)

def SigUSR1Handler(signum, frame):
    """ grab signal to show """
    global g_menu
    g_menu.callback_signal()
    return

а frame?

и еще не понимаю

from dbus.mainloop.glib import DBusGMainLoop

dbus.mainloop.glib.DBusGMainLoop(set_as_default=True)

import signal

где используется?

функция def find_icon(icon) из файла __init__.py содержит такие строки:

if themes:
    for iconbasecat in themes:

cfg_file = home = os.getenv('HOME') + '/.config/snapfly/usermenu'

в этом нет необходимости, home и так не используется в функции.

if cmd != ' ' and cmd != None and len(cmd)!=0:

Страх Господень))) cmd != None заменить на cmd is not None, len(cmd) != 0 на cmd)))

p = Popen(cmd, shell=True).pid

Присваивания, присваивания. Везде присваивания по коду. Зачем? Вы это потом не используете же!

ui.py:11

from snapfly_core import logINFO

быдлокод) не используется опять)

Зачем так эмоционально? ТС конструктивной критики просил.))) Такое часто бывает, если код рефакторился конкретно, сам из за этого иногда просто модуль с нуля переписываю, если рефакторинг затрагивает больше 30%. Тупо, но получается более опрятный код и без лишних кусков.

cmd != None заменить на cmd is not None

Абсолютно верно!

>>> timeit.Timer('s=None; (s!= None)').timeit()
0.1669459342956543
>>> timeit.Timer('s=None; (s is not None)').timeit()
0.097883939743041992

>iconfile = iconpath+«/»+iconbasecat+'/'+size+'/'+category+'/'+icon+'.'+extension

omg

Естественно тут надо:

iconfile = «%s.%s» % («/».join([iconpath, iconbasecat, size, category, icon]), extension)

Потому что на суммирование кучи строк подряд уходит дохренища ресурсов, а на string format и на объединение массива - нет.

Приличнее всего, конечно, os.path.join(), но, так как программа некроссплатформенна by design, это уже излишне.

Воот, спасибо за отличную конструктивную критику. Именно то, что хотелось :)

>Вместо непосредственного разбора опций командной строки рекомендую воспользоваться модулем optparse из стандартной библиотеки. С его помощью легко получишь длинные и короткие опции, информацию о версии и красивый help.

Немного настораживает, что он deprecated в новом питоне.

Вместо создания симлинков в bin рекомендовал бы вынести всю логику в пакет и создать коротенький скрипт для запуска как я это сделал в pybackdrop. Логика тут проста: импортируемые библиотеки автоматически компилируются в пайтоновский байткод, а скрипты нет.

Спасибо, с этим еще не разбирался. В ближайшем времени обязательно займусь.

>1. Пожалуйста, заставьте развидеть строки вида (84): print(«Error in config file: Unknown parameter (»+line+").")

Про разность в скорости обработки не знал, обязательно поправлю.

2. Насколько я понял функция IsInt вызывается только из одного места один раз. Зачем выносить тогда в функцию?

Теоретически, она сможет пригодиться позже. Да и как-то по привычке вынес.

3. Уберите это:

config_file.write(data) config_file.write(data) config_file.write(data) config_file.write(data) config_file.write(data)

и usermenu.write тоже)

лучше уж определите целиком data и запишите в файл (экономьте время): open(FILE, MODE).write(DATA)

Да, тоже напрягал вид, но было не до этого. Сделаю так в ближайшее время.

4. множественные print идущих друг за другом тоже уберите)

Угу, при очередном сеансе рефакторинга займусь этим.

и вы уж определитесь, пишете на третьем питоне или втором? :)

print() или print?

На самом деле не обращал внимания, старался писать везде print(). Был не в курсе просто насчет того, что в разных питонах разный синтаксис (на 2.6 вроде всё работает)

кстати еще уродства вроде:

line = line.strip('\n') line = line.strip(' ') line = line.split('=') можно заменить на line = line.strip('\n').strip(' ').split('=') и лаконичней и быстрее работает)

Согласен, по моему, лучше выглядит.

Насчет ConfigParser — что-то я не вижу смысла использовать его пока. Пока в конфиге нету ничего очень сложного, думаю, можно и своим костылём парсить.

> своим костылём парсить

key = «value=10», убедительно? :) У тебя это не будет работать. Задолбали, не хотите, не делайте. :)

>Мне больше не нравятся вот такие конструкции

r, g, b = hexcolor[:2], hexcolor[2:4], hexcolor[4:]

r, g, b = [int(n, 16) for n in (r, g, b)]

return (float(r)/256, float(g)/256, float(b)/256)

Мне тоже. Пока до туда не доходили руки еще (это от adeskmenu куски).

Кстати, авторы, почему вы тратите время на двойное присваивание?

hexcolor = color_hex.strip() hexcolor = hexcolor[1:]

И до этого тоже не дотрагивался пока )

ёмаё... Фи-Фи-Фи! понимаю анонимусов кричащих что питон тормозит.

Уже понял, что конкатенация через + у строк тормозит :) Исправлю.

Боже, самое слабое место питона: циклы. А тут циклы в циклах в циклах циклов)) Да еще с if'aми и els'ами, и внутри try-except)

Согласен, несколько монструозно ) Во время ближайшего сеанса рефакторинга подумаю, что можно упростить.

line = line.strip('\n ').split('=')

Вдвойне спасибо, надо будет почитать ман про strio и split в питоне )

def process_IN_CREATE(self, event):

...

def process_IN_DELETE(self, event):

Хм, забыл убрать) event остался от экспериментов.

а frame?

import signal где используется?

Да, тоже случайно остался со времен, когда SIGUSR1 меню показывалось.

и еще не понимаю

from dbus.mainloop.glib import DBusGMainLoop при dbus.mainloop.glib.DBusGMainLoop(set_as_default=True)

Осталось со времен эксперементирования :)

>функция def find_icon(icon) из файла __init__.py содержит такие строки:

Функция будет переписана (пока её не трогал)

cfg_file = home = os.getenv('HOME') + '/.config/snapfly/usermenu'

Ой, это со старых времен осталось, исправлю )

Страх Господень))) cmd != None заменить на cmd is not None, len(cmd) != 0 на cmd)))

Не обращал внимания. Кстати, а разница в скорости есть, или исключительно читабельность? Да, прочитал ниже, что есть... интересно )

Присваивания, присваивания. Везде присваивания по коду. Зачем? Вы это потом не используете же!

На всякий случай :) На самом деле, тоже пока не трогал эту часть, но замечание понял.

быдлокод) не используется опять)

Раньше использовалось :)

Потому что на суммирование кучи строк подряд уходит дохренища ресурсов, а на string format и на объединение массива - нет.

Теперь буду знать.

>key = «value=10», убедительно? :) У тебя это не будет работать. Задолбали, не хотите, не делайте. :)

Ну пока такие значения не планируются же. Как только усложнится, прикручу. Хотя, в порядке эксперимента, можно и сейчас поковырять )

Хотелось бы чтоб у авторов не начался ынтэрпрайз головного мозга и они не будут делать из простой утилиты чудовище какое-то :)

Будет добавляться и дорабатываться только функционал меню, никаких плагинов, никаких сложностей. Иначе оно уже перестанет соответствовать нашим требованиям :)