Code review

[gerich-home]

Hey guys!

Some time ago I asked a question to myself if there was a "perfect" team, that followed all these good design principles and practices.

So I decided to do a small research/comparison of the code quality of open-source code of different large, well-known companies like Microsoft, Intel, JetBrains and etc.
I just want to know where is the best place to work from the perspective of good software design.

I want to give some feedback on your code.
Taking into account that there are some stars and forks of your repository I assume the product itself is useful and investing time into its quality and design will give you lots of benefit, flexibility and etc.
I believe my criticism should have some positive results :)

I was really execited to read some of you code, examples:
https://github.com/01org/IntelSEAPI/blob/master/runtool/sea_runtool.py
https://github.com/01org/IntelSEAPI/blob/master/runtool/importers/osx.py
https://github.com/01org/IntelSEAPI/blob/master/runtool/importers/etw.py
https://github.com/01org/IntelSEAPI/blob/master/runtool/importers/pprof_importer/profile.py
https://github.com/01org/IntelSEAPI/blob/master/runtool/decoders/SteamVR.py
https://github.com/01org/IntelSEAPI/blob/master/runtool/decoders/MSNT_SystemTrace.py
https://github.com/01org/IntelSEAPI/blob/master/sea_itt_lib/sea_itt_lib.cpp
https://github.com/01org/IntelSEAPI/blob/master/sea_itt_lib/ETLRelogger.cpp
...

So, just after opening these files I got a sense that you, guys, totally ignore some basic principles of software design - naming, code splitting and etc.

Just looking at filesizes I understand that something goes wrong here. Most functions span hunderds of lines! (at my work, I write functions of up to 20 lines of code, no more, on daily basis and I believe there is no problem splitting them into the smaller ones, no matter what programming language do you write code in).

There is no common code style (this is most noticable in C++ code, there I see inclusions of c-like code with a mix of code contating higher-level abstractions).

Doing some very simple refactorings like extract method, decompose conditional, rename variable, rename method, convert method to method object can significantly improve the quality of code.

I do really recommend you reading some related literature to find info on the topics like:
Code Smells, Refactoring, SOLID (especially SRP, OCP), KISS, DRY, GRASP, Code Complexity, Code cohesion, LCOM metric.

Something below can help you:
https://refactoring.guru/en
https://martinfowler.com/books/refactoring.html
http://www.cvc.uab.es/shared/teach/a21291/temes/object_oriented_design/materials_adicionals/principles_and_patterns.pdf
https://www.amazon.com/Clean-Code-Handbook-Software-Craftsmanship/dp/0132350882

You would say, "send me a PR", but as I said, I just do a research and review work :) DIY

Hope my review will be helpful to you :)

[araud]

No, it wasn't helpful. And I wonder why spending your time doing that? :)

'a "perfect" team, that followed all these good design principles and practices' would be code oriented more than business oriented with all the consequences. Feature quality is incomparably much more valuable. Personally I wouldn't suggest anyone to become part of code oriented team. Everything is measured by result. And I consider as good code the one that has created a good feature, not the one that just looks shining.

Do you advertise an automatic code brushing solution or want to help making code better? I am for it. :) I don't mind to have shining code after all, if it doesn't steal feature development time.

Or you JUST pointing out that the code is not perfect, because you could write better? :)

[gerich-home]

No, it wasn't helpful.
It is pity.

And I wonder why spending your time doing that? :)
Just for fun, like watching movies :) and to see what happens outside my own company, where guys (at least my team) are able to produce high quality code, while being more(!) productive in terms of being business oriented.

I consider as good code the one that has created a good feature, not the one that just looks shining.
I believe it is a true statement - the code that produces a good feature is good.
But I do also believe that you should also be confident enough that you do not break another features that exist when you are adding new.
I cannot believe you are confident when you add a really large feature that needs lot of existing code to be changed.
You should have very plain code with a very small cyclomatic complexity metric (so you have 1-2 scenarios to test manually) or you should have plenty of automatic tests (you have no, as I see: https://github.com/01org/IntelSEAPI/search?p=1&q=test&type=&utf8=%E2%9C%93).
You should not feel like adding a new feature is like adding it from the scratch (you should reuse a lot of you existing business model, you should feel like the feature is already there and you need just enable it). The code (that code oriented command produces) helps adding more and cooler features to the product.

Do you advertise an automatic code brushing solution
I do really want to run some C++/Python code analysis tools to take a look at what they say - just for fun (I see you use these languages mostly (Python 42.4%,C 28.6%, C++ 16.9%, ...).
And I think there should exist some automated (or even automatic) C++/Python linters that can do smth for you.
Also as you use C++/Python I think smth similar to ReSharper C++ or PyCharm should help you producing more clean code without thinking about your "spent" time:
https://www.jetbrains.com/resharper/eap/
https://www.jetbrains.com/pycharm/

I'll be back (but with some code metrics of your solution) :)

[gerich-home]

More to add.
DDD - domain driven design - the approach used by many "serious" companies as a way to streamline their development. The idea is to focus on business but reflect it in the code. It gives both of 2 worlds - good and clean code due to its alignment with features business want and fast emission of new features (what business expects) due to the same reason :)

[araud]

Well, great! I see that you are very knowledgeable in code quality area. And I see that you really want to help, not just to ridicule. And since this is open-source project, you can make your contribution! Next time you have a half of hour to spend around this code, please improve a function or two. Please don't forget to run the tests, they are in "test_*.sh/bat"

[gerich-home]

Default pylint config shows the following:
Your code has been rated at 5.86/10

pylint_results.txt