| Total Complexity | 44 |
| Total Lines | 286 |
| Duplicated Lines | 0 % |
| Changes | 1 | ||
| Bugs | 0 | Features | 0 |
Complex classes like AlbumBusinessLayer often do a lot of different things. To break such a class down, we need to identify a cohesive component within that class. A common approach to find such a component is to look for fields/methods that share the same prefixes, or suffixes.
Once you have determined the fields that belong together, you can apply the Extract Class refactoring. If the component makes sense as a sub-class, Extract Subclass is also a candidate, and is often faster.
While breaking up the class, it is a good idea to analyze how other classes use AlbumBusinessLayer, and based on these observations, apply Extract Interface, too.
| 1 | <?php declare(strict_types=1); |
||
| 30 | class AlbumBusinessLayer extends BusinessLayer { |
||
| 31 | protected $mapper; // eclipse the definition from the base class, to help IDE and Scrutinizer to know the actual type |
||
| 32 | private $logger; |
||
| 33 | |||
| 34 | public function __construct(AlbumMapper $albumMapper, Logger $logger) { |
||
| 35 | parent::__construct($albumMapper); |
||
| 36 | $this->mapper = $albumMapper; |
||
| 37 | $this->logger = $logger; |
||
| 38 | } |
||
| 39 | |||
| 40 | /** |
||
| 41 | * {@inheritdoc} |
||
| 42 | * @see BusinessLayer::find() |
||
| 43 | * @return Album |
||
| 44 | */ |
||
| 45 | public function find(int $albumId, string $userId) : Entity { |
||
| 46 | $album = parent::find($albumId, $userId); |
||
| 47 | return $this->injectExtraFields([$album], $userId)[0]; |
||
| 48 | } |
||
| 49 | |||
| 50 | /** |
||
| 51 | * {@inheritdoc} |
||
| 52 | * @see BusinessLayer::findAll() |
||
| 53 | * @return Album[] |
||
| 54 | */ |
||
| 55 | public function findAll(string $userId, int $sortBy=SortBy::None, int $limit=null, int $offset=null, |
||
| 56 | ?string $createdMin=null, ?string $createdMax=null, ?string $updatedMin=null, ?string $updatedMax=null) : array { |
||
| 57 | $albums = parent::findAll($userId, $sortBy, $limit, $offset, $createdMin, $createdMax, $updatedMin, $updatedMax); |
||
| 58 | $effectivelyLimited = ($limit !== null && $limit < \count($albums)); |
||
| 59 | $everyAlbumIncluded = (!$effectivelyLimited && !$offset && !$createdMin && !$createdMax && !$updatedMin && !$updatedMax); |
||
|
|
|||
| 60 | return $this->injectExtraFields($albums, $userId, $everyAlbumIncluded); |
||
| 61 | } |
||
| 62 | |||
| 63 | /** |
||
| 64 | * Returns all albums filtered by artist (both album and track artists are considered) |
||
| 65 | * @param integer $artistId the id of the artist |
||
| 66 | * @param string $userId the name of the user |
||
| 67 | * @return Album[] albums |
||
| 68 | */ |
||
| 69 | public function findAllByArtist(int $artistId, string $userId) : array { |
||
| 70 | $albums = $this->mapper->findAllByArtist($artistId, $userId); |
||
| 71 | return $this->injectExtraFields($albums, $userId); |
||
| 72 | } |
||
| 73 | |||
| 74 | /** |
||
| 75 | * Returns all albums filtered by album artist |
||
| 76 | * @param integer $artistId the id of the artist |
||
| 77 | * @param string $userId the name of the user |
||
| 78 | * @return Album[] albums |
||
| 79 | */ |
||
| 80 | public function findAllByAlbumArtist(int $artistId, string $userId) : array { |
||
| 81 | $albums = $this->mapper->findAllByAlbumArtist($artistId, $userId); |
||
| 82 | $albums = $this->injectExtraFields($albums, $userId); |
||
| 83 | \usort($albums, ['\OCA\Music\Db\Album', 'compareYearAndName']); |
||
| 84 | return $albums; |
||
| 85 | } |
||
| 86 | |||
| 87 | /** |
||
| 88 | * Returns all albums filtered by genre |
||
| 89 | * @param int $genreId the genre to include |
||
| 90 | * @param string $userId the name of the user |
||
| 91 | * @param int|null $limit |
||
| 92 | * @param int|null $offset |
||
| 93 | * @return Album[] albums |
||
| 94 | */ |
||
| 95 | public function findAllByGenre(int $genreId, string $userId, int $limit=null, int $offset=null) : array { |
||
| 98 | } |
||
| 99 | |||
| 100 | /** |
||
| 101 | * Returns all albums filtered by release year |
||
| 102 | * @param int $fromYear |
||
| 103 | * @param int $toYear |
||
| 104 | * @param string $userId the name of the user |
||
| 105 | * @param int|null $limit |
||
| 106 | * @param int|null $offset |
||
| 107 | * @return Album[] albums |
||
| 108 | */ |
||
| 109 | public function findAllByYearRange( |
||
| 134 | } |
||
| 135 | |||
| 136 | /** |
||
| 137 | * {@inheritdoc} |
||
| 138 | * @see BusinessLayer::findAllByName() |
||
| 139 | * @return Album[] |
||
| 140 | */ |
||
| 141 | public function findAllByName( |
||
| 142 | string $name, string $userId, bool $fuzzy = false, int $limit=null, int $offset=null, |
||
| 143 | ?string $createdMin=null, ?string $createdMax=null, ?string $updatedMin=null, ?string $updatedMax=null) : array { |
||
| 144 | $albums = parent::findAllByName($name, $userId, $fuzzy, $limit, $offset, $createdMin, $createdMax, $updatedMin, $updatedMax); |
||
| 145 | return $this->injectExtraFields($albums, $userId); |
||
| 146 | } |
||
| 147 | |||
| 148 | /** |
||
| 149 | * Add performing artists, release years, genres, and disk counts to the given album objects |
||
| 150 | * @param Album[] $albums |
||
| 151 | * @param string $userId |
||
| 152 | * @param bool $allAlbums Set to true if $albums contains all albums of the user. |
||
| 153 | * This has now effect on the outcome but helps in optimizing |
||
| 154 | * the database query. |
||
| 155 | * @return Album[] |
||
| 156 | */ |
||
| 157 | private function injectExtraFields(array $albums, string $userId, bool $allAlbums = false) : array { |
||
| 158 | if (\count($albums) > 0) { |
||
| 159 | // In case we are injecting data to a lot of albums, do not limit the |
||
| 160 | // SQL SELECTs to only those albums. Very large amount of SQL host parameters |
||
| 161 | // could cause problems with SQLite (see #239) and probably it would be bad for |
||
| 162 | // performance also on other DBMSs. For the proper operation of this function, |
||
| 163 | // it doesn't matter if we fetch data for some extra albums. |
||
| 164 | $albumIds = ($allAlbums || \count($albums) >= 999) |
||
| 165 | ? null : Util::extractIds($albums); |
||
| 166 | |||
| 167 | $artists = $this->mapper->getPerformingArtistsByAlbumId($albumIds, $userId); |
||
| 168 | $years = $this->mapper->getYearsByAlbumId($albumIds, $userId); |
||
| 169 | $diskCounts = $this->mapper->getDiscCountByAlbumId($albumIds, $userId); |
||
| 170 | $genres = $this->mapper->getGenresByAlbumId($albumIds, $userId); |
||
| 171 | |||
| 172 | foreach ($albums as &$album) { |
||
| 173 | $albumId = $album->getId(); |
||
| 174 | $album->setArtistIds($artists[$albumId] ?? []); |
||
| 175 | $album->setNumberOfDisks($diskCounts[$albumId] ?? 1); |
||
| 176 | $album->setGenres($genres[$albumId] ?? null); |
||
| 177 | $album->setYears($years[$albumId] ?? null); |
||
| 178 | } |
||
| 179 | } |
||
| 180 | return $albums; |
||
| 181 | } |
||
| 182 | |||
| 183 | /** |
||
| 184 | * Returns the count of albums where the given Artist is featured in |
||
| 185 | * @param integer $artistId |
||
| 186 | * @return integer |
||
| 187 | */ |
||
| 188 | public function countByArtist(int $artistId) : int { |
||
| 189 | return $this->mapper->countByArtist($artistId); |
||
| 190 | } |
||
| 191 | |||
| 192 | /** |
||
| 193 | * Returns the count of albums where the given artist is the album artist |
||
| 194 | * @param integer $artistId |
||
| 195 | * @return integer |
||
| 196 | */ |
||
| 197 | public function countByAlbumArtist(int $artistId) : int { |
||
| 198 | return $this->mapper->countByAlbumArtist($artistId); |
||
| 199 | } |
||
| 200 | |||
| 201 | public function findAlbumOwner(int $albumId) : string { |
||
| 202 | $entities = $this->findById([$albumId]); |
||
| 203 | if (\count($entities) != 1) { |
||
| 204 | throw new BusinessLayerException( |
||
| 205 | 'Expected to find one album but got ' . \count($entities)); |
||
| 206 | } else { |
||
| 207 | return $entities[0]->getUserId(); |
||
| 208 | } |
||
| 209 | } |
||
| 210 | |||
| 211 | /** |
||
| 212 | * Adds an album if it does not exist already or updates an existing album |
||
| 213 | * @param string|null $name the name of the album |
||
| 214 | * @param integer $albumArtistId |
||
| 215 | * @param string $userId |
||
| 216 | * @return Album The added/updated album |
||
| 217 | */ |
||
| 218 | public function addOrUpdateAlbum(?string $name, int $albumArtistId, string $userId) : Album { |
||
| 219 | $album = new Album(); |
||
| 220 | $album->setName(Util::truncate($name, 256)); // some DB setups can't truncate automatically to column max size |
||
| 221 | $album->setUserId($userId); |
||
| 222 | $album->setAlbumArtistId($albumArtistId); |
||
| 223 | |||
| 224 | // Generate hash from the set of fields forming the album identity to prevent duplicates. |
||
| 225 | // The uniqueness of album name is evaluated in case-insensitive manner. |
||
| 226 | $lowerName = \mb_strtolower($album->getName() ?? ''); |
||
| 227 | $hash = \hash('md5', "$lowerName|$albumArtistId"); |
||
| 228 | $album->setHash($hash); |
||
| 229 | |||
| 230 | return $this->mapper->insertOrUpdate($album); |
||
| 231 | } |
||
| 232 | |||
| 233 | /** |
||
| 234 | * Check if given file is used as cover for the given album |
||
| 235 | * @param int $albumId |
||
| 236 | * @param int[] $fileIds |
||
| 237 | * @return boolean |
||
| 238 | */ |
||
| 239 | public function albumCoverIsOneOfFiles(int $albumId, array $fileIds) : bool { |
||
| 240 | $albums = $this->findById([$albumId]); |
||
| 241 | return (\count($albums) && \in_array($albums[0]->getCoverFileId(), $fileIds)); |
||
| 242 | } |
||
| 243 | |||
| 244 | /** |
||
| 245 | * updates the cover for albums in the specified folder without cover |
||
| 246 | * @param integer $coverFileId the file id of the cover image |
||
| 247 | * @param integer $folderId the file id of the folder where the albums are looked from |
||
| 248 | * @return boolean True if one or more albums were influenced |
||
| 249 | */ |
||
| 250 | public function updateFolderCover(int $coverFileId, int $folderId) : bool { |
||
| 251 | return $this->mapper->updateFolderCover($coverFileId, $folderId); |
||
| 252 | } |
||
| 253 | |||
| 254 | /** |
||
| 255 | * set cover file for a specified album |
||
| 256 | * @param int|null $coverFileId the file id of the cover image |
||
| 257 | * @param int $albumId the id of the album to be modified |
||
| 258 | */ |
||
| 259 | public function setCover(?int $coverFileId, int $albumId) { |
||
| 260 | $this->mapper->setCover($coverFileId, $albumId); |
||
| 261 | } |
||
| 262 | |||
| 263 | /** |
||
| 264 | * removes the cover art from albums, replacement covers will be searched in a background task |
||
| 265 | * @param integer[] $coverFileIds the file IDs of the cover images |
||
| 266 | * @param string[]|null $userIds the users whose music library is targeted; all users are targeted if omitted |
||
| 267 | * @return Album[] albums which got modified, empty array if none |
||
| 268 | */ |
||
| 269 | public function removeCovers(array $coverFileIds, array $userIds=null) : array { |
||
| 270 | return $this->mapper->removeCovers($coverFileIds, $userIds); |
||
| 271 | } |
||
| 272 | |||
| 273 | /** |
||
| 274 | * try to find cover arts for albums without covers |
||
| 275 | * @param string|null $userId target user; omit to target all users |
||
| 276 | * @return string[] users whose collections got modified |
||
| 277 | */ |
||
| 278 | public function findCovers(string $userId = null) : array { |
||
| 279 | $affectedUsers = []; |
||
| 280 | $albums = $this->mapper->getAlbumsWithoutCover($userId); |
||
| 281 | foreach ($albums as $album) { |
||
| 282 | if ($this->mapper->findAlbumCover($album['albumId'], $album['parentFolderId'])) { |
||
| 283 | $affectedUsers[$album['userId']] = 1; |
||
| 284 | } |
||
| 285 | } |
||
| 286 | return \array_keys($affectedUsers); |
||
| 287 | } |
||
| 288 | |||
| 289 | /** |
||
| 290 | * Given an array of Track objects, inject the corresponding Album object to each of them |
||
| 291 | * @param Track[] $tracks (in|out) |
||
| 292 | */ |
||
| 293 | public function injectAlbumsToTracks(array &$tracks, string $userId) { |
||
| 316 | } |
||
| 317 | } |
||
| 318 | } |
||
| 319 |
In PHP, under loose comparison (like
==, or!=, orswitchconditions), values of different types might be equal.For
integervalues, zero is a special case, in particular the following results might be unexpected: