В очередной раз при правке кода на питоне у меня пригорело от list comprehensions.


А вот что пишут настоящие живые люди, которых никто не заставляет под дулом пистолета:



I find the list comprehension much clearer than filter+lambda



Или:



Personally I find list comprehensions easier to read. It is more explicit what is happening from the expression [i for i in list if i.attribute == value] as all the behaviour is on the surface not inside the filter function.



Ну давайте посмотрим, как этот much clearer way выглядит in the wild. Как-то так:


    def getSupportedTrackers(self):
        trackers = self.getTrackers()

        if not self.site.connection_server.tor_manager.enabled:
            trackers = [tracker for tracker in trackers if ".onion" not in tracker]

        trackers = [tracker for tracker in trackers if self.getAddressParts(tracker)]  # Remove trackers with unknown address

        if "ipv6" not in self.site.connection_server.supported_ip_types:
            trackers = [tracker for tracker in trackers if helper.getIpType(self.getAddressParts(tracker)["ip"]) != "ipv6"]

        return trackers


Просто сплошной [blabla for blabla in blablas if ...blabla...].


Просто в начале каждой такой строки ты должен мысленно стирать кусок [tracker for tracker in trackers if и читать, что же там дальше. И как писал Роберт Мартин в «Чистом коде», любые конструкции, которые принуждают читателя тренироваться пропускать себя мимо глаз, являются источником скрытых ошибок. Пропустив 500 раз мимо глаз типовой фрагмент кода, на 501-й раз вы пропускаете ПОЧТИ такой же фрагмент, в котором содержится ошибка. И в силу одинаковой натренированности рефлексов у всех разработчиков продукта, эта ошибка может оставаться незамеченной годами.


Давайте посмотрим, как этот же код можно преписать на лямбдах на руби:


    def getSupportedTrackers():
        trackers = @getTrackers()

        if not @site.connection_server.tor_manager.enabled
            trackers = trackers.filter {|tracker| not tracker.include? ".onion"}
        end

        trackers = trackers.filter {|tracker| @getAddressParts(tracker)}

        if not @site.connection_server.supported_ip_types.include? "ipv6"
            trackers = trackers.filter {|tracker| helper.getIpType(@getAddressParts(tracker)["ip"]) != "ipv6"}
        end

        return trackers
    end


Уже стало лучше за счёт уменьшения количества бойлерплейта, который приходится пропускать мимо. Но 3 вызова trackers.filter подряд и два идентичных вызова getAddressParts говорят нам, что этот код надо переписать.


Заметьте, что необходимость рефакторинга для устранения дублирования не была очевидна в коде с list comprehensions, потому что они за своей многословностью и нечитабельным синтаксисом скрывают суть происходящего.


Убираем дублирование:


    def getSupportedTrackers()
        tor_enabled = @site.connection_server.tor_manager.enabled
        ipv6_enabled = @site.connection_server.supported_ip_types.include? "ipv6"

        trackers = @getTrackers()

        trackers = trackers.filter {|tracker|
            if (not tor_enabled) and (tracker.include? ".onion"
                next false
            end

            address_parts = @getAddressParts(tracker)
            if not address_parts
                next false
            end

            if (not ipv6_enabled) and (helper.getIpType(address_parts["ip"]) == "ipv6"
                next false
            end

            next true
        }

        return trackers
    end


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


P.S. Или для любителей длинных однострочников:


    def getSupportedTrackers()
        tor_enabled = @site.connection_server.tor_manager.enabled
        ipv6_enabled = @site.connection_server.supported_ip_types.include? "ipv6"

        trackers = @getTrackers()

        trackers = trackers.filter {|tracker|
            next false if (not tor_enabled) and (tracker.include? ".onion"

            address_parts = @getAddressParts(tracker)
            next false if not address_parts

            next false if (not ipv6_enabled) and (helper.getIpType(address_parts["ip"]) == "ipv6"

            next true
        }

        return trackers
    end


Но этот вариант по моему мнению хуже.









 ,






URL записи