Почему этот код работает для 8 различных тестовых случаев, кроме 1 в C ++? - PullRequest
0 голосов
/ 09 апреля 2020

Итак, я написал эту функцию на C ++, которая в основном подсчитывает максимальное число в массиве, а затем распечатывает количество максимальных чисел в массиве. Вот код функции:

int Number_of_maxNum(vector<int> ar) {
      int max=0;
      int Number_of_Maxnum=0;
      int d = ar.size();

      for(int i=0;i<=d;i++){
          if(ar[i]>max){
          max=ar[i];
          }
      }

      for(int j=0;j<=d;j++){
          if(ar[j]==max){
             Number_of_Maxnum++;
          }
      }

      return Number_of_Maxnum;

}

Теперь этот код, однако, не работает для следующего массива в качестве ввода: {44, 53, 31, 27, 77, 60, 66, 77, 26, 36} Он должен распечатать 2, но распечатать 1

Если кто-то может объяснить, что на самом деле происходит с этим входом, который дает 1 в качестве входа, Это будет

Ответы [ 2 ]

4 голосов
/ 09 апреля 2020

У вас есть неопределенное поведение. Массивы / векторы индексируются от 0 до Size-1. Так что измените i<=d на i<d. Это, скорее всего, причина этого странного результата. Поскольку вы читаете свой вектор за пределами его границы, в результате получается (фактически) случайное последнее значение (обратите внимание, что это UB, оно может даже обработать sh всю вашу программу).

Другое дело, что вы должны инициализировать int max = std::numeric_limits<int>::min();, если вы не гарантируете, что все элементы ar неотрицательны.

Наконец, вы можете выполнить всю обработку за один l oop. Попробуйте это:

int Number_of_maxNum(const vector<int>& ar)  // <--- do this to avoid vector copy
{
      int currentMax = std::numeric_limits<int>::min();
      int counter = 0;
      for (int value : ar)  // <--- do this to avoid error prone manual indexing
      {
          if (value == currentMax)
          {
              counter++;
          }
          else if (value > currentMax)
          {
              currentMax = value;
              counter = 1;
          }
      }
      return counter;
}
1 голос
/ 09 апреля 2020

Принятый ответ, безусловно, правильный, а также объясняет, почему ваш код неверен.

Тем не менее, вы также должны рассмотреть возможность использования STL, чтобы делать то, что вам нужно, например:

int Number_of_maxNum(const std::vector<int>& ar)  
{
  if (ar.size() == 0)
    return 0;

  auto max = *std::max_element(ar.cbegin(), ar.cend());

  return std::count(ar.cbegin(), ar.cend(), max);
}

Вот некоторые из преимуществ:

  1. Проще прочитайте (и напишите, как только вы к этому привыкнете).
  2. Нет проблем с ошибками "один на один" (как у вас было в вашем решении).
  3. Не беспокойтесь об инициализации максимальное число, чтобы быть наименьшим возможным числом.

Одним из недостатков этого решения является то, что оно зацикливается на векторе дважды. Этого все еще можно избежать, используя соответствующий алгоритм, например,

int Number_of_maxNum(const std::vector<int>& ar)  
{
  return std::accumulate(ar.cbegin(), ar.cend(), 0, 
          [max = std::numeric_limits<int>::min()]  
          (int count, int num) mutable {
             return num > max ? max = num, 1 : count + (num == max);
  });
}

Это фактически обычный метод для -l oop, поэтому я не уверен, что, написав его таким образом, можно многого добиться. Кроме того, изменчивые лямбды можно считать запахом кода. Вы должны использовать свое суждение, чтобы решить, какую технику использовать, когда узнаете о возможных вариантах.

Добро пожаловать на сайт PullRequest, где вы можете задавать вопросы и получать ответы от других членов сообщества.
...