Dependency injection ... we should have some of that

+++ b/src/FocalPoint.php
@@ -21,11 +21,11 @@ class FocalPoint {
-  private $fid;
+  private $file;

Drupal doesn't use private; protected is greatly preferred.

+++ b/src/FocalPoint.php
@@ -38,51 +38,51 @@ class FocalPoint {
-   * @param int $fid
+   * @param File $file

@param File should be @param \Drupal\file\FileInterface.

+++ b/src/FocalPoint.php
@@ -38,51 +38,51 @@ class FocalPoint {
+  public function __construct(File $file) {

Likewise, we prefer to type hint interfaces, not concrete classes, so that we can use mock objects in unit testing. So this should be FileInterface, not File.

+++ b/src/FocalPoint.php
@@ -38,51 +38,51 @@ class FocalPoint {
-   * Implements \Drupal\focal_point\FocalPoint::getFocalPoint().
+   * Implements \Drupal\focal_point\FocalPoint::setFocalPoint().

This comment is strange -- is setFocalPoint() an abstract method or something? This line should probably just be iced.

+++ b/src/FocalPoint.php
@@ -38,51 +38,51 @@ class FocalPoint {
+   * @param string $focal_point
+   *
+   * @param bool $save
+   *
+   * @throws \InvalidArgumentException

These need more info :)

+++ b/src/FocalPoint.php
@@ -38,51 +38,51 @@ class FocalPoint {
+   * Implements \Drupal\focal_point\FocalPoint::getFocalPoint().

This line is kinda meaningless, since it's on a method called...FocalPoint::getFocalPoint() :) Should be removed.

+++ b/src/FocalPoint.php
@@ -38,51 +38,51 @@ class FocalPoint {
-   * @return array
+   * @return string

Needs more info about the meaning of the return value.

+++ b/src/FocalPoint.php
@@ -109,37 +111,23 @@ class FocalPoint {
+    if (!empty($this->focalPoint)) {
       \Drupal::database()->merge('focal_point')
-        ->key(array('fid' => $this->fid))
-        ->fields(array('focal_point' => $focal_point))
+        ->key(array('fid' => $this->file->id()))
+        ->fields(array('focal_point' => $this->focalPoint))

Ideally, \Drupal::database() should not be used in a class. The database should be injected in the constructor -- \Drupal\Core\Database\Connection.

+++ b/src/FocalPoint.php
@@ -208,4 +198,17 @@ class FocalPoint {
+   * @param int $fid
+   *
+   * @return int|FALSE

These need more info.

+++ b/src/FocalPoint.php
@@ -208,4 +198,17 @@ class FocalPoint {
+  protected function getFocalPointFromDatabase($fid) {
+    return db_query('SELECT focal_point FROM {focal_point} WHERE fid = :fid', array(':fid' => $fid))->fetchField();

Should not use db_query(). The database should be injected in the constructor so you can do $this->database->select().

+++ b/tests/src/Unit/FocalPointTest.php
@@ -17,6 +18,55 @@ use Drupal\focal_point\FocalPoint;
+    $file = $this->getMockBuilder('Drupal\file\Entity\File')
+      ->disableOriginalConstructor()
+      ->getMock();

Generally, we prefer to mock interfaces over concrete classes. Also, if you mock FileInterface instead of File, you will not need to call disableOriginalConstructor().