SuperMiQi Ответов: 2

Коллекция объектов в C++ (обзор кода)


Всем Привет,

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

Вот код, которым я хотел бы поделиться с вами:

#include <iostream>
#include <memory>
#include <vector>
#include <algorithm> 

using namespace std;

template<class T, size_t N>
constexpr size_t size(T (&)[N]) { return N; }

class Group
{
private:
    vector<signed long> _channels;
    int                 _id{0};

public:

    Group() = delete;
    Group(signed long channels[], size_t size, int id)
    {
        _channels.assign(channels, channels + size);
        _id = id;
    }

    int getId() const
    {
        return _id;
    }
    
    int channelCount() const
    {
        return _channels.size();
    }
    
    signed long getChannel(int index) const
    {
        if (index < 0 || index >= channelCount())
            throw "Invalid channel index";
            
        return _channels[index];
    }
    
    vector<signed long> Channels() const
    {
        return _channels;
    }
    
};

class Groups
{
private:
    vector<unique_ptr<Group>> _groups;
    int _groupIndex{0};
    
public:

    Groups(){};
    
    int Add(signed long channels[], size_t size)
    {
        _groupIndex++;
        _groups.push_back(make_unique<Group>(channels, size, _groupIndex));
        return _groupIndex;
    }
    
    int Size() const
    {
        return _groups.size();
    }
    
    void Clear() 
    {
        return _groups.clear();
    }
    
    Group operator[](int index)
    {
        return getIten(index);
    }
        
    Group getIten(int index)
    {
        if (index < 0 || index >= Size())
            throw "Invalid group index";
        
        return *_groups[index].get();
    }
    
};

int main()
{
   Groups groups;
   
   signed long grp1Channels[] = {0, 3, 5};
   groups.Add(grp1Channels, size(grp1Channels));
   
   signed long grp2Channels[] = {6, 8, 10};
   groups.Add(grp2Channels, size(grp2Channels));
   
   cout<< groups.Size() << endl;
   cout<< groups[0].channelCount() << endl;
   
   for (signed long channel : groups[0].Channels()) 
   {
     cout << channel << endl;
   }
   
   cout<< groups[0].getId() << endl;
   
   return 0;
}


На выходе получается следующее:

>>> $main  <<<<
2
3
0
3
5
1


Я беспокоился о getItem, чтобы увидеть, хорошо ли возвращать указатель группы, а не Группы. В настоящее время это находится в режиме только для чтения, но что произойдет, если я захочу изменить значение канала, например? Как вы думаете, что было бы лучшим гибким способом ?

Заранее Вам большое спасибо.
С уважением.
Мики

Что я уже пробовал:

Я создал код выше.

Gerry Schmitz

Жизнь слишком коротка, чтобы возиться с вещами, которые "работают" только для того, чтобы увидеть, может ли она быть "лучше" (в данном случае).

https://martinfowler.com/bliki/Yagni.html

2 Ответов

Рейтинг:
1

Stefan_Lang

Технически, у вас есть немного больше, чем std::vector<std::vector<long>> Вы можете переписать несколько дополнительных функций-членов, которые вы написали как глобальные функции. Тогда вы могли бы воспользоваться полной функциональностью std::vector, а не просто очень ограниченным выбором, который вы случайно предоставили в своей реализации Group of Groups. И вы можете в полной мере воспользоваться высокой эффективностью этой реализации, а не задаваться вопросом, является ли ваша собственная реализация неоптимальной.

Но чтобы ответить на ваш вопрос, я заметил следующее, что вы должны исправить, просто потому, что это общее соглашение о кодировании. Если вы переопределяете оператор индекса для своего класса, то вы должны предоставить два варианта: один должен быть объявлен const и возвращать копию требуемого элемента, другой не должен быть const и возвращать ссылку на этот элемент, чтобы его можно было изменить извне:

const Group operator[](int) const; // return a copy
Group& operator[](int); // return a reference

Если вы сделаете это, он косвенно ответит на ваш вопрос: в вашей текущей реализации getIten() имеет точно такую же функциональность, как оператор индекса (за исключением проверки диапазона). Поэтому вы должны предоставить две версии: одну-функцию const, возвращающую копию, а другую-неконстантную, возвращающую ссылку.

Обратите внимание, что этот ответ основан на общих соглашениях о кодировании и имеет мало общего с производительностью. Хотя, конечно, следование общепринятым соглашениям о кодировании обычно также помогает получить хорошую производительность. ;-)


SuperMiQi

Это действительно то, что я искал.
Большое вам спасибо за ваш ответ.

Рейтинг:
0

KarstenK

Просто напишите функцию SetChannel с помощью [] оператор по своим каналам. Это нормальный шаблон кодирования.

void SetChannel(Group &group, int index)
{
  _channels[index] = group;
}
Проверьте, что индекс является допустимым и для оптимальной работы кода со ссылками или указателями, чтобы избежать временных объектов. (иногда это может быть большой проблемой)


SuperMiQi

Большое вам спасибо за ваш ответ.