Code Review 2 - December 1, 2017 Snapshot

[GrazianoMJ]

Branch: chihwei_tung

• From last update, noticed that csv combine folder was added.
  • Code very straightforward and easy to follow but contains no comments or description of the purpose.
  • Looking at your code, you are able to open the files but it looks like you don't close them explicitly. This is considered bad practice.
    • There is a method that is quite helpful to avoid having to keep track of open and close functions:

      with open(filename, "w") as file:  # Opens a file for writing and creates variable "file"
      # Put your lines of code here.
      # When you are done, you simply write your code outside the with statement. No close required.

      Branch: shanshan_zhao

• Thank you very much for adjusting the README file.
• Reviewing MainActivity.java and the commit history, it seems that there have been no updates to the code or any attempts to address any of the issues I mentioned in my previous code review. My original comments still stand.

  Branch: weixuan_jiang

• Similar to the shanshan_zhao branch, I did not find any changes made to the branch or attempts at addressing any of the issues mentioned in my previous code review. My original comments still stand.

  Branch: zhonghao_guo

• Detailed summary of work in README.md:
  • This seems to continue the format as it did during the previous code review. The good aspect of this is that it illustrates the progress being made on the code. The bad aspect however is that it is becoming more difficult to follow because I don't know what has been added and what all the code does. -I think it may be a good idea to switch to adding these type of updates as comments to your commits and update the README.md file to describe what the code is doing as a whole.
• Pitch folder:
  • Pitch.m/Pitch_extraction.m still have the same issues as I mentioned in my previous code review.
• MFCC folder:
  • I am curious as to what MFCC stands for. In your README.md files it simply says "will be uploaded soon...". It might be worth while to explain the information here.
  • Variables are still incredibly difficult to understand due to poor naming (what does "ccc" mean?)
    • If these variables represent common symbols in scientific formulas that is fine but it may be useful to comment on what the formula.
  • Plots do not have titles or axis labels. It would be better to understand what the data is if these labels are added.
• Formats folder:
  • format_extraction.m
    • Same issues as mentioned above.
      • The code that is provided by Theodoros Giannaopoulos is very well formatted. I would use that code as an example of good commenting and variable names.
• VSP_MATLAB folder/ VSP_WIN folder/ VSP_FINAL folder
  • VSP.m is very well written! It is short, has a great comment at the beginning explaining everything. I am assuming that this code is a way for you to perform you code as an executable?

    Final Thoughts:

• You guys have made a lot of progress if I am not mistaken based on your Trello and your presentations. Excellent work!
• Although I understand the desire to avoid going back into code you have completed to update comments or formatting, I would do so because you may need to explain your code later to a interviewer/may need to utilize a part of the code for another project.

[zhaoss99]

Thanks for your advise. I've update the branch of shanshan_zhao. There is our newest app in 601_app/Android-SpeexDenoise-master.