Repository: xbmc/xbmc
File: xbmc/video/VideoInfoScanner.cpp (C++)

Code before:
    int movieYear = -1; // hint that movie title was not found
    if (result == InfoType::TITLE)
    {
      CVideoInfoTag* tag = pItem->GetVideoInfoTag();
      movieTitle = tag->GetTitle();
      movieYear = tag->GetYear(); // movieYear is expected to be >= 0
    }

    std::string identifierType;
    std::string identifier;
    long lResult = -1;
    if (info2->IsPython() && CUtil::GetFilenameIdentifier(movieTitle, identifierType, identifier))
    {
      const std::unordered_map<std::string, std::string> uniqueIDs{{identifierType, identifier}};
      if (GetDetails(pItem, uniqueIDs, url, info2,
                     (result == InfoType::COMBINED || result == InfoType::OVERRIDE) ? loader.get()
                                                                                    : nullptr,
                     pDlgProgress))
      {
        if ((lResult = AddVideo(pItem, info2->Content(), false, useLocal)) < 0)
          return InfoRet::INFO_ERROR;

        if (fetchEpisodes)
        {
          InfoRet ret = RetrieveInfoForEpisodes(pItem, lResult, info2, useLocal, pDlgProgress);
          if (ret == InfoRet::ADDED)
          {
            m_database.SetPathHash(pItem->GetPath(), pItem->GetProperty("hash").asString());
            return InfoRet::ADDED;
          }
        }
        return InfoRet::ADDED;
      }
    }

    if (pURL && pURL->HasUrls())
      url = *pURL;
    else if ((retVal = FindVideo(movieTitle, movieYear, info2, url, pDlgProgress)) <= 0)
      return retVal < 0 ? InfoRet::CANCELLED : InfoRet::NOT_FOUND;

    CLog::Log(LOGDEBUG, "VideoInfoScanner: Fetching url '{}' using {} scraper (content: '{}')",
              url.GetFirstThumbUrl(), info2->Name(), TranslateContent(info2->Content()));
    const std::unordered_map<std::string, std::string> uniqueIDs{{identifierType, identifier}};

    if (GetDetails(pItem, {}, url, info2,
                   (result == InfoType::COMBINED || result == InfoType::OVERRIDE) ? loader.get()
                                                                                  : nullptr,
                   pDlgProgress))
    {
      if ((lResult = AddVideo(pItem, info2->Content(), false, useLocal)) < 0)
        return InfoRet::INFO_ERROR;

Review comment (suggestion):
I know I'm late to the party, but from only reading this line, who could tell what this newly introduced `true`means? That's why I'm trying to avoid bool params, knowing that introducing an enum class to model the two possible values is a bit more work, but produces far more readable code. @78andyp

Same applies for default parameters, which are most of the time just laziness of the person introducing a new parameter, but making the code more complicated to read and maintain. I try to avoid default params whenever I can.

And default bool params are doubling my pain. :-)

Code after:
    int movieYear = -1; // hint that movie title was not found
    if (result == InfoType::TITLE)
    {
      CVideoInfoTag* tag = pItem->GetVideoInfoTag();
      movieTitle = tag->GetTitle();
      movieYear = tag->GetYear(); // movieYear is expected to be >= 0
    }

    std::string identifierType;
    std::string identifier;
    long lResult = -1;
    if (info2->IsPython() && CUtil::GetFilenameIdentifier(movieTitle, identifierType, identifier))
    {
      const std::unordered_map<std::string, std::string> uniqueIDs{{identifierType, identifier}};
      if (GetDetails(pItem, uniqueIDs, url, info2,
                     (result == InfoType::COMBINED || result == InfoType::OVERRIDE) ? loader.get()
                                                                                    : nullptr,
                     pDlgProgress))
      {
        if ((lResult = AddVideo(pItem, info2->Content(), false, useLocal)) < 0)
          return InfoRet::INFO_ERROR;

        if (fetchEpisodes)
        {
          InfoRet ret =
              RetrieveInfoForEpisodes(pItem, lResult, info2, useLocal, pDlgProgress, true);
          if (ret == InfoRet::ADDED)
          {
            m_database.SetPathHash(pItem->GetPath(), pItem->GetProperty("hash").asString());
            return InfoRet::ADDED;
          }
        }
        return InfoRet::ADDED;
      }
    }

    if (pURL && pURL->HasUrls())
      url = *pURL;
    else if ((retVal = FindVideo(movieTitle, movieYear, info2, url, pDlgProgress)) <= 0)
      return retVal < 0 ? InfoRet::CANCELLED : InfoRet::NOT_FOUND;

    CLog::Log(LOGDEBUG, "VideoInfoScanner: Fetching url '{}' using {} scraper (content: '{}')",
              url.GetFirstThumbUrl(), info2->Name(), TranslateContent(info2->Content()));
    const std::unordered_map<std::string, std::string> uniqueIDs{{identifierType, identifier}};

    if (GetDetails(pItem, {}, url, info2,
                   (result == InfoType::COMBINED || result == InfoType::OVERRIDE) ? loader.get()
                                                                                  : nullptr,
                   pDlgProgress))
    {
      if ((lResult = AddVideo(pItem, info2->Content(), false, useLocal)) < 0)