ftranghese / EC601-Fall2017-Seamless-Track-Detection

16 stars 8 forks source link

Code Review of Chunxi Wang #11

Closed tracycxw closed 6 years ago

tracycxw commented 6 years ago

README.md

It is a little confused for me to understand which code should I first to check. GitHub file structure and directory structure are not clearly understandable. It seems that it is because the project is still under development. But the good thing is that the overview of every folders are clear and well-organized.

perspective_transform.py

There are many import library at the beginning, maybe they can be sorted by the first character, this will be easier for people to review.

There're many variables in the code, it will better to make some comments on them so that the code will be more readable and others can better catch the point.

fully_conv_NN.py

It's good to give an introduction at the beginning, this make readers better understand the whole point. And it's also good to notice the sort of import libraries.

Some places in the code violate DRY (Do not Repeat Yourself) principle.Duplicated code is a risk to safety. If you have identical or very similar code in two places, then the fundamental risk is that there’s a bug in both copies, and some maintainer fixes the bug in one place but not the other. In the line 54-128, there are many repeat codes, this is bad from maintainability perspective. You can build a function and call it.

full_CNN_model.json

This json file is not readable, all the words show in one line.

Summary

In brief, the project you guys made is cool and attractive. The Github contains video, images, text and links, which are really good and clear to show the results. And I learned a lot of useful code skills during this code review. Look forward to your final demo in the Sprint4 !