LINUX.ORG.RU

Хотелось бы узнать Ваше мнение по поводу читаемости кода

 


0

1

Прошлый вариант


...
from snmp.models import MIBList, MIBObject, Hosts, SupportedOid
...

class MIBBaseClass(object):
    def __init__(self):
        self.ListMIBModel = MIBList #Модель со всеми MIBами
        self.ObjectMIBModel = MIBObject #Модель со всеми MIB объектами
        self.HostModel = Hosts #Модель с параметрами подключения к хостам
        self.SupportedOidModel = SupportedOid #Модель с поддерживаемыми oid для конкретного хоста
        self.results = {}
        self.status = ""
        def results(self):
            return self.results
        def status(self):
            return self.status

... 


class OidSupportedClass(MIBBaseClass):
    def get(self):
        bulk = []
        for host in self.HostModel.objects.all():
            conn = {"DestHost": host.host, "Community": host.community, "Version": host.version}
            [bulk.append(self.SupportedOidModel(host=host, oid=oiditem)) for oiditem in self.ObjectMIBModel.objects.all() if netsnmp.snmpwalk(oiditem.name, **conn)]
            """for oiditem in self.ObjectMIBModel.objects.all():
                if netsnmp.snmpwalk(oiditem.name, **conn):
                    bulk.append(self.SupportedOidModel(host=host, oid=oiditem))
            """
        self.results = bulk
    def save(self):
        bulk = [item for item in self.results if not self.SupportedOidModel.objects.filter(host__id=item.host.id, oid__id=item.oid.id).exists()]
        self.SupportedOidModel.objects.bulk_create(bulk)

Новый вариант

import netsnmp
import json
from pysmi.reader import HttpReader
from pysmi.writer import CallbackWriter
from pysmi.parser.smi import SmiV2Parser
from pysmi.codegen.jsondoc import JsonCodeGen
from pysmi.compiler import MibCompiler


from snmp.models import MIBList, MIBObject, Hosts, SupportedOid

import logging
logger = logging.getLogger(__name__)

class MIBBaseClass(object):
    def __init__(self):
        self.ListMIBModel = MIBList #Модель со всеми MIBами
        self.ObjectMIBModel = MIBObject #Модель со всеми MIB объектами
        self.HostModel = Hosts #Модель с параметрами подключения к хостам
        self.SupportedOidModel = SupportedOid #Модель с поддерживаемыми oid для конкретного хоста
        self.results = {}
        self.status = ""

class OidSupportedClass(MIBBaseClass):
    def get(self):
        bulk = []
        for host in self.HostModel.objects.all():
            conn = {"DestHost": host.host, "Community": host.community, "Version": host.version}
            for oiditem in self.ObjectMIBModel.objects.all():
                if netsnmp.snmpwalk(oiditem.name, **conn):
                    bulk.append(self.SupportedOidModel(host=host, oid=oiditem))
        self.results = bulk
    def save(self):
        def exists_indb(item):
            return self.SupportedOidModel.objects.filter(
                host__id=item.host.id,
                oid__id=item.oid.id
            ).exists()
        bulk = [item for item in self.results if not exists_indb(item)]
        self.SupportedOidModel.objects.bulk_create(bulk)
class MIBParserClass(MIBBaseClass):
    """
    Парсинг MIB файлов с сайта mibs.snmplabs.com/asn1/
    TODO: Вынести куданибудь self.http_sources
    """
    def __init__(self):
        super(MIBParserClass, self).__init__()
        self.http_sources = [('mibs.snmplabs.com', 80, '/asn1/@mib@')]
    def get(self):
        def _get_result(mib_name, mib_objects, cbCtx):
            self.results[mib_name] = json.loads(mib_objects)
        mib_parser = MibCompiler(SmiV2Parser(), JsonCodeGen(), CallbackWriter(_get_result))
        mib_parser.addSources(*[HttpReader(*item) for item in self.http_sources])
        self.status = mib_parser.compile(
            *{item.name for item in self.ListMIBModel.objects.filter(compiled=False)}
        )
    def save(self):
        def exists_in_miblist(mib_name):
            return self.ListMIBModel.objects.filter(name=mib_name).exists()
        def exists_in_mibobjects(item):
            return self.ObjectMIBModel.objects.filter(name=item.name).exists()
        bulk_listmib = [self.ListMIBModel(name=item) for item in self.results.keys() if not exists_in_miblist(item)]
        self.ListMIBModel.objects.bulk_create(bulk_listmib)
        bulk_mibobjects = []
        for mib_name, mib_objects in self.results.items():
            self.ListMIBModel.objects.filter(name=mib_name).update(compiled=True)
            for item in mib_objects.values():
                values = {"mib": self.ListMIBModel.objects.get(name=mib_name),
                          "typeclass": item.get("class"),
                          "maxaccess": item.get("maxaccess"),
                          "name": item.get("name"),
                          "nodetype": item.get("nodetype"),
                          "oid": item.get("oid"),
                          "status": item.get("status"),
                          "syntax": item.get("syntax"),
                         }
                if item.get("oid") and item.get("syntax"):
                    bulk_mibobjects.append(self.ObjectMIBModel(**values))
        bulk_mibobjects = [item for item in bulk_mibobjects if not exists_in_mibobjects(item)]
        self.ObjectMIBModel.objects.bulk_create(bulk_mibobjects)



Последнее исправление: dcopm999 (всего исправлений: 2)

def results(self): return self.results

list.append внутри list comprehension

К черту читаемость, что за бред я наблюдаю?

t184256 ★★★★★
()

Генератор списка выкинь и никогда так больше не делай. Код после него, упрятанный в док-строку — нормальный, кошерный и читаемый. А уж расширяется вообще на ура по сравнению с кошмаром в генераторе списка.

Virtuos86 ★★★★★
()
[bulk.append(self.SupportedOidModel(host=host, oid=oiditem)) for oiditem in self.ObjectMIBModel.objects.all() if netsnmp.snmpwalk(oiditem.name, **conn)]
...
bulk = [item for item in self.results if not self.SupportedOidModel.objects.filter(host__id=item.host.id, oid__id=item.oid.id).exists()]

слишком уж длинно, и сложно читать эти кусочки, может стоит как-то разбить на доп функции либо вообще выкинуть, их лучше использовать в самых простых случаях.

Int64 ★★★
()
Ответ на: комментарий от dcopm999

append внутри генератора это жесть, во втором случае, если очень хочется, можно написать так:

def save(self):
    def exists(item):
        return self.SupportedOidModel.objects\
            .filter(host__id=item.host.id, oid__id=item.oid.id)\
            .exists()

    bulk = [item for item in self.results if not exists(item)]
    self.SupportedOidModel.objects.bulk_create(bulk)

Int64 ★★★
()
Ответ на: комментарий от dcopm999

либо можно еще filter сделать:

bulk = filter(not_exists, self.results)

Int64 ★★★
()
Последнее исправление: Int64 (всего исправлений: 1)
        self.results = {}
        self.status = ""
        def results(self):
            return self.results
        def status(self):
            return self.status
            

Если ты хочешь сделать геттеры/сеттеры, то это правильно делать так:

# Python >= 2.6
class Obj(object):
    results = property()
    status = property()

    def __init__(self):
        self._results = {"default_result": None}
        self._status = "default_status"

    @results.getter
    def results(self):
        """This is `self.results`."""
        return self._results

    @results.setter
    def results(self, value):
        self._results = value

    @results.deleter
    def results(self):
        self._results = {}

    @status.getter
    def status(self):
        """This is `self.status`."""
        return self._status

    @status.setter
    def status(self, value):
        self._status = value

    @status.deleter
    def status(self):
        self._status = 'No more'

if __name__ == '__main__':
    obj = Obj()
    print obj.results
    print obj.status
    obj.results = {"some_result": 1}
    obj.status = "some status"
    print obj.results
    print obj.status
    del obj.results
    del obj.status
    print obj.results
    print obj.status

# ==> {'default_result': None}
#     default_status
#     {'some_result': 1}
#     some status
#     {}
#     No more

Virtuos86 ★★★★★
()
Ответ на: комментарий от crutch_master

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

Virtuos86 ★★★★★
()
Последнее исправление: Virtuos86 (всего исправлений: 1)
Ответ на: комментарий от crutch_master

можно, просто не писать геттеры и сетеры, зачем они в подобных случаях? Если вдруг понадобится расширить поведение на гет или сет для определенного поля, то для него в будущем можно легко сделать один геттер или сеттер, я еще могу понять зачем для Java такое делают (хоть и смысла тоже особого нету в большинстве случаев), там так нельзя, а в питоне зачем?

Int64 ★★★
()
Ответ на: комментарий от Int64

Мне интересны его возможности в метапрограммировании. Вот, я, например, на js накидал:

#!/usr/bin/js
meta = function(fields) {
    for (i in fields) {
        var f = fields[i];
        var self = this;
        this[f] = null;
        this["get"+f] = function(fi) {return function() {return self[fi];}}(f);
        this["set"+f] = function(fi) {return function(v) {self[fi]=v;}}(f);
    }
}
function log(s) {console.log(s);}
var m = new meta(["foo", "bar", "baz"]);
log(m.getfoo());
log(m.getbar());
log(m.getbaz());
m.setfoo(5);
m.setbar(3);
m.setbaz(1);
log(m.getfoo());
log(m.getbar());
log(m.getbaz());
//null
//null
//null
//5
//3
//1
Как это будет выглядеть на питоне?

crutch_master ★★★★★
()
Последнее исправление: crutch_master (всего исправлений: 1)
self.results = [self.SupportedOidModel(host=host, oid=oiditem)
                     for host in self.HostModel.objects.all()
                         for oiditem in self.ObjectMIBModel.objects.all()
                             if netsnmp.snmpwalk(oiditem.name,
                                    DestHost = host.host,
                                    Community = host.community,
                                    Version = host.version)]
aedeph_ ★★
()
Ответ на: комментарий от crutch_master
class AvoidBoilerplateMeta(type):
    def __new__(cls, name, bases, attrs):
        for (attr, obj) in tuple(attrs.items()):
            if not attr.startswith("_") and isinstance(obj, property):
                def fget(self, a=attr):
                    return getattr(self, "_" + a)
                def fset(self, value, a=attr):
                    setattr(self, "_" + a, value)
                def fdel(self, a=attr):
                    delattr(self, "_" + a)
                attrs[attr] = property(fget, fset, fdel)
        return super(AvoidBoilerplateMeta, cls).__new__(cls, name, bases, attrs)

class Obj(object):
    __metaclass__ = AvoidBoilerplateMeta
    results = property()
    status = property()

    def __init__(self):
        self._results = {"default_result": None}
        self._status = "default_status"

if __name__ == '__main__':
    obj = Obj()
    print obj.results
    print obj.status
    obj.results = {"some_result": 1}
    obj.status = "some status"
    print obj.results
    print obj.status
    del obj.results
    del obj.status
    print obj.results # FAIL: AttributeError: 'Obj' object has no attribute '_results'
    print obj.status # FAIL: AttributeError: 'Obj' object has no attribute '_status'

Не уверен, что реализация наилучшая, но работает :-)

Сообщение об ошибке («AttributeError: …») можно сделать понятнее, но в рамках PoC реализации и так сгодится.

Virtuos86 ★★★★★
()
Последнее исправление: Virtuos86 (всего исправлений: 3)
Ответ на: комментарий от Virtuos86

Ну, понятно, я тоже не гуру js'а. В питоне геттер/сеттер назначается и дергается, когда присваиваешь что-то? Что-то он сложный какой-то. В js всё просто и понятно. Json + функции, как тип данных, ну и путаница с ссылками, == и ===.

crutch_master ★★★★★
()

Если ты задаешься вопросом, как оформлять свой питон код, то самое время посмотреть pep8

Aswed ★★★★★
()

Сегодня попытался освоить python.
Не привычно)
В самом начале начались сыпаться ошибки синтаксиса.
Потом cо временем вроде освоился.

tyamur ★★
()
Последнее исправление: tyamur (всего исправлений: 1)

80 символов по горизонтали или в печь.

d_a ★★★★★
()

Хотелось бы узнать Ваше мнение по поводу читаемости кода

1. Открой этот файлик в Пичарм и нажми ctrl+alt+L, будет тебе pep-8

2. Убери нахрен длинные однострочники и прочие понты. Они не предназначены для того чтобы пихать туда длинные конструкции

Siado ★★★★★
()
Ответ на: комментарий от crutch_master

В js всё просто и понятно

Это потому что ты с большими проектами не работал. Как будешь работать будет тебе колбек через колбек на колбеке колбеком погоняет, вот тогда и будет тебе понятно.

Siado ★★★★★
()
Ответ на: комментарий от Siado

Это потому что ты с большими проектами не работал. Как будешь работать будет тебе колбек через колбек на колбеке колбеком погоняет, вот тогда и будет тебе понятно.

У меня больше опасений вызывает инфраструктура. Хоть мне js и нравится, но что-то серьёзное делать, скорее всего, я на нём никогда не буду.

колбек на колбеке

Это больше проблема хренового проектирования.

crutch_master ★★★★★
()
Ответ на: комментарий от crutch_master

Ну, понятно, я тоже не гуру js'а.

Что значит «тоже»?) Я очень даже неплохо в Питоне шарил раньше, теперь, конечно, многое подзабылось, да и Питон уже 3.6, а не 2.7 на дворе :)

В питоне геттер/сеттер назначается и дергается, когда присваиваешь что-то?

Когда работаешь с полями инстанса класса, объекта, в общем. На самом деле, property это «сахарок», я могу pure python реализацию накатать, но тогда тебе еще сложнее покажется :).

В js всё просто и понятно.

После Питона очень раздражает отсутствие строгой типизации и вменяемого ООП.

Virtuos86 ★★★★★
()
Ответ на: комментарий от crutch_master

Лютое при чем. Еще говорят у перла совсем все плохо. А вот это говно - хорошо да.

anonymous
()
Ответ на: комментарий от Virtuos86

После Питона очень раздражает отсутствие строгой типизации и вменяемого ООП.

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

crutch_master ★★★★★
()
Ответ на: комментарий от Virtuos86

После Питона очень раздражает отсутствие строгой типизации и вменяемого ООП.

лол, как будто в питоне есть вменяемое ООП и строгая типизация ))

Int64 ★★★
()
Ответ на: комментарий от crutch_master

Это больше проблема хренового проектирования.

Это особенности веба, где жабаскрипт доминирует.

Siado ★★★★★
()
Ответ на: комментарий от Int64

После Питона очень раздражает отсутствие строгой типизации и вменяемого ООП.

лол, как будто в питоне есть вменяемое ООП и строгая типизация ))

В Питоне строгая типизация. Или ты из дислексиков, которые путают строгую типизацию со статической?

Virtuos86 ★★★★★
()
Ответ на: комментарий от Siado

ну к примеру отсутствиет public, private, protected, package, все держится на соглашении между разработчиками. и очень не нравится как задаются поля для класса и отсутствие статической типизации.

Int64 ★★★
()
Ответ на: комментарий от Virtuos86

В Питоне строгая типизация. Или ты из дислексиков, которые путают строгую типизацию со статической?

я в питоне почти не работаю, думал там слабая типизация.

Int64 ★★★
()
Ответ на: комментарий от Int64

ну к примеру отсутствиет public, private, protected, package, все держится на соглашении между разработчиками

Так ты приверженец этого студенческого мифа. Понятно, понятно.

Siado ★★★★★
()
Ответ на: комментарий от Siado

а какой конкретно миф? знаю что нижнее подчеркивание вроде бы означает private, но я все равно могу использовать его где хочу

Int64 ★★★
()
Ответ на: комментарий от Siado

Ждем новых откровений: интерфейсы не нужны, можно и так говнякать (апдейт после посвящения в го: интерфейсы няшки и нужны), ООП маздай, дженерики для лохов и т.п. истины быдлокодера.

anonymous
()
Ответ на: комментарий от Int64

думал там слабая типизация.

Канеш слабая, в булевом контексте происходят неявные васянские касты, например. Разве такое возможно при строгой типизации?

anonymous
()
Ответ на: комментарий от Virtuos86

После Питона очень раздражает отсутствие строгой типизации и вменяемого ООП.

Шютка года про вменяемое ООП в питоне. Лол.

anonymous
()
Ответ на: комментарий от Int64

Лорчую, в питоне вообще геттеры и сеттеры очень редко нужны. Явное лучше неявного, просто напиши функцию.

pawnhearts ★★★★★
()
Ответ на: комментарий от Int64

все держится на соглашении между разработчиками

Как и везде. Ты можешь пролезть к private если очень хочешь в языках где они есть.

отсутствие статической типизации.

Есть typehints и инструменты для проверки типов, просто они не обязательные. https://imgur.com/a/taivs

pawnhearts ★★★★★
()
Ответ на: комментарий от pawnhearts

Как и везде. Ты можешь пролезть к private если очень хочешь в языках где они есть.

В java9 с этим решили покончить, теперь рефлекшеном не залезешь, только если модуль сам не разрешит либо jvm будет запущена с соответствующими ключами. Да и во времена апплетов думаю песочница была огорожена в том числе с учетом модификаторов доступа к членам классов, не проверял.

Aber ★★★★★
()
Ответ на: комментарий от Aber

В любом случае это защита от дурака. Если ты лезешь менять приватные члены класса ты знаешь что делаешь. И знаешь что всё может сломаться с апдейтом библиотеки.

pawnhearts ★★★★★
()
Ответ на: комментарий от pawnhearts

Ты можешь пролезть к private если очень хочешь в языках где они есть.

Можно и хрен дверью прищемить. Явное указание публичного интерфейса всяко лучше каких-то уродских черточек (которые можно вообще забыть). Вы же любите всё явное, а тут вдруг такой выверт. Фанбойство оно такое.

anonymous
()

Худшего кода давно не видел.

ei-grad ★★★★★
()
Ответ на: комментарий от anonymous

Ну ты же не забываешь private слово написать, в чём проблема с подчёрквиванием?

kukuruku ★★
()
Вы не можете добавлять комментарии в эту тему. Тема перемещена в архив.