got it, will start to use these tools and add more comments. On Tue, Jun 26, 2018, 7:44 AM Aleksander Alekseeev < a.aleks...@postgrespro.ru> wrote:
> Hello Charles, > > > Here is my current working status. Resolved all warnings found by > > Aleksander previously. Having two threads in parallel. One is the > > thrift binary type implementation, the other is thrift compact byte > > interface implementation. For these two threads, simple data type has > > been implemented and tested, but still need time to focus on > > complicated data type like struct, map or list. Let me know if you > > have any questions. Here is the repo with latest updates, > > https://github.com/charles-cui/pg_thrift > > Thanks for keeping us informed! > > I noticed that there are not many comments in your code. It's > considered good practice to write at least brief comments for every > procedure with 1) short description of what it does 2) what every > argument means 3) what does the procedure return 4) whether it changes > global state somehow (better avoid it). However, there is no strict > requirement to write these comments, especially if there are many > similar procedures and (1-4) are obvious. > > Also I advise to check your code with following tools unless you've > already done this: > > * lcov will show whether the code is covered with tests well. If some > part of code is not covered there is probably a test missing or maybe > the code is dead and should be removed. > * Clang Static Analyzer may find some errors that are difficult to > notice with a naked eye. > * Valgrind solves the same issue, but unlike Clang Static Analyzer > it checks your code not statically but in runtime. > > There are plenty of corresponding tutorials online. I wrote a few > myself [1][2][3]. However, although they should be readable through > Google Translate, I guess you may prefer tutorials in English. > > [1]: https://eax.me/c-code-coverage/ > [2]: https://eax.me/c-static-analysis/ > [3]: https://eax.me/valgrind/ > > -- > Best regards, > Aleksander Alekseev >
