Code review example: Inputs aren't set in the constructor: url is computed from empty values in Angular

Retired from the DiffDojo practice pool in October 2026; the review below is the canonical answer. An AI wrote this pull request for the task underneath. Read the diff and decide what you would say before you scroll to the review.

Angular · State & data flow · intermediate

The task

Avatar component with a fallback

1. Show the user's avatar from the src input.
2. When src is empty, show a generated fallback based on the name input.
3. Standalone component; the fallback must reflect the actual inputs at render time.

Adds the avatar component: computes the image URL from the src input, falling back to a name-based placeholder when src is missing. Standalone, renders the resolved URL.

The diff

The review

Issue 1: Inputs aren't set in the constructor: url is computed from empty values

src/app/components/avatar.ts, lines 11, 13, 14, 15

1. The constructor runs before inputs exist. Angular constructs the component, *then* assigns @Input bindings, *then* calls ngOnInit. Reading this.src and this.name in the constructor sees only the field initializers, both ''.
2. So url is frozen wrong. It's computed once, from empty src and empty name, and always lands on the fallback URL with name= blank. The real bindings never touch it.
3. It even survives a glance. A fallback image renders, so nothing looks broken until you notice every avatar is the same empty-name placeholder.

The fix: compute after inputs are set. Move it to ngOnInit, or derive it so it also reacts to later input changes, a getter, or (with signal inputs) a computed.

The fix

import { Component, Input } from '@angular/core';

@Component({
  selector: 'app-avatar',
  template: `<img [src]="url" alt="" />`,
})
export class Avatar {
  @Input() src = '';
  @Input() name = '';

  get url(): string {
    return this.src || `https://ui-avatars.com/api/?name=${encodeURIComponent(this.name)}`;
  }
}

Why it matters

Reading an @Input in the constructor → it's not there yet. Inputs are bound between construction and ngOnInit; anything derived from them belongs in ngOnInit, a getter, or a computed, never the constructor.

Real-world stakes: every avatar renders as the same blank-name placeholder in production while looking fine in a story that sets the field defaults, and the same shape silently breaks any config, id, or flag a component reads from an input at construction time. Models place initialization logic in the constructor by C#/Java habit, where it would be correct; in Angular it runs one lifecycle step too early.

Read more: angular input undefined in constructor, read input in ngOnInit angular, angular input lifecycle timing

The verdict

Request changes. Inputs aren't set in the constructor: url is computed from empty values

How to review Angular code · All code review examples