Комментарии 16
Сразу скажу, что это лично мое мнение, и оно не должно совпадать с мнением других людей. Например, как я понял из статьи, оно точно не совпадает с мнением разработчика работодателя, который делал Вам замечания.
Комментировать очевидный код не нужно. Комментарии нужны только в том случае, когда из кода не совсем может быть понятно (по мнению автора кода) что происходит, или, например, используются какие-то хитрые конструкции/оптимизации/паттерны. В Вашем случае:
# Закроем все окна и все сокеты
Зачем? Разве из следующих двух строк это не очевидно? Такой комментарий только захламляет код, его "чуточку сложнее" воспринимать.
Комментарий, дублирующий документацию сторонних библиотек (как работа логгера): зачем? В крайнем случае - ссылка на документацию, и то навряд ли нужно, ибо обычный программист работающий со сторонними библиотеками в курсе, что почти в 99.99% случаев у библиотек адекватная документация и если что-то не понятно - то нужно искать объяснения использования именно там, а не у Вас в коде.
# Настройки для отображения видео
А вот это - случай поинтереснее. Если бы так случилось, что программисту дают впервые смотреть этот код и он не знает что он должен делать - тут комментарий оправдан, потому что именование переменных немного странное. Что значит down_points ? Что это за нижние точки? Зачем две переменные down_width и down_height, если они используются только единожды? Задел на будущее? Почему они используются в cv2.resize, ресайз не предусматривает увеличение, только уменьшение? Но, опять же, тут лучше не комментировать, а просто переназвать переменные, которые будут отражать суть для чего они были инициализированы.
если они используются только единожды
А я с вами не соглашусь.
Магические числа - это знатный антипаттерн, поэтому наличие переменной лучше, чем её отсутствие.
frameSize = (400, 400)
Как Вам такой вариант без дополнительных переменных и магических чисел? Дополнительная переменная в данном случае уместна только в том случае, когда она используется более одного раза, или, например, попадает в скоуп извне. Ну или всё равно на используемую память приложением. Иначе переменная должна своим именем говорить, что в ней.
наличие переменной лучше
Что Вы можете сказать о переменной down_width, не зная контекста? И лучше ли в данном случае? Лично мне данное имя в контексте последующего кода говорит о том, что ресайз на увеличение не должен происходить, и задумка именно на уменьшение размера картинки (и стоило бы добавить проверку, так ли это на самом деле, если я, к примеру, хочу транслировать видео 320х240).
согласен с вами, спасибо за комментарий
frameSize = (400, 400)
Отличный вариант. И название переменной тоже хорошее.
Но она тоже используется только раз, что нарушает ваше же пожелание.
Смотрите в чём заключается антипаттерн:
либо объявляете переменную и присваиваете ей значение
либо вы пишите комментарий, что это за число и на что влияет, а если комментария нет, то это антипаттерн.
Да, тут можно - исходя из удачного нейминга вашей переменной - догадаться, что первое число - ширина кадра, второе - высота.
Опять же - наименование переменных, это навык, который прокачивается с опытом, и неудачно выбранное название переменной не является плохим паттерном, а является именно плохим названием конкретной переменной.
В целом, Вы полностью правы и я согласен, что в данном конкретном случае нарушается то, о чём я говорю =)
Но давайте снимем розовые очки и представим ситуацию из реального мира. Во-первых, стоит проверить, а нужно ли вообще вызывать cv2.resize? Возможен вариант, что frameSize[0] и frameSize[1] идентичны входящему размеру, следовательно, нужна проверка использующая frameSize. При чём, в случае идентичности размеров нужна единожды, а сам вызов ресайза в цикле будет вредным (если Вам, конечно же, важно быстродействие и не всё равно на лишние такты и память). Хотя тут тоже можно возразить, что совсем ничего не мешает использовать дополнительные переменные, которые, в целом не нужны.
Я к чему. Я согласен, что магические числа - плохо. Но я не согласен, что в данном конкретном случае использование frameSize не оправдано. Просто всунуть tuple вместо переменной в ресайз - как раз таки будет "магическими числами". И комментарий совсем не поможет.
Перед исполнением тестового хорошим правилом будет задавать вопросы по нему, чтобы уточнить ТЗ (scoping) и собрать дополнительные детали, а не "ОК, ясно, сделаю". Это покажет другой стороне что вы умеете общаться и договариваться "на берегу", а не чисто исполнитель "от а до я", и даст вам больше шансов пройти на следующий этап. Опять же, можно будет сказать что "я сделал о чем мы договаривались, и даже больше", и попросить более развернутый ответ по отказу.
Не знаю чему учат сейчас, но когда специальность ещё называлась "инженер-программист", то помимо алгоритмов и прочих матанов студентов учили мыслить "инженерно". Помимо прочего это как раз и включает в себя стойкость программы к ошибкам, обработка частных случаев, написание программы таким образом, чтобы другие могли легко внести изменения или хотя бы понять, что к чему. Так что стратегия "пойду учить логирование и обрывы связи" может быть не совсем подходящей. Лучше найти какой-нибудь открытый проект и предложить свою помощь. Там и задание дадут, и код забесплатно проверят и укажут на слабые места.
В моих свежих воспоминаниях, когда я еще не был инженером-программистом, четко всплывают слова преподов по профильным предметам, которые гласили примерно следующее "В СМЫСЛЕ на наш предмет выделено 120 часов? Должно быть в 2 раза больше!.. И что нам, половину лекций выбрасывать?.."
И те 120 часов отдавались философии/экономике/ОБЖ и пр.
Я инженер-программист с моим умением мыслить "инженерно", получается, только наполовину :)
В общем тяжело нашим студентам. Рынок идет семимильными шагами, а ООП в программу обучения включили дай бог года 3-4 назад.
вариант трансляции потока с камеры без лишних логов и комментариев )
#server
import zmq, cv2
def main():
socket = zmq.Context().socket(zmq.PUSH)
socket.bind("tcp://*:8000")
while True:
cap = cv2.VideoCapture(0)
while (cap.isOpened()):
ret, frame = cap.read()
socket.send_pyobj(frame)
if __name__ == '__main__':
main()
#client
import zmq, cv2
def main():
socket = zmq.Context().socket(zmq.PULL)
socket.connect("tcp://localhost:8000")
while True:
image = socket.recv_pyobj()
image = cv2.resize(image, (800, 600))
cv2.imshow('frame', image)
if cv2.waitKey(1) & 0xFF == ord('q'):
break
if __name__ == '__main__':
main()
Мне кажется или задающий тестовое еще сам не определился с тем что он хочет увидеть. И следующему задаст более расширенное тз.
Но в целом ощущение от Вашего кода как копи-паста из интернета, причем не из лучших источников. Если бы вы немного overwhelmed код написав все с помощью ООП, сделав DI библиотеки. Можно добавить ABC. Добавить тестов, завернуть все в контейнер это показало бы что Вы более опытный программист.
Непонятно что именно хотели проверить с помощью этого тестового. Что автор умеет решать задачи? Ну за 4 года скорее всего научился использовать разные библиотеки и изучить новую не проблема. К тому же где гарантия, что задание сделал именно собеседуемый?
При беглом чтении сложилось ощущение, что работы тут минимум часов на 8 без каких-либо гарантий трудоустройства и вообще продолжения разговора даже при успешном прохождении этого этапа. Т.е. работник должен выкинуть день-два, так что хотелось бы и от компании видеть какой-то симметричный ответ. Хотя бы оплату потраченного времени.
Каюсь, тестовые тоже иногда делаю, если они интересные. Например, поработать с AST для реализации вычислителя математических выражений вида 2*x+5*y^2-k+... Оно было несложным, в 3 часа уложился, но второе собеседование мне не смогли назначить за 2 недели. Но так хотя бы материал для статьи в блог.
Также можно разбить на несколько функций, прописать докстринги, указать type hints
Пытаюсь устроиться на работу #1 Тестовое задание на pyZMQ