hoangdvspkt / giaiPTBac2

0 stars 0 forks source link

Code review #1

Open tiennth opened 7 years ago

tiennth commented 7 years ago

1. Coding convention

2. Code design

3. Logic.

4. License

Hiện tại đang có 3 bạn có cùng sản phẩm này, anh không biết ai là tác giả.

hoangdvspkt commented 7 years ago

Dạ em cảm ơn nhận xét của anh ạ! Em xin lỗi anh vì sự cố này, lý do vì trong ngày đi học trên trường em đã làm bài và có quay clip nhưng đến cuối buổi thì em lại quên không up bài lên github và về nhà thì máy tính của em lúc đó do cấu hình yếu nên không thể chạy máy ảo OS để làm bài được. (Em có quay video bài làm của mình trong link nộp bài trước đó ạ). Cho nên em đã dùng code của bạn để up ạ, vì ý tưởng làm bài của bọn em cũng khá giống nhau... Rất xin lỗi anh vì sự việc lần này. Em sẽ rút kinh nghiệm ở những lần sau để không tái phạm. Em cảm ơn anh ạ

Vào 23:21 19 tháng 4, 2017, Tien Nguyen notifications@github.com đã viết:

  1. Coding convention

    • Tên biến, tên hàm có theo chuẩn nhưng không thống nhất. Lúc camelCase lúc không.
    • Về spacing thì nên thống nhất, không nên xuống dòng tùy tiện.
    • Có thể bỏ qua khai báo type của biến khi khai báo biến và assign giá trị như:

OK NG let a = Double(tf_a.text!)! let a:Double = Double(tf_a.text!)!

  • Nên có comment khi viết code.
  1. Code design

    • Scope của biến (local, global) hiện tại chưa tốt. Khai báo biến delta, nghiem1, nghiem2 với scope còn lớn => có thể giảm scope xuống nhỏ hơn nữa
    • Code xử lý business (giải phương trình) nên tách biệt khỏi code xử lý UI. Hiện tại cả 2 đang gom chung. Vd: Tách việc giải phương trình ra 1 method. Method giải phương trình xử lý và trả về kết quả. Kết quả đó được show lên UI trong 1 method khác.
  2. Logic.

    • Method func textField(_ textField: UITextField, shouldChangeCharactersIn range: NSRange, replacementString string: String) -> Bool chưa tối ưu logic.
    • Đã có check empty và validate data user input => Good.
  3. License

Hiện tại đang có 3 bạn có cùng sản phẩm này, anh không biết ai là tác giả.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/hoangdvspkt/giaiPTBac2/issues/1, or mute the thread https://github.com/notifications/unsubscribe-auth/ARkJxWp6mFD53hVc26-PQ6u91a3UrmXIks5rxjR2gaJpZM4NB6yw .