Dart side of face detector

[bparrishMines]

No description provided.

[mravn-google]

LGTM

[mravn-google]

release => releases

[mravn-google]

This code silently does the wrong thing if you add a new FaceDetectorMode in the future. We could add something like

String modeToString(FaceDetectorMode mode) {
  final String full = mode.toString();
  return full.substring(full.indexOf('.') + 1);
}

This method could alternatively be implemented using a switch.

[mravn-google]

That doesn't give any new information :-)
You might want to add that instances are immutable.

[mravn-google]

Dartdoc missing on constructor. The preconditions enforced by your asserts should be documented.

[mravn-google]

Using forEach might be considered clunkier than using List.map

return reply.map<Face>((dynamic data) => Face._(data)).toList();

and is likely slower than using a plain for loop:

final List<Face> faces = <Face>[];
for (dynamic data in reply) {
  faces.add(Face._(data));
}
return faces;

[mravn-google]

In degrees or radians? Is the rotation normalized to some interval like 0..360?

Same below.

[mravn-google]

What is the coordinate system here? Image pixels with (0, 0) the upper left corner?

[mravn-google]

[nit] You could move the const out to the list literal to avoid repeating it:

const <Point<int>>[
  Point<int>(17, 18),
  Point<int>(19, 20),
]

[mravn-google]

Half of the above code could be reduced to

for (FaceLandmarkType type in FaceLandmarkType.values) {
  expect(face.landmark(type).type, type)
}

[mravn-google]

This switch could be replaced by a lookup into a Map<String, FaceLandmarkType>:

enum FaceLandmarkType { ... }

Map<String, FaceLandmarkType> _stringToTypeMap;

String _typeToString(FaceLandmarkType type) {
  final String full = type.toString();
  return full.substring(full.indexOf('.') + 1);
}

FaceLandmarkType _stringToType(String string) {
  _stringToTypeMap ??= Map<String, FaceLandmarkType>.fromIterables(
    FaceLandmarkType.values.map<String>(_typeToString),
    FaceLandmarkType.values,
  );
  FaceLandmarkType type = _stringToTypeMap[string];
  if (type == null) {
    throw ArgumentError.value(string, 'Landmark name', 'Not valid landmark.');
  }
  return type;
}

[bparrishMines]

I'm not sure how important readability is, but i also got this to work. haha

_landmarks = Map<FaceLandmarkType, FaceLandmark>.fromIterables(
          FaceLandmarkType.values,
          List<FaceLandmark>.generate(
            FaceLandmarkType.values.length,
            (index) {
              final FaceLandmarkType type = FaceLandmarkType.values[index];
              return FaceLandmark._(
                type,
                Point<int>(
                  data['landmarks'][_enumToString(type)][0],
                  data['landmarks'][_enumToString(type)][1],
                ),
              );
            },
          ),
        );

[bparrishMines]

It would fail if missing one though. I'll probably have to add a check.

[mravn-google]

If you have a list of type List<T> and an expression expr(t) that has type U when t has type T, then you can replace

List<U>.generate(list.length, (int index) => expr(list[index]))

by the simpler

list.map<U>((T t) => expr(t)).toList()

provided expr does not use index. Since you only need an Iterable<U>, you can dispense with the .toList() and simplify your code to

_landmarks = Map<FaceLandmarkType, FaceLandmark>.fromIterables(
  FaceLandmarkType.values,
  FaceLandmarkType.values.map<FaceLandmark>((FaceLandmarkType type) {
    final dynamic landmarkMap = data['landmarks'];
    final String typeString = _enumToString(type);
    if (landmarkMap[typeString] == null) return null;
    return FaceLandmark._(
      type,
      Point<int>(
        landmarkMap[typeString][0],
        landmarkMap[typeString][1],
      ),
    );
  }
);

Next, you can avoid repeated lookup of data['landmarks'] and landmarkMap[typeString] by a bit of hoisting:

final Map<dynamic, dynamic> landmarkMap = data['landmarks'];
_landmarks = Map<FaceLandmarkType, FaceLandmark>.fromIterables(
  FaceLandmarkType.values,
  FaceLandmarkType.values.map<FaceLandmark>((FaceLandmarkType type) {
    final List<dynamic> position = landmarkMap[_enumToString(type)];
    return (position == null) ? null : FaceLandmark._(type, Point<int>(position[0], position[1]));
  }
);