kovalibre / electro

My first Python programm
1 stars 0 forks source link

Комментарии по коду #2

Closed sentyaev closed 6 years ago

sentyaev commented 6 years ago
  1. Именование переменных
    now = datetime.date.isoformat(datetime.date.today())

    Я бы назвал эту переменную today, т.к. datetime.date.today() возвращает именно день без времени, а now - это скорее текущий момент времени.

Очень сложно понять смысл переменных joint_kv, joint, j_per_per, приходится вчитываться в код и смотреть как ты эти переменные используешь. Это признак плохого именования. В идеале, по имени переменной должно быть понятно ее назначение. Ну и вообще мне сложно понять почему ты используешь слово joint, можешь объяснить?

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

  1. Константы Ты используешь константы для доступа к словарю такие как Kvartira, Meter и т.д. Я бы их объявил в начале файла и потом использовал не строки, а эти констатны Например:

    KVARTIRA = 'Kvartira'
    METER = 'Meter'
    ROOM = 'Room_'

    А исползовал бы так:

    kvar = k52[today][KVARTIRA]

    Какой смысл? Представь, что когда ты писал код ты допустил ошибку в строке, например вместо Kvartira ты бы написал Kvortira, то чтобы исправить тебе бы пришлось исправить это во всех местах, а с константами только в одном. Это вообще такое правило хорошего тона, создавать константы для всяких строковых названий или числовых данных которые не меняются.

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

  3. Структура программы Возможно ты еще про это не читал, но хорошо знать. У тебя сейча просто скрипт, это вполне норм, но можно его немного улучшить. Если ты еще не читал про фунции, то пока на это можешь забить. Стандартная структура программы:

    
    import something
    import another_something

def some_work(): print('hello world')

if name == "main": some_work()



В целом довольно хорошо все для начала. 
Удели больше внимания именованию переменных, это сильно упрощает чтение и понимание программы для других людей. Это вообще одно из главных правил в программировании - программа должна быть читаемой, не только для тебя, но и для других, т.к. мы всегда работаем в команде. Все остальное менее важно.
kovalibre commented 6 years ago
  1. Про now понял, исправлю.

    "Joint - совместный, общий, коллективный". Длинные имена переменных - это нормально? Как назвать переменные, отвечающие за "Общее потребление коммунальной электроэнергии", "Общая стоимость коммунальной электроэнергии", "Стоимость коммунальной энергии с человека" - придется ж голову поломать!)) Может лучше комментарием расшифровать рядом и оставить короткие условные имена? Все-таки короткими переменными легче пользоваться.

Переменная temp - это заглушка, я постарался комментарием это пояснить. Там где сейчас программа обращается к этой переменной, в будущих версиях она будет обращаться к сохраненному значению из прошлого, например к показаниям счетчика за прошлый месяц, чтобы вычислить текущий расход. Т. к. пока этих данных негде взять, я влепил ноль. Назвал temp - сокращенно от "temporary", чтобы легче ее потом найти и заменить на рабочий код.

  1. С константами понятно - попробую, пощупаю.

  2. Я пока прочитал 1/3 из книги, про функции только что вот начал. Пока мне не очевидно, где их применить, но разберусь))

  3. Структуру причешу.

sentyaev commented 6 years ago

Длинные имена это нормально, т.к. IDE или редактор кода тебе все равно помогут не набирать их постоянно, достаточно ввести пару букв и редактор подскажет название переменной. С другой стороны нужно стараться всетаки не делать имена переменных слишком длинными)))

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

Вот примеры как бы я назвал переменные:

  1. "Общее потребление коммунальной электроэнергии". Это же потребление энергии для квартиры, так? Можно назвать apartment_power_consumption
  2. "Общая стоимость коммунальной электроэнергии" - apartment_power_cost
  3. "Стоимость коммунальной энергии с человека" - person_power_cost

Но на самом деле ты потом узнаешь про классы и эта проблема длинных имен отвалится.

Может лучше комментарием расшифровать рядом и оставить короткие условные имена? Не стоит, это признак плохого кода. В комментариях обычно описывается сложный алгоритм, или пояснение почему было выбрано определенное решение. Твои коментарии кстати очень хорошие, ну кроме того про temp.

Если temp это предыдущее значение на счетчике электроэнергии, то можно назвать: electric_meter_value, а то что у тебя называется watt_kv обозвать current_electric_meter_value (текущее значение счетчика электроэнергии). И вот как можно будет изменить код:

Было:
cons_kv = watt_kv - temp
Стало:
power_consumption = current_electric_meter_value - electric_meter_value

Про функции. Они позволяют разбить программу на блоки, либо выделить общую функциональность для переиспользования. А главное они позволяют абстрагироваться от деталей. Вот например как можно разбить эту программу.

тариф = получить_текущий_тариф()
квартира = получить_данные_по_квартире()
электроэнергия = получить_последние_показания_счетчика()
результат = расчитать_платежи(тариф, квартира, электроэнергия)
вывод_результата(результат)

Есть такой принцип - Single responsibility principle (Принцип единственной ответственности) Имеется ввиду то, что каждая функция должна выполнять что-то одно. Но про это уже потом, когда у тебя фунции появятся.

kovalibre commented 6 years ago

Потратил три часа на исправления до 0.0.2 и только потом уже увидел второе сообщение здесь) Завтра поразмыслю, сейчас уже 4 часа ночи, пора б и вздремнуть, а то чёт засосало))