molczane
commented
3 months ago Hi Evgeny,
Thank you for this detailed and informative feedback.
I will surely get to better understand the rules you pointed out.
Anyways, I had a lot of fun doing this project and will try to become a JetBrains intern in the future.
Thanks, Ernest Mołczan
W dniu wt., 7.05.2024 o 20:36 Evgeniy Stepanov @.***> napisał(a):
Hello Ernest,
thank you for participating JetBrains Intership program and for choosing my internship project! My name is Evgeny, I am Team Lead in Rider Platform. Now I am going to provide you a comprehensive code review.
First of all, you written excellent readme.md file with detailed explanation of your vision and various feature. I liked it. Do the same in the future.
About UX. I am not sure I like your desing because it is sometimes difficult to read black code symbols on dark-gray background. Also I constantly feel application is unfocuced because of that background color.
The code is quite simple and easy-to-read. It is fine. Also you made avalonia integration well, left a good amount of comments in business logic and tried to split view and view model. However, the lack of industrial programming experience is very noticeable. Below there are several examples which I suggest you to improve.
- Do not put business logic into code behind. For example in OnTreeCaretEvent you mixed up some AST/Text operations and UI updates. Usually this approach leads to the spaghetti code over the years.
- There are code duplicates. Cleanup such things before final commit/merge/review. Code base should be clean and straightforward.
- Meaningless or unused code. Try to avoid it.
4.
Naming. private StackPanel stackPanel; but one line above private SyntaxTreeCaretEvent _treeCaretEvent; 5.
StringBuilder is your friend when you work with strings.
The general suggestion is to pay more attention to small details and code tidiness. While this may not seem important for a one-day home project, it becomes critical in real production. It's better to demonstrate the ability to write clean code and have knowledge not only of .NET or Avalonia frameworks, but also of MVC/MVVM and SOLID. This may sound a bit 'old-school', but I have to repeat, it becomes crucial in production.
— Reply to this email directly, view it on GitHub https://github.com/molczane/CodeVisualizer/issues/1, or unsubscribe https://github.com/notifications/unsubscribe-auth/A6S25W75DAISGSN5NLQNY3DZBENMBAVCNFSM6AAAAABHLRB7N6VHI2DSMVQWIX3LMV43ASLTON2WKOZSGI4DGOJYG42DONY . You are receiving this because you are subscribed to this thread.Message ID: @.***>
Hello Ernest,
thank you for participating JetBrains Intership program and for choosing my internship project! My name is Evgeny, I am Team Lead in Rider Platform. Now I am going to provide you a comprehensive code review.
First of all, you written excellent
readme.mdfile with detailed explanation of your vision and various feature. I liked it. Do the same in the future.About UX. I am not sure I like your desing because it is sometimes difficult to read black code symbols on dark-gray background. Also I constantly feel application is unfocuced because of that background color.
The code is quite simple and easy-to-read. It is fine. Also you made avalonia integration well, left a good amount of comments in business logic and tried to split
viewandview model. However, the lack of industrial programming experience is very noticeable. Below there are several examples which I suggest you to improve.OnTreeCaretEventyou mixed up some AST/Text operations and UI updates. Usually this approach leads to the spaghetti code over the years.https://github.com/molczane/CodeVisualizer/blob/4275e6ec53e617d64f1766570376c20d20f93239/RoslynAvaloniaTask/Views/MainWindow.axaml.cs#L113
https://github.com/molczane/CodeVisualizer/blob/4275e6ec53e617d64f1766570376c20d20f93239/RoslynAvaloniaTask/Views/MainWindow.axaml.cs#L210 https://github.com/molczane/CodeVisualizer/blob/4275e6ec53e617d64f1766570376c20d20f93239/RoslynAvaloniaTask/ViewModels/MainWindowViewModel.cs#L61
https://github.com/molczane/CodeVisualizer/blob/4275e6ec53e617d64f1766570376c20d20f93239/RoslynAvaloniaTask/ViewModels/MainWindowViewModel.cs#L107 https://github.com/molczane/CodeVisualizer/blob/4275e6ec53e617d64f1766570376c20d20f93239/RoslynAvaloniaTask/Views/MainWindow.axaml.cs#L150
Naming.
private StackPanel stackPanel;but one line aboveprivate SyntaxTreeCaretEvent _treeCaretEvent;StringBuilderis your friend when you work with strings.https://github.com/molczane/CodeVisualizer/blob/4275e6ec53e617d64f1766570376c20d20f93239/RoslynAvaloniaTask/Views/MainWindow.axaml.cs#L243
The general suggestion is to pay more attention to small details and code tidiness. While this may not seem important for a one-day home project, it becomes critical in real production. It's better to demonstrate the ability to write clean code and have knowledge not only of .NET or Avalonia frameworks, but also of MVC/MVVM and SOLID. This may sound a bit 'old-school', but I have to repeat, it becomes crucial in production.