search

ImLewel / ImLewel_Notepad

Notepad for CGW OOP
MIT License
1 stars 0 forks source link

Code review #1

Open tshemsedinov opened 1 year ago

tshemsedinov commented 1 year ago
  1. Hardcoded path: https://github.com/ImLewel/ImLewel_Notepad/blob/0b25e45f46b0b8a04228c6ff618b6f557cd8f2c3/ImLewel_Notepad/Form1.cs#L21
  2. Long if-else-if chains is a subject of refactoring, for example to collections: https://github.com/ImLewel/ImLewel_Notepad/blob/0b25e45f46b0b8a04228c6ff618b6f557cd8f2c3/ImLewel_Notepad/Form1.cs#L49-L59
  3. Holly shit, it a carpet of code: https://github.com/ImLewel/ImLewel_Notepad/blob/0b25e45f46b0b8a04228c6ff618b6f557cd8f2c3/ImLewel_Notepad/FindWindow.Designer.cs and https://github.com/ImLewel/ImLewel_Notepad/blob/0b25e45f46b0b8a04228c6ff618b6f557cd8f2c3/ImLewel_Notepad/Form1.Designer.cs
  4. Naming: label1, label2, checkBox1, comboBox1...
  5. Do you know about or operator https://github.com/ImLewel/ImLewel_Notepad/blob/0b25e45f46b0b8a04228c6ff618b6f557cd8f2c3/ImLewel_Notepad/FindWindow.cs#L19-L22
ImLewel commented 1 year ago
  1. I'm able to leave this option empty but from user experience in various text editors I would say this is the best start point of choosing destination, this is very subjective in my opinion.
  2. I' not sure whether same number of same events should be better practice, maybe I can use list or something like that, but...
  3. It is autogenerated code of form
  4. Forgot to change, thank you
  5. Shame on me, that was obvious
ImLewel commented 1 year ago

Hope this is not the only code review, mostly waiting on https://github.com/ImLewel/ImLewel_FPS_Shooter review

tshemsedinov commented 1 year ago

Have no time to comment but looks terrible, you need to read "Clean code"

ImLewel commented 1 year ago

I don't know which files have you watched but code is pretty clean even if there are a lot of lines in some of scripts, good namings (where they are stable and not added during some tests), caching or serialization instead of runtime assignment where this is possible, decomposition of code and some optimization of usage different instances, variables and whatever are also there. Also movement code has a lot of language or API features and has variety of approaches to solve different problems. I expected at least 85 from this project, 90 would be great, but you didn't even reviewed it. Anyway, thanks for review, have a nice weekend.

tshemsedinov commented 1 year ago

Codebase of FSP_Shooter is also terrible carpet of code, see review: https://github.com/ImLewel/ImLewel_FPS_Shooter/issues/1