ronantakizawa/codereview-bench huggingface dataset:
Repository: xbmc/xbmc
File: xbmc/playlists/SmartPlayList.cpp (C++)

Code before:
      return false;

  num = stoi(str);
  return true;
}

bool CSmartPlaylistRule::ValidateDate(const std::string& input, void* data)
{
  if (!data)
    return false;

  CSmartPlaylistRule* rule = static_cast<CSmartPlaylistRule*>(data);

  //! @todo implement a validation for relative dates
  if (rule->m_operator == CDatabaseQueryRule::OPERATOR_IN_THE_LAST ||
      rule->m_operator == CDatabaseQueryRule::OPERATOR_NOT_IN_THE_LAST)
    return true;

  // must be YYYY-MM-DD
  if (input.size() != 10)
    return false;

  if (input[4] != '-' || input[7] != '-')
    return false;

  int year, month, day;

  if (!StrToInt(input.substr(0, 4), year))
    return false;

  if (!StrToInt(input.substr(5, 2), month))
    return false;

  if (!StrToInt(input.substr(8, 2), day))
    return false;

  CDateTime dt;
  return dt.SetDate(year, month, day);
}

std::vector<Field> CSmartPlaylistRule::GetFields(const std::string &type)
{
  std::vector<Field> fields;
  bool isVideo = false;
  if (type == "mixed")
  {
    fields.push_back(FieldGenre);
    fields.push_back(FieldAlbum);
    fields.push_back(FieldArtist);
    fields.push_back(FieldAlbumArtist);
    fields.push_back(FieldTitle);

Review comment (suggestion):
https://github.com/xbmc/xbmc/blob/master/docs/CODE_GUIDELINES.md#52-multiple-declarations

And I would define the vars close to where they are actually used for the first time.

Code after:
{
template<typename T>
std::optional<T> ToNumeric(std::string_view str)
{
  const char* end{str.data() + str.size()};
  T result{};

  auto [ptr, ec] = std::from_chars(str.data(), end, result);

  if (ec != std::errc{} || ptr != end || result < 0)
    return std::nullopt;

  return result;
}
} // namespace

bool CSmartPlaylistRule::ValidateDate(const std::string& input, void* data)
{
  if (!data)
    return false;

  CSmartPlaylistRule* rule = static_cast<CSmartPlaylistRule*>(data);

  //! @todo implement a validation for relative dates
  if (rule->m_operator == CDatabaseQueryRule::OPERATOR_IN_THE_LAST ||
      rule->m_operator == CDatabaseQueryRule::OPERATOR_NOT_IN_THE_LAST)
    return true;

  // The date format must be YYYY-MM-DD
  if (input.size() != 10)
    return false;

  const std::string_view sv{input};

  if (sv[4] != '-' || sv[7] != '-')
    return false;

  const auto year{ToNumeric<int>(sv.substr(0, 4))};
  if (!year.has_value())
    return false;

  const auto month{ToNumeric<int>(sv.substr(5, 2))};
  if (!month.has_value())
    return false;

  const auto day{ToNumeric<int>(sv.substr(8, 2))};
  if (!day.has_value())
    return false;

  CDateTime dt;
  return dt.SetDate(year.value(), month.value(), day.value());
Repository: xbmc/xbmc
File: xbmc/dialogs/GUIDialogSmartPlaylistRule.h (C/C++)

Code before:

class CGUIDialogSmartPlaylistRule : public CGUIDialog
{
public:
  CGUIDialogSmartPlaylistRule(void);
  ~CGUIDialogSmartPlaylistRule(void) override;
  bool OnMessage(CGUIMessage& message) override;
  bool OnBack(int actionID) override;
  void OnInitWindow() override;
  void OnDeinitWindow(int nextWindowID) override;

  static bool EditRule(KODI::PLAYLIST::CSmartPlaylistRule& rule, const std::string& type = "songs");

protected:
  void OnField();
  void OnOperator();
  void OnOK();
  void OnCancel();
  void UpdateButtons();
  void OnBrowse();
  std::vector<std::pair<std::string, int>> GetValidOperators(
      const KODI::PLAYLIST::CSmartPlaylistRule& rule);
  KODI::PLAYLIST::CSmartPlaylistRule m_rule;
  bool m_cancelled;
  std::string m_type;
  CGUIEditControl* m_editControl;
};


Review comment (bug):
default init `CGUIEditControl* m_editControl{nullptr};` for safety.

Code after:
      public CGUIDialog
{
public:
  CGUIDialogSmartPlaylistRule(void);
  ~CGUIDialogSmartPlaylistRule(void) override;
  bool OnMessage(CGUIMessage& message) override;
  bool OnBack(int actionID) override;
  void OnInitWindow() override;
  void OnDeinitWindow(int nextWindowID) override;

  static bool EditRule(KODI::PLAYLIST::CSmartPlaylistRule& rule, const std::string& type = "songs");

protected:
  void OnField();
  void OnOperator();
  void OnOK();
  void OnCancel();
  void UpdateButtons();
  void OnBrowse();
  std::vector<std::pair<std::string, int>> GetValidOperators(
      const KODI::PLAYLIST::CSmartPlaylistRule& rule);
  KODI::PLAYLIST::CSmartPlaylistRule m_rule;
  bool m_cancelled;
  std::string m_type;
};
Repository: xbmc/xbmc
File: xbmc/playlists/SmartPlayList.cpp (C++)

Code before:
}

bool CSmartPlaylistRule::ValidateRating(const std::string &input, void *data)
{
  char *end = NULL;
  std::string strRating = input;
  StringUtils::Trim(strRating);

  double rating = std::strtod(strRating.c_str(), &end);
  return (end == NULL || *end == '\0') &&
         rating >= 0.0 && rating <= 10.0;
}

bool CSmartPlaylistRule::ValidateMyRating(const std::string &input, void *data)
{
  std::string strRating = input;
  StringUtils::Trim(strRating);

  int rating = atoi(strRating.c_str());
  return StringValidation::IsPositiveInteger(input, data) && rating <= 10;
}

namespace
{
template<class T>
std::optional<T> DigitsToNumeric(const std::string_view& str)
{
  if (str.length() > std::numeric_limits<T>::digits10)
    return {};

  T value{0};
  for (unsigned char c : str)
  {
    if (!std::isdigit(c))
      return {};
    value *= 10;
    value += c - '0';
  }
  return value;
}
} // namespace

bool CSmartPlaylistRule::ValidateDate(const std::string& input, void* data)
{
  if (!data)
    return false;

  CSmartPlaylistRule* rule = static_cast<CSmartPlaylistRule*>(data);

  //! @todo implement a validation for relative dates
  if (rule->m_operator == CDatabaseQueryRule::OPERATOR_IN_THE_LAST ||

Review comment (refactor):
The function body is fine as is, but please [pass `std::string_view` by value](https://quuxplusone.github.io/blog/2021/11/09/pass-string-view-by-value/).

As for the function: [`std::from_chars`](https://en.cppreference.com/w/cpp/utility/from_chars) could do all the validations for you but it's also fine as is.

Code after:
bool CSmartPlaylistRule::ValidateRating(const std::string &input, void *data)
{
  char *end = NULL;
  std::string strRating = input;
  StringUtils::Trim(strRating);

  double rating = std::strtod(strRating.c_str(), &end);
  return (end == NULL || *end == '\0') &&
         rating >= 0.0 && rating <= 10.0;
}

bool CSmartPlaylistRule::ValidateMyRating(const std::string &input, void *data)
{
  std::string strRating = input;
  StringUtils::Trim(strRating);

  int rating = atoi(strRating.c_str());
  return StringValidation::IsPositiveInteger(input, data) && rating <= 10;
}

namespace
{
template<typename T>
std::optional<T> ToNumeric(std::string_view str)
{
  const char* end{str.data() + str.size()};
  T result{};

  auto [ptr, ec] = std::from_chars(str.data(), end, result);

  if (ec != std::errc{} || ptr != end || result < 0)
    return std::nullopt;

  return result;
}
} // namespace

bool CSmartPlaylistRule::ValidateDate(const std::string& input, void* data)
{
  if (!data)
    return false;

  CSmartPlaylistRule* rule = static_cast<CSmartPlaylistRule*>(data);

  //! @todo implement a validation for relative dates
  if (rule->m_operator == CDatabaseQueryRule::OPERATOR_IN_THE_LAST ||
      rule->m_operator == CDatabaseQueryRule::OPERATOR_NOT_IN_THE_LAST)
    return true;

  // The date format must be YYYY-MM-DD
  if (input.size() != 10)
Repository: xbmc/xbmc
File: xbmc/playlists/SmartPlayList.cpp (C++)

Code before:
{
  if (data == NULL)
    return true;

  CSmartPlaylistRule *rule = static_cast<CSmartPlaylistRule*>(data);

  // check if there's a validator for this rule
  StringValidation::Validator validator = NULL;
  for (const translateField& field : fields)
  {
    if (rule->m_field == field.field)
    {
        validator = field.validator;
        break;
    }
  }
  if (validator == NULL)
    return true;

  // split the input into multiple values and validate every value separately
  std::vector<std::string> values;
  if (input.empty())
    values = {""};
  else
    values = StringUtils::Split(input, RULE_VALUE_SEPARATOR);

  return (std::all_of(values.begin(), values.end(),
                      [data, validator](const auto& s) { return validator(s, data); }));
}

bool CSmartPlaylistRule::ValidateRating(const std::string &input, void *data)
{
  char *end = NULL;
  std::string strRating = input;
  StringUtils::Trim(strRating);

  double rating = std::strtod(strRating.c_str(), &end);
  return (end == NULL || *end == '\0') &&
         rating >= 0.0 && rating <= 10.0;
}

bool CSmartPlaylistRule::ValidateMyRating(const std::string &input, void *data)
{
  std::string strRating = input;
  StringUtils::Trim(strRating);

  int rating = atoi(strRating.c_str());
  return StringValidation::IsPositiveInteger(input, data) && rating <= 10;
}

namespace

Review comment (suggestion):
I don't think we need to handle empty input special. `StringUtils::Split` won't insert anything into values and `std::all_of` on an empty range is `true`.

Code after:
{
  if (data == NULL)
    return true;

  CSmartPlaylistRule *rule = static_cast<CSmartPlaylistRule*>(data);

  // check if there's a validator for this rule
  StringValidation::Validator validator = NULL;
  for (const translateField& field : fields)
  {
    if (rule->m_field == field.field)
    {
        validator = field.validator;
        break;
    }
  }
  if (validator == NULL)
    return true;

  if (input.empty())
    return validator("", data);

  // Split the input into multiple values and validate every value separately
  const std::vector<std::string> values{StringUtils::Split(input, RULE_VALUE_SEPARATOR)};

  return (std::all_of(values.begin(), values.end(),
                      [data, validator](const auto& s) { return validator(s, data); }));
}

bool CSmartPlaylistRule::ValidateRating(const std::string &input, void *data)
{
  char *end = NULL;
  std::string strRating = input;
  StringUtils::Trim(strRating);

  double rating = std::strtod(strRating.c_str(), &end);
  return (end == NULL || *end == '\0') &&
         rating >= 0.0 && rating <= 10.0;
}

bool CSmartPlaylistRule::ValidateMyRating(const std::string &input, void *data)
{
  std::string strRating = input;
  StringUtils::Trim(strRating);

  int rating = atoi(strRating.c_str());
  return StringValidation::IsPositiveInteger(input, data) && rating <= 10;
}

namespace
{
Repository: xbmc/xbmc
File: xbmc/video/dialogs/GUIDialogVideoManagerExtras.cpp (C++)

Code before:
}

int CGUIDialogVideoManagerExtras::NewVideoAssetId(const CFileItem& item)
{
  const std::string typeNewVideoVersion{
      CGUIDialogVideoManagerExtras::GenerateVideoExtra(URIUtils::GetFileName(item.GetDynPath()))};

  return m_database.AddVideoVersionType(typeNewVideoVersion, VideoAssetTypeOwner::AUTO,
                                        VideoAssetType::EXTRA);
}

CGUIDialogVideoManager::ManagerMessages CGUIDialogVideoManagerExtras::InitializeMessages()
{
  ManagerMessages messages;
  messages.ManagerTitleId = 40025;
  messages.RemoveTitleId = 40039;
  messages.RemoveTextId = 40040;
  messages.AddDialogTitleId = 40015;
  messages.AddNoDefaultMovieVersionId = 40034;

  return messages;
}

const CGUIDialogVideoManager::ManagerMessages& CGUIDialogVideoManagerExtras::DialogMessages()
{
  static ManagerMessages msg{InitializeMessages()};
  return msg;
}


Review comment (suggestion):
I don't think you need the `InitializeMessages` method.

Doesn't `static ManagerMessages msg{InitializeMessages{40025, 40039, 40040, 40015, 40034}};` work? Maybe you need to define a respective ctor for `ManagerMessages`.

Code after:
        else
        {
          CLog::LogF(LOGERROR, "unexpected asset type {}", static_cast<int>(newAsset.m_assetType));
          return false;
        }

        CGUIDialogOK::ShowAndGetInput(
            CVariant{40015},
            StringUtils::Format(g_localizeStrings.Get(msgid), newAsset.m_assetTypeName));
        return false;
      }

      // The video is an asset of another movie or different asset type of same movie

      // The video is a version, ask for confirmation of the asset type change
      if (newAsset.m_assetType == VideoAssetType::VERSION &&
          !CGUIDialogYesNo::ShowAndGetInput(CVariant{40015},
                                            StringUtils::Format(g_localizeStrings.Get(40036))))
      {
        return false;
      }

      std::string videoTitle;
      if (newAsset.m_mediaType == MediaTypeMovie)
      {
        videoTitle = m_database.GetMovieTitle(newAsset.m_idMedia);
      }
      else
        return false;

      if (newAsset.m_idMedia != dbId && newAsset.m_mediaType == mediaType)
      {
        unsigned int msgid{};

        if (newAsset.m_assetType == VideoAssetType::VERSION)
          msgid = 40017; // video is a version of another movie
        else if (newAsset.m_assetType == VideoAssetType::EXTRA)
          msgid = 40027; // video is an extra of another movie
        else
        {
          CLog::LogF(LOGERROR, "unexpected asset type {}", static_cast<int>(newAsset.m_assetType));
          return false;
        }

        if (!CGUIDialogYesNo::ShowAndGetInput(
                CVariant{40015}, StringUtils::Format(g_localizeStrings.Get(msgid),
                                                     newAsset.m_assetTypeName, videoTitle)))
        {
          return false;
        }
      }
Repository: xbmc/xbmc
File: xbmc/cores/VideoPlayer/DVDCodecs/Video/DVDVideoCodec.cpp (C++)

Code before:
/*
 *  Copyright (C) 2010-2018 Team Kodi
 *  This file is part of Kodi - https://kodi.tv
 *
 *  SPDX-License-Identifier: GPL-2.0-or-later
 *  See LICENSES/README.md for more information.
 */

#include "DVDVideoCodec.h"

#include "cores/VideoPlayer/DVDCodecs/DVDFactoryCodec.h"

#include <string>
#include <vector>

//******************************************************************************
// VideoPicture
//******************************************************************************

VideoPicture::VideoPicture() = default;

VideoPicture::~VideoPicture()
{
  if (videoBuffer)
  {
    videoBuffer->Release();
  }
}

void VideoPicture::Reset()
{
  if (videoBuffer)
    videoBuffer->Release();
  videoBuffer = nullptr;
  pts = DVD_NOPTS_VALUE;
  dts = DVD_NOPTS_VALUE;
  iFlags = 0;
  iRepeatPicture = 0;
  iDuration = 0;
  iFrameType = 0;
  color_space = AVCOL_SPC_UNSPECIFIED;
  color_range = 0;
  chroma_position = AVCHROMA_LOC_UNSPECIFIED;
  color_primaries = AVColorPrimaries::AVCOL_PRI_UNSPECIFIED;
  color_transfer = AVCOL_TRC_UNSPECIFIED;
  colorBits = 8;

Review comment (suggestion):
Sometimes having this in the cpp is the better approach. Namely, if the class has members which are forward declared, then this forward declare has to be replaced with an include in the class' header file. Otherwise the compiler does not know how to compile the function it generates. Just as a note, seems to work just fine here without need to include other headers.

Code after:
/*
 *  Copyright (C) 2010-2018 Team Kodi
 *  This file is part of Kodi - https://kodi.tv
 *
 *  SPDX-License-Identifier: GPL-2.0-or-later
 *  See LICENSES/README.md for more information.
 */

#include "DVDVideoCodec.h"

#include "cores/VideoPlayer/DVDCodecs/DVDFactoryCodec.h"

#include <string>
#include <vector>

//******************************************************************************
// VideoPicture
//******************************************************************************
VideoPicture::~VideoPicture()
{
  if (videoBuffer)
  {
    videoBuffer->Release();
  }
}

void VideoPicture::Reset()
{
  if (videoBuffer)
    videoBuffer->Release();
  videoBuffer = nullptr;
  pts = DVD_NOPTS_VALUE;
  dts = DVD_NOPTS_VALUE;
  iFlags = 0;
  iRepeatPicture = 0;
  iDuration = 0;
  iFrameType = 0;
  color_space = AVCOL_SPC_UNSPECIFIED;
  color_range = 0;
  chroma_position = AVCHROMA_LOC_UNSPECIFIED;
  color_primaries = AVColorPrimaries::AVCOL_PRI_UNSPECIFIED;
  color_transfer = AVCOL_TRC_UNSPECIFIED;
  colorBits = 8;
  stereoMode.clear();

  qp_table = nullptr;
Repository: xbmc/xbmc
File: xbmc/video/dialogs/GUIDialogVideoBookmarks.cpp (C++)

Code before:
bool CGUIDialogVideoBookmarks::OnAction(const CAction &action)
{
  switch(action.GetID())
  {
  case ACTION_CONTEXT_MENU:
  case ACTION_MOUSE_RIGHT_CLICK:
    {
      OnPopupMenu(m_viewControl.GetSelectedItem());
      return true;
    }
  }
  return CGUIDialog::OnAction(action);
}

int CGUIDialogVideoBookmarks::ItemToBookmarkIndex(int item)
{
  if (item < 0 || item >= (int)m_vecItems->Size())
    return -1;

  std::shared_ptr<CFileItem> fileItem = m_vecItems->Get(item);

  if (!fileItem->GetProperty("isbookmark").asBoolean(false))
    return -1;

  const int bookmarkIdx{fileItem->GetProperty("bookmark").asInteger32(-1)};
  if (bookmarkIdx < 0 || bookmarkIdx >= m_bookmarks.size())
  {
    CLog::LogF(LOGERROR, "invalid bookmark index {} for {} bookmark(s)", bookmarkIdx,
               m_bookmarks.size());
    return -1;
  }
  return bookmarkIdx;
}


void CGUIDialogVideoBookmarks::OnPopupMenu(int item)
{
  const int bookmarkIdx{ItemToBookmarkIndex(item)};
  if (bookmarkIdx < 0)
    return;

  const CBookmark& bm{m_bookmarks[bookmarkIdx]};

  // highlight the item
  (*m_vecItems)[item]->Select(true);

  CContextButtons choices;
  choices.Add(1, (bm.type == CBookmark::EPISODE
                      ? 20405
                      : 20404)); // "Remove episode bookmark" or "Remove bookmark"


Review comment (suggestion):
This leads to a „potential data loss“ warning. Add `static_cast<int>(m_book…`

Code after:
bool CGUIDialogVideoBookmarks::OnAction(const CAction &action)
{
  switch(action.GetID())
  {
  case ACTION_CONTEXT_MENU:
  case ACTION_MOUSE_RIGHT_CLICK:
    {
      OnPopupMenu(m_viewControl.GetSelectedItem());
      return true;
    }
  }
  return CGUIDialog::OnAction(action);
}

int CGUIDialogVideoBookmarks::ItemToBookmarkIndex(int item) const
{
  if (item < 0 || item >= static_cast<int>(m_vecItems->Size()))
    return -1;

  const std::shared_ptr<CFileItem> fileItem{m_vecItems->Get(item)};

  if (!fileItem->GetProperty("isbookmark").asBoolean(false))
    return -1;

  const int bookmarkIdx{fileItem->GetProperty("bookmark").asInteger32(-1)};
  if (bookmarkIdx < 0 || bookmarkIdx >= static_cast<int>(m_bookmarks.size()))
  {
    CLog::LogF(LOGERROR, "invalid bookmark index {} for {} bookmark(s)", bookmarkIdx,
               m_bookmarks.size());
    return -1;
  }
  return bookmarkIdx;
}

void CGUIDialogVideoBookmarks::OnPopupMenu(int item)
{
  const int bookmarkIdx{ItemToBookmarkIndex(item)};
  if (bookmarkIdx < 0)
    return;

  const CBookmark& bm{m_bookmarks[bookmarkIdx]};

  // highlight the item
  (*m_vecItems)[item]->Select(true);

  CContextButtons choices;
  choices.Add(1, (bm.type == CBookmark::EPISODE
                      ? 20405
                      : 20404)); // "Remove episode bookmark" or "Remove bookmark"

  const int button{CGUIDialogContextMenu::ShowAndGetChoice(choices)};
Repository: xbmc/xbmc
File: xbmc/video/dialogs/GUIDialogVideoBookmarks.cpp (C++)

Code before:
  (*m_vecItems)[item]->Select(true);

  CContextButtons choices;
  choices.Add(1, (bm.type == CBookmark::EPISODE
                      ? 20405
                      : 20404)); // "Remove episode bookmark" or "Remove bookmark"

  const int button{CGUIDialogContextMenu::ShowAndGetChoice(choices)};

  // unhighlight the item
  (*m_vecItems)[item]->Select(false);

  if (button == 1)
    Delete(bm);
}

void CGUIDialogVideoBookmarks::Delete(int item)
{
  const int bookmarkIdx{ItemToBookmarkIndex(item)};
  if (bookmarkIdx >= 0)
    Delete(m_bookmarks[bookmarkIdx]);
}

void CGUIDialogVideoBookmarks::Delete(const CBookmark& bm)
{
  CVideoDatabase videoDatabase;
  videoDatabase.Open();
  const std::string path{g_application.CurrentFileItem().GetDynPath()};
  videoDatabase.ClearBookMarkOfFile(path, bm, bm.type);
  videoDatabase.Close();
  CUtil::DeleteVideoDatabaseDirectoryCache();
  Update();
}

void CGUIDialogVideoBookmarks::OnRefreshList()
{
  m_bookmarks.clear();
  std::vector<CFileItemPtr> items;

  // open the d/b and retrieve the bookmarks for the current movie
  m_filePath = g_application.CurrentFileItem().GetDynPath();

  CVideoDatabase videoDatabase;
  videoDatabase.Open();
  videoDatabase.GetBookMarksForFile(m_filePath, m_bookmarks);
  videoDatabase.GetBookMarksForFile(m_filePath, m_bookmarks, CBookmark::EPISODE, true);
  videoDatabase.Close();

  std::unique_lock<CCriticalSection> lock(m_refreshSection);
  m_vecItems->Clear();


Review comment (suggestion):
You should handle videodb open not succeeding. At least that’s what we’re doing in most places on our code base. There must be a reason for this. 

Code after:

  CContextButtons choices;
  choices.Add(1, (bm.type == CBookmark::EPISODE
                      ? 20405
                      : 20404)); // "Remove episode bookmark" or "Remove bookmark"

  const int button{CGUIDialogContextMenu::ShowAndGetChoice(choices)};

  // unhighlight the item
  (*m_vecItems)[item]->Select(false);

  if (button == 1)
    Delete(bm);
}

void CGUIDialogVideoBookmarks::Delete(int item)
{
  const int bookmarkIdx{ItemToBookmarkIndex(item)};
  if (bookmarkIdx >= 0)
    Delete(m_bookmarks[bookmarkIdx]);
}

void CGUIDialogVideoBookmarks::Delete(const CBookmark& bm)
{
  CVideoDatabase videoDatabase;
  if (!videoDatabase.Open())
    return;

  const std::string path{g_application.CurrentFileItem().GetDynPath()};
  videoDatabase.ClearBookMarkOfFile(path, bm, bm.type);
  videoDatabase.Close();
  CUtil::DeleteVideoDatabaseDirectoryCache();
  Update();
}

void CGUIDialogVideoBookmarks::OnRefreshList()
{
  m_bookmarks.clear();
  std::vector<CFileItemPtr> items;

  // open the d/b and retrieve the bookmarks for the current movie
  m_filePath = g_application.CurrentFileItem().GetDynPath();

  CVideoDatabase videoDatabase;
  if (!videoDatabase.Open())
    return;

  videoDatabase.GetBookMarksForFile(m_filePath, m_bookmarks);
  videoDatabase.GetBookMarksForFile(m_filePath, m_bookmarks, CBookmark::EPISODE, true);
  videoDatabase.Close();