CS3281/82 - Facebook Open Academy - Code Sprint - Day 2
Today is a harvest day! We finally started to write our first line of code, submit our first pull request(PR), and get our first PR merged!
As a practice of understanding codebender’s Github workflow, we are suppose to checkout a branch and start to work on the refactoring of the existing code. I was working on a file called ‘DefaultHandler.php’ because its code styles are not quite compatible with the PSR-2 coding standard we are using in codebender.
What is PSR? How to check your code with PSR?
PSR stands for PHP Standards Recommendations. It is a standard that was accepted among the PHP community. In particular, PSR-2 specify a coding style guide for every PHP developer, with the aim to achieve simpler and cleaner codes in the PHP projects.
PHP_CodeSniffer is a set of PHP scripts that help developers to check and correct their code styles. We will use the command
phpcs /path/to/fileto help us identify the issues regarding the code styles, and try our best to refactor and reformat them.
After half an hour of coding, we started to submit our pull request (mine). And John, which is our mentor, will go through the code review with us, point out some issues in our code, and help us to learn better. I made some minor mistake, such as mixing both code refactoring and logic changing into a single commit named “Refactor code”. This is not a good practice since it did more things than the commit describe, which may lead to confusion for other developers working on the project.
Another issue occurred with the following code after my refactoring:
As we can see, the function will check if a library exist in the builtin_libraries folder, and return a json object with a success attribute. If we found such library, then the success will become true, else we will return an error message and the success become false. It seems perfectly correct here (and it is!), but during the code review, John pointed out a very interesting idea of “true path”. That is to say, when we are checking some conditions in the function body, we should exit early if the condition doesn’t meet, and it is better to return the success(true) result at the end of the function. In this way, your logic flow in the code will be much clearer and simpler if you are expected to include more condition checking in the future. (see the comments)
After I fixed the above-mentioned issue, my PR finally got merged! It was really exciting for us to be able to contribute to a product that will be used by many people around the world!
In the afternoon, we finalized our database schema together with our mentors, and after we have created issues for them in the repo, we are ready to divide responsibility and begin to implement our future. We are excited to achieve more in the foreseeable future.