В очередной раз при правке кода на питоне у меня пригорело от 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
Но этот вариант по моему мнению хуже.
