Flutter - Implementing a simple ListView with MVVM

Question

I've implemented a simple ListView in MVVM that displays a few PrepSteps (short form of PreparationSteps). While implementing I wasn't sure about if my approach is "the way to go" so I decided to get the code reviewed.

First I want to share the image of the application so you get a better idea of what I have accomplished.

---

Model:

prep_step.dart:

class PrepStep {
  static int id = 1;

  String name;
  String shortDescription;
  String longDescription;
  int number;
  bool isFinished;

  PrepStep() {
    this.shortDescription = 'This is a very short description of the Step';
    this.longDescription =
        'This is a slightly longer description of the Step. Detailed instructions go here.';
    this.number = id++;
    this.isFinished = false;
    this.name = 'Step ${this.number}';
  }

  String getDueDays() {
    if (this.number == 1) {
      return '1 Day';
    } else {
      return '${this.number} Days';
    }
  }
}

prep_step_list.dart:

import 'package:itinerary/model/prep_step.dart';

class PrepStepList {
  List<PrepStep> prepSteps;

  PrepStepList({this.prepSteps}) {

  }
}

ViewModel:

prep_step_view_model.dart:

import 'package:itinerary/model/prep_step.dart';

class PrepStepViewModel {
  PrepStep step;

  PrepStepViewModel({step}) {
    this.step = step;
  }
}

prep_step_list_view_model.dart:

import 'package:itinerary/model/prep_step_list.dart';
import 'package:itinerary/viewmodel/prep_step_view_model.dart';

class PrepStepListViewModel {
  PrepStepList prepStepList;
  List<PrepStepViewModel> prepStepViewModels;

  PrepStepListViewModel({prepStepList}) {
    this.prepStepViewModels = new List();

    this.prepStepList = prepStepList;
    if (this.prepStepList != null) {
      for (int index = 0; index < this.prepStepList.prepSteps.length; index++) {
        prepStepViewModels.add(new PrepStepViewModel(step: this.prepStepList.prepSteps[index]));
      }
    }
  }
}

View:

prep_step_view.dart:

import 'package:flutter/material.dart';
import 'package:itinerary/viewmodel/prep_step_view_model.dart';

class PrepStepView extends StatefulWidget {
  PrepStepViewModel stepViewModel;

  PrepStepView({stepViewModel}) {
    this.stepViewModel = stepViewModel;
  }

  @override
  _PrepStepViewState createState() => _PrepStepViewState();
}

class _PrepStepViewState extends State<PrepStepView> {
  @override
  Widget build(BuildContext context) {
    return Card(
      child: Column(
        children: <Widget>[
          ListTile(
            title: Text(this.widget.stepViewModel.step.name),
            subtitle: Text(
                'Step ${this.widget.stepViewModel.step.number.toString().padLeft(2, '0')} - Due in ${this.widget.stepViewModel.step.getDueDays()}'),
            trailing: Checkbox(
              value: this.widget.stepViewModel.step.isFinished,
              onChanged: onCheckBoxChanged,
              tristate: true,
            ),
          ),
          Divider(
            height: 1.0,
          ),
          ListTile(
            leading: Icon(Icons.description),
            title: Text(this.widget.stepViewModel.step.shortDescription),
            subtitle: Text('Description'),
          )
        ],
      ),
    );
  }

  void onCheckBoxChanged(bool value) {
    setState(() {
      this.widget.stepViewModel.step.isFinished = value;
    });
  }
}

prep_step_list_view.dart:

import 'package:flutter/material.dart';
import 'package:itinerary/view/prep_step_view.dart';
import 'package:itinerary/viewmodel/prep_step_list_view_model.dart';

class PrepStepListView extends StatefulWidget {

  PrepStepListViewModel prepStepListViewModel;

  PrepStepListView({prepStepListViewModel}) {
    this.prepStepListViewModel = prepStepListViewModel;
  }

  @override
  _PrepStepListViewState createState() => _PrepStepListViewState();
}

class _PrepStepListViewState extends State<PrepStepListView> {
  @override
  Widget build(BuildContext context) {
    return ListView.builder(itemBuilder: (context, index) {
      return PrepStepView(stepViewModel: this.widget.prepStepListViewModel.prepStepViewModels[index]);
    }, itemCount: this.widget.prepStepListViewModel.prepStepViewModels.length,);
  }
}

Main:

import 'package:flutter/material.dart';
import 'package:itinerary/model/prep_step_list.dart';
import 'package:itinerary/view/prep_step_list_view.dart';
import 'package:itinerary/view/prep_step_view.dart';
import 'package:itinerary/model/prep_step.dart';
import 'package:itinerary/viewmodel/prep_step_list_view_model.dart';
import 'package:itinerary/viewmodel/prep_step_view_model.dart';

void main() => runApp(new MyApp());

class MyApp extends StatelessWidget {
  @override
  Widget build(BuildContext context) {
    return new MaterialApp(
      title: 'Itinerary',
      theme: new ThemeData(
        primarySwatch: Colors.orange,
      ),
      home: new ItineraryApp(),
    );
  }
}

class ItineraryApp extends StatefulWidget {
  ItineraryApp() {}

  @override
  _ItineraryAppState createState() => _ItineraryAppState();
}

class _ItineraryAppState extends State<ItineraryApp> {
  List<PrepStep> prepSteps;

  @override
  Widget build(BuildContext context) {
    return Scaffold(
        appBar: AppBar(
          title: Text('Itinerary'),
        ),
        body: PrepStepListView(
            prepStepListViewModel: PrepStepListViewModel(
                prepStepList: PrepStepList(prepSteps: prepSteps))));
  }

  @override
  void initState() {
    super.initState();
    prepSteps = new List<PrepStep>();
    for (int counter = 0; counter < 30; counter++) {
      PrepStep step = new PrepStep();
      this.prepSteps.add(step);
    }
  }
}

---

The PrepStepList implementation seem to be a little bit weird because there is so much wrapping going on especially in the ViewModel class, but I don't know how to improve that. Maybe my approach is not the right one?

Answer 1

I'm not sure if this is helpful, but this is some feedback:

1. PrepStep shouldn't have a static int id because that will be a class-wide variable. This means that if you crease another PrepList, the numbers will not start at 1. See the dart docs:

   Class variables and methods

   Use the static keyword to implement class-wide variables and methods.

   Static variables

   Static variables (class variables) are useful for class-wide state and constants:

   class Queue {   static const initialCapacity = 16;   // ··· }
   
   void main() {   assert(Queue.initialCapacity == 16); } Static variables aren’t initialized until they’re used.
   

   You want every PrepStep to have its own number, which you have, but...

2. number is a pretty confusable name for the step number. But you also don't need the step number because you can get it from the index of the List. Ask yourself if PrepSteps need to know their numbers. What happens when you insert a step in between? You'll have to regenerate.

3. Will each PrepStep just be part of exactly one list? If that's so, then you can use indexOf to generate the step number.
4. Instead of prepStepViewModels.add(new PrepStepViewModel(..., can't you use .map(PrepStepViewModel)? You might need to change the constructor of PrepStepViewModel so it can take one positional argument... i.e. take out the curly braces on the constructor.
5. Store isFinished in the model, not the viewModel.

I'm going through the AppBrewery Flutter Course and it is quite good at explaining some of these language issues. I like it a lot. (I don't get a commission for mentioning it.)