2016-02-03 4 views
0

В соответствии с CodeClimate два метода в упрощенном классе ниже являются дубликатами друг друга с массой около 40. Каким образом можно реорганизовать удалять дублирование? Эквивалентное понимание словаря имеет очень незначительно меньшую массу, но в остальном идентично.Удаление дубликата кода Python

class DataAdaptor: 
    def __init__(self): 
     self._feeds = {'field1': 'temperature', 'field2': 'humidity'} 

    def parse_data(self, data): 
     content = {} 
     for field, feed in self._feeds.items(): 
      if feed in data: 
       content[field] = data[feed] 
     return content 

    def parse_content(self, content): 
     data = {} 
     for field, feed in self._feeds.items(): 
      if field in content: 
       data[feed] = content[field] 
     return data 

Разъяснение: версия с словарными постижениями имеет почти точно такую ​​же дублирование массы, но я думаю, что это немного менее ясно.

class DataAdaptor: 
    def __init__(self): 
     self._feeds = {'field1': 'temperature', 'field2': 'humidity'} 

    def parse_data(self, data): 
     return {field: data[feed] for field, feed in self._feeds.items() if feed in data} 

    def parse_content(self, content): 
     return {feed: content[field] for field, feed in self._feeds.items() if field in content} 

Это зеленое поле развития, поэтому мы свободны делать что-нибудь.

+0

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

ответ

1

Альтернатива, которую я рассматриваю, заключается в том, чтобы хранить словарь обратного поиска.

class DataAdaptor: 
    def __init__(self): 
     self._feeds = {'field1': 'temperature', 'field2': 'humidity'} 
     self._fields = {'temperature': 'field1', 'humidity': 'field2'} 

    def parse_data(self, data): 
     return self._reform_data(data, self._fields) 

    def parse_content(self, content): 
     return self._reform_data(content, self._feeds) 

    def _reform_data(self, data, names) 
     return {names[k]: data[k] for k in names if k in data} 

Или есть более простой способ поменять ключи и значения в словаре?

0

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

parse_data(self, foo): 
    return {field: foo[feed] for field, feed in self._feeds.iteritems() \ 
           if feed in foo} 

... где parse_content немного отличается в то, что проверено и вернулся.

parse_content(self, foo): 
    return {feed: foo[field] for field, feed in self._feeds.iteritems() \ 
           if field in foo} 

Помогает ли это?

+0

К сожалению, версия для понимания словаря имеет почти ту же массу дублирования. – Nzbuu

0

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

class DataAdaptor: 
    def __init__(self): 
     self._feeds = {'field1': 'temperature', 'field2': 'humidity'} 

    def parse_content(self, **kwargs): 
     data, content = kwargs.get("data", {}), kwargs.get("content", {}) 
     return {k: data[v] for k, v in self._feeds.items() if v in data} if data\ 
      else {self._feeds[k]: content[k] for k in self._feeds.keys() & content} 

Вы также можете использовать self._feeds.keys() & data, чтобы получить общие ключи для python2 вы бы необходимо изменить его self._feeds.viewkeys() & data

Вы можете также использовать именованные аргументы:

class DataAdaptor: 
    def __init__(self): 
     self._feeds = {'field1': 'temperature', 'field2': 'humidity'} 

    def parse_content(self, data=None, content=None): 
     return {k: data[v] for k, v in self._feeds.items() if v in data} if data\ 
      else {self._feeds[k]: content[k] for k in self._feeds.keys() & content} 

Хотя лично я бы прив er для использования двух методов.

+0

Я не уверен, что любой из этих вариантов фактически уменьшает дублирование. Они просто перемещают его в один метод. – Nzbuu

+0

@Nzbuu, вы запустили его на CodeClimate? Вы могли бы сделать это в одной команде, но человек был бы уродлив. –

0

Вместо распаковки элемента в кв пару, мы можем обратиться к и V по индексу и волшебно инвертировать индекса при необходимости

def _parse_lol(self, source, num) 
    result = {} 
    for item in self._feeds.items(): 
     if item[1] in source: 
      result[num] = source[1-num] 
    return result 

def parse_data(self, data): 
    return self.parse_lol(data, 0) 

def parse_content(self, content): 
    return self.parse_lol(data, 1) 

I может криво 0 и 1,

UPD

def parse_parse(self, **kwargs): 
    direction = ["content", "data"].index(kwargs.keys()[0]) 
    source = kwargs.values()[0] 
    result = {} 
    for item in self._feeds.items(): 
     if item[1] in source: 
      result[item[direction]] = source[item[1-direction]] 
    return result 
+0

Я, хотя ваш адаптер shoud соответствует некоторому интерфейсу. Если это не так, мы можем ввести параметр 'direction'. 'num' здесь. И могут быть некоторые константы. Или мы можем перевести имя KW на индекс в каком-то словаре – Eugene

+1

Я не уверен, что этот код работает. В частности, строка 'result [i] = source [j]' не использует 'item'. – Nzbuu

+0

Да, пропустил этот. Добавленный элемент к этому назначению – Eugene

3

Я прочитал первые три ответа, и я хотел бы предложить что-то другое. Не переписывайте эти функции повторно. Они в порядке, как есть. Они короткие, понятные и легко читаемые. Зачем возиться с ними? Я никогда не использовал CodeClimate и понятия не имею, что означает масса в 40, но я считаю, что ошибочно рассматривать любой статический инструмент проверки кода как абсолютный окончательный авторитет в отношении того, как нужно писать часть программного обеспечения. Бьюсь об заклад, у вас есть больше вещей, о которых можно беспокоиться, чем частичное дублирование в простой небольшой функции, что, как вы уже сказали, может быть написано как единственное понимание словаря.

У меня есть одно предложение: измените имена двух функций на что-то вроде select_by_matching_values и select_by_matching_keys. Это дает видимость тому, что они делают и как они отличаются друг от друга.

+0

Из того, что я понимаю, «масса» - это всего лишь мера размера дублированной логики. – Nzbuu

+0

Я очень согласен с вашим мнением, что этот код довольно ясен и прост. Мне просто интересно, есть ли что-то, что мне не хватает. – Nzbuu

Смежные вопросы