User Favorites Service I'm practicing my OOP and so created this service that stores a user's favorites from an application.

The favorite has an ID, a owner ID, and his type. This service only saves and makes this information available.

When instantiating the service, must be provided a API KEY and the types of favorites available.

class FavoritesService {

  /**
   * string
   */
  private $key;

  /**
   * array
   */
  private $data;

  /**
   * array
   */
  private $types;

  /**
   * @param string $key The key of the application.
   * @param array $types Array containing the type of favorites.
   */
  public function __construct($key, $types) {    
    $this-&gt;key = $key;
    $this-&gt;types = $types;
  }

  /**
   * Saves a favorite.
   *
   * @param integer $id The ID of the favorite.
   * @param integer $ownerId The ID of his owner.  
   * 
   * @throws InvalidArgumentException If the provided argument is not a valid type.
   * @throws Exception If the save to somewhere goes wrong.
   *
   * @return boolean Indicating that all went fine.
   */
  public function save($id, $ownerId, $type) {
    if (!in_array($type, $this-&gt;types)) {
        throw new InvalidArgumentException('Invalid favorite type.');
    }
    $this-&gt;data[$ownerId][] = self::buildResult($id, $type);
    // also save everything to somewhere
    return true;
  }

  /**
   * Provides all the favorites of a owner.
   *
   * @param integer $ownerId The ID of the owner.
   *
   * @return array|null All the favorites from the user or, if the user doesn't exists, null.
   */
  public function get($ownerId) {
      if (isset($this-&gt;data[$ownerId])) {
        return $this-&gt;data[$ownerId];
      }
      // checks somewhere if the data exists
      return null;
  }

  /**
   * Resets all the favorites of a owner.
   *
   * @param integer $ownerId The ID of the owner.
   *
   * @throws Exception If the reset to somewhere goes wrong.
   *
   * @return boolean Indicating that all went fine.
   */
  public function reset($ownerId) {
    unset($this-&gt;data[$ownerId]);
    // also update everything to somewhere
    return true;
  }

 /**
  * Builds a result of a favorite.
  *
  * @param integer $id The ID of the favorite.
  * @param string $type The type of the favorite.
  *
  * @return array An array with the favorite.
  */
  private static function buildResult($id, $type) {
    return [
        "id" =&gt; $id,
        "type" =&gt; $type
    ];
  }
}


An usage example. Users favorite tourism Places.

$favoritesService = new FavoritesService(
  'MY_SECRET_API_KEY',
  ['HOTEL', 'REGION', 'POI']
);

$favoritesService-&gt;save(1, 123, 'HOTEL');
$favoritesService-&gt;save(1, 321, 'REGION');
print_r($favoritesService-&gt;get(1));

$favoritesService-&gt;reset(1);
print_r($favoritesService-&gt;get(1));


My question, is this a good OOP design? 

Is there a better naming than Service? It seems to generic to me.
I would agree with your concern over naming, as this doesn't really seems to be a "service", but rather a simple data store.  So perhaps FavoritesStore or similar is more appropriate.  You have some portions of code that are omitted that might suggest a more permanent data store being used in conjunction with this class, so perhaps this is really acting as a cache (FavoritesCache)?



So favorites seem to be their own entity in this context that should probably have their own class representation(s).

You might consider a structure like:

abstract class Favorite {
    const TYPE;
    public $id;
    public $ownerId;

    public function __construct($id, $ownerId)
    {
        $this-&gt;validatePositiveInteger($id);
        $this-&gt;validatePositiveInteger($ownerId);
        $this-&gt;id = $id;
        $this-&gt;ownerId = $ownerId;
    }

    protected function validatePositiveInteger($int)
    {
        if(!is_int($int) || $int &lt; 1) {
            throw new InvalidArgumentException(
                'Positive integer value expected'
            );
        }
    }
}

class FavoriteHotel extends Favorite {
    const TYPE = 'Hotel';
}

class FavoriteRegion extends Favorite {
    const TYPE = 'Region';
}

// etc.


This also allows you to potentially tie different properties/behaviors to different types of favorites.  If you don't have the need to customize different favorites and are concerned over creating/managing a large number of favorite classes, you could alternately have just a single concrete class where upon instantiation you check against a valid list of favorite types.  In this case, perhaps consider extending this class from SplEnum so you can get enum-like behavior for your favorite types.

This would take the responsibility out of the store to enforce favorite types.

Also if you are treating this as a cache and your Favorite as a model for some record somewhere in a permanent store (i.e. database).  Then perhaps this class has methods on it to persist to permanent store or instantiate favorites objects from the permanent store.

So perhaps something like:

public function persist()
{
    // your logic to save to permanent store
}

public static function getInstanceById($id)
{
    // access your permanent store to get single record
    $class = 'Favorite' . $row-&gt;type;
    return new $class($row-&gt;id, $row-&gt;ownerId);
}

public static function getInstancesForOwnerId($ownerId)
{
    $instances = [];
    // access your permanent store to get records
    // for each record
    $class = 'Favorite' . $row-&gt;type;
    $instances[] = new $class($row-&gt;id, $row-&gt;ownerId);
    return $instances;
}

public static function deleteByOwnerId($ownerId)
{
    // your logic to delete records in permanent store
}


The last three methods might also be considered for separation into their own factory class (i.e. FavoriteFactory) if you need to expand on the means by which you instantiate favorite objects.

The takeaway here is that favorite class(es) represent the data model and own the means for how they are persisted into and recovered from the database.  Your simple in memory store/cache does not and should not need to know or understand this.

Your cache would just need to utilize this methods like:

// in save function after setting to cache
$favorite-&gt;persist();

// in get function when you have cache miss
$favorites = Favorite::getInstancesForOwnerId($ownerId);

// in reset function
$result = Favorite::deleteByOwnerId($ownerId);




I don't understand why you have buildResult method or have it exposed statically.  Do you really need a method to build an array? Why is this not just done in save().

i.e.

$this-&gt;data[$ownerId][] = ['id': $id, 'type': $type];


Also, if you consider making favorite into class as suggested, you can clean up your save() code like this:

public function save(Favorite $favorite) {
    $this-&gt;data[$favorite-&gt;ownerId][] = $favorite;
    // other stuff
    return true;
}




I don't see what API key is doing in this class, as it is not used anywhere.  I would also question why an API key would migrate as far down the call stack as where this store probably lives.  Typically, if you are exposing a service for GET'ing, POST'ing, PUT'ing, DELETE'ing data in a store, end user authentication via a mechanism such as an API key would be one of the first things done in validating the request. If the key is not valid, you should fail out the request with a 403 or whatever long before you get to the point of trying to set up data storage objects such as this. 



Consider changing save method name to set and reset to unset as I think you would find these more typical method names for setting and unsetting items in a store or cache.



You are inconsistent in your approach to validating data passed to your public methods.  You do throw InvalidArgumentException in one case, but you do nothing to validate integer values for example.  Of course, that can get taken care of quite easily in some cases if you take the suggestion to put favorite into class that can be type-hinted against.




  /**
   * string
   */
  private $key;



Glad to see you using doc blocks, however these property blocks should probably be changed to @var string, @var array, etc.



You code indentation is inconsistent.



Consider making your code compliant against PSR-1 and PSR-2 since you are so close to being compliant already (i.e. you have generally good coding style).

That would require you to:


add namespace declaration
move opening brace for methods/functions onto line after declaration


The professional PHP development community has really gravitated towards the PSR standards in recent years, so probably best to be aware of them (and hopefully adopt them).  If you can configure you IDE and/or use style checker tools to enforce the coding standards, that is always a good approach.