what does this piece of code do?

#include "stdio.h"
int main (void) {
    int a=10, b=0, c=10;
    char* bits ="TFy!QJu ROo TNn(ROo)SLq SLq ULo+UHs UJq TNn*R\
    Pn/QPbEWS_JSWQAIJO^NBELPeHBFHT}TnALVlBLOFAkHFOuFETpHCStHAU\
    FAgcEAelclcn^r^r\\tZvYxXyT|S~Pn SPm SOn TNn ULo0ULo#ULo-WH\
    q!WFs XDt!";
    a = bits[b];
    while (a != 0) {
        a = bits[b];
        b++;
        while (a > 64) {
            a--;
            if (++c == 'Z') {
                c /= 9;
                putchar(c);
            } else {
                putchar(33 ^ (b & 0x01));
            }
        }
    }
    return 0;
}

requirement analysis review

• can be discussed / reviewed
• even with a customer representative

architecture review

code review

def query_progress(user_id:int) -> float:
    # establish connection
    con= sqlite3.connect("data.db")
    # build query
    progress_query = f"""
    SELECT
        lesson / 50.0 AS progress
    FROM activity
    WHERE
        user_id = {user_id} AND
        result = 'success'
    ORDER BY
        lesson DESC
    LIMIT 1
    ;
    """
    # execute query
    res =con.execute(progress_query)
    progress=res.fetchone()[0]
    return progress

informal

• asking a colleague to have a look at the code
  • they express their opinion and that’s all
  • no documentation
  • no process
• pair programming is also a kind of constant informal review

walkthrough

• not a formal process / review
• led by the author(s)
• the author(s) guide the participants through the work product to achieve a common understanding and to gather feedback
• useful for higher level documents like requirement specification
  • e.g., risk storming, sprint review

technical

• less formal review
• led by the trained moderator or a technical expert
• often performed as a peer review without management participation
• issues are found by experts (e.g., architects, designers)
• technical reviews can vary from quite informal to very formal

(Fagan) inspection [4]

review types by formality – summary

you are not your code

use I-messages

talk about the code, not the coder

ask questions

refer to the author’s behavior, not their traits

wrong

You are sloppy when it comes to following the style guide.

Can’t you just configure your IDE properly?

right

I believe that you should pay more attention to the style guide.

Try to enable the auto-formatting in your IDE.

OIR-rule of giving feedback

Observation

Describe your observations in an objective and neutral way. Refer to the behavior if you have to talk about the author. Using an I-message is often useful here.

e.g., “This method has 100 lines.”

Impact

Explain the impact that the observation has on you. Use I-messages.

e.g., “This makes it hard for me to grasp the essential logic of this method.”

Request

Use an I-message to express your wish or proposal.

e.g., “I suggest extracting the low-level-details into subroutines and give them expressive names.”

three filters for feedback

always ask yourself, if your feedback is true, necessary and kind

(from April Wensel [6] via [5])

• is it true?
  • avoid statements assuming an absolute truth
  • avoid the words “right”, “wrong”, “never”, “always” and “should”
  • refer to your opinion instead
• is it necessary?
  • does the demanded requested change make the reviewed code better
• is it kind?

praise

• don’t forget to express your appreciation if you have reviewed good code
• praising doesn’t hurt you but will motivate the author
• however, be specific and separate the prasie from the criticism

looks good to me

how can you tell if the code matches the task?

how to write the commit message?

1. Separate subject from body with a blank line
2. Limit the subject line to 50 characters
3. Capitalize the subject line
4. Do not end the subject line with a period
5. Use the imperative mood in the subject line
6. Wrap the body at 72 characters
   • least important
7. Explain what and why not how
8. Reference the issue!
   • my addition for traceability
   • GH-33 (GitHub), GL-33 (GitLab),
   • or custom identifier for a project (Jira, JetBrains)

keeping clean history

discussion