Categories
Android iOS Programming

Some Principals for Coding

📖 3 min read

Design

  • Do the interactions of various pieces of code make sense?
  • Does this change belong in your code base, or in a API / Library?
  • Does it integrate well with the rest of your system?
  • Is now a good time to add this functionality?

The most important thing to cover in a review is the overall design. With this step you are making sure that the app is being improved on.

Functionality

  • Does this do what the developer intended?
  • Is what the developer intended good for the users of this code?

The “users” are usually both end-users (when they are affected by the change) and developers (who will have to “use” this code in the future).

Mostly, we expect developers to test their code well-enough that they work correctly by the time they get to code review. However, as the reviewer you should still be thinking about edge cases, looking for concurrency problems, trying to think like a user, and making sure that there are no bugs that you see just by reading the code.

Security

  • Is the code saving any customer data onto disk?
  • Is the code connecting to a new API connecting to a new source not covered by SSL pinning?
  • Is the API receiving information that don’t need?
  • Does the code contain any non secure API / HTTP calls?
  • If i tried to attack this, is this easy to break?

This is a harder one to fully look out for, but is very important. Mostly we expect our developers to think about this while they are coding. Think about what you code, and get an idea on how you could break it. Once the idea is there, try to fix the security hole that you see.

Complexity

  • Is the code more complex than it should be?
  • Is the intent of the code clear to you, or a hypothetical new employee first looking at the App?
  • Check this at every line of the code — are individual lines too complex?
  • Are functions too complex?
  • Are classes too complex?

“Too complex” usually means “can’t be understood quickly by code readers.” It can also mean “developers are likely to introduce bugs when they try to call or modify this code.”

A particular type of complexity is over-engineering, where developers have made the code more generic than it needs to be, or added functionality that isn’t presently needed by the system.

Reviewers should be especially vigilant about over-engineering. Encourage developers to solve the problem they know needs to be solved now, not the problem that the developer speculates might need to be solved in the future.

Tests

  • Will the tests actually fail when the code is broken?
  • If the code changes beneath them, will they start producing false positives?
  • Does each test make simple and useful assertions?
  • Are the tests separated appropriately between different test methods?

Make sure that you have unit tests that are appropriate for the change. In general, tests should be added in the same commit as the code itself.

Make sure that the tests in the code is correct, sensible, and useful. Tests do not test themselves, and we rarely write tests for our tests—a human must ensure that tests are valid.

Comments

  • Did the developer write clear comments in understandable English?
  • Are all of the comments actually necessary?

Usually comments are useful when they explain why some code exists, and should not be explaining what some code is doing. If the code isn’t clear enough to explain itself, then the code should be made simpler. There are some exceptions (regular expressions and complex algorithms often benefit greatly from comments that explain what they’re doing, for example) but mostly comments are for information that the code itself can’t possibly contain, like the reasoning behind a decision.

Categories
Code Review

What to look for in code reviews

📖 2 min read

Philosophy

The primary purpose of code review is to make sure that the overall code health of your mobile code base is improving over time. All of the tools and processes of code review are designed to this end.

In order to accomplish this, a series of trade-offs have to be balanced. First, developers must be able to make progress on their tasks. If you never submit an improvement to the code base, then the code base never improves. Also, if a reviewer makes it very difficult for any change to go in, then developers are dis-incentivized to make improvements in the future.

On the other hand, it is the duty of the reviewer to make sure that each PR is of such a quality that the overall code health of their code base is not decreasing as time goes on. This can be tricky, because often, code bases degrade through small decreases in code health over time, especially when a team is under significant time constraints and they feel that they have to take shortcuts in order to accomplish their goals.

In general, reviewers should favor approving a PR once it is in a state where it definitely improves the overall code health of the system being worked on, even if the PR isn’t perfect.

In General

In doing a code review and preparing code for review, you should make sure that:

  • The code is well-designed.
  • The functionality is good for the users of the code.
  • Any UI changes are sensible and look good.
  • Any parallel programming is done safely.
  • The code isn’t more complex than it needs to be.
  • The developer isn’t implementing things they might need in the future.
  • Code has appropriate unit tests.
  • Tests are well-designed.
  • The developer used clear names for everything, free from abbreviations.
  • Comments are clear and useful, and mostly explain why instead of what.
  • Code is appropriately documented.
  • The code conforms to the style guides.

Make sure to review every line of code you’ve been asked to review, even go over the pull request twice, look at the context, make sure you’re improving code health, and compliment developers on good things that they do.

Categories
Android App Design

Clean Architecture within an app.

📖 2 min read

One of the hardest things to do is to make sure that a codebase is easy to understand. From a junior to a senior developer, making sure that the main concepts are passed on and understood is an up hill battle. Clean Architecture is a solution that aim to solve this problem.

Clean Architecture is focused on breaking up your app into logical components. This could mean breaking up services that connect to an outside data source and to displaying of the data that is fetched.

Main concepts of clean architecture are as follows.

Use Case.

A use case is a description of the way that an automated system is used. It specifies the input to be provided by the user, the output to be returned to the user, and the processing steps involved in producing that output. A use case describes application-specific business rules as opposed to the Critical Business Rules within the Entities.

Use Cases – Clean Architecture by Robert C Martin

Repositories.

Repositories are used to allow Data Models to connect with use cases and supply information to then allow them to run their business logic, but also allows them to not be depended on using Dependency Inversion Principle.

The Dependency Inversion Principle The code that implements high-level policy should not depend on the code that implements low-level details. Rather, details should depend on policies.

DIP – Clean Architecture by Robert C Martin

Services.

Services are the details to the high level policy (Use case). Where the data is coming from, what data was required to get that information. What Authentication’s required and what response was received. Services allow for the underlying OS and API to gather the data for you and then returns them within the data layer model.

Once data has been returned from the service, it is up to the mappers to then map it to a domain model, as the Domain doesn’t (and shouldn’t) care about how it was received and in what previous format it was. It is then up to the Repository to then send up the domain model up to the use case.

All of these concepts within one diagram.