1

Чтобы метод InventorySorter работал пришлось каждому элементу массива присвоить string.Empty

Как можно обойтись без этого?

public class Inventory
{
    public string[] inventorySlots = new string[9] {string.Empty, string.Empty, string.Empty, string.Empty, string.Empty, string.Empty, string.Empty, string.Empty, string.Empty };

    public void InventorySorter(string name)
    {
        for (int i = 0; i < inventorySlots.Length; i++)
        {
            if (inventorySlots[i] == string.Empty)
            {
                inventorySlots[i] = name;
                break;
            }
        }
    }
}
4
  • 2
    Если убрать ваши string.Empty, то в массиве будут значения по умолчанию: null. 31 авг 2020 в 21:36
  • Судя по сути кода, вам следует использовать список вместо массива: List<string>. 31 авг 2020 в 21:38
  • 2
    Название метода странное: Sorter. Вроде он ничего не сортирует, а добавляет. 31 авг 2020 в 21:41
  • А что должен делать этот метод?
    – aepot
    1 сен 2020 в 6:08

3 ответа 3

2

Можно доработать условие добавления элемента, например если строка не инициализирована null или пуста, то ячейку считать пустой. Здесь неоднозначность какую ячейку инвентрая считать пустой: null или ""? Обе?

Тогда условие проверки должно всегда проверять оба эти условия. Например условие для пустой ячейки можно реализовать так: if (string.IsNullOrEmpty(inventorySlots[i])) {...}, а для не пустой вот так if (inventorySlots[i]?.Length > 0) {...}.

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

public string[] inventorySlots = Enumerable.Repeat(string.Empty, 9).ToArray();

Посмотрите другие статические методы класса Enumerable, там много всего полезного.

Затем можно оптимизировать сравнение if (inventorySlots[i].Length == 0).

Но я бы на вашем месте в инвентаре хранил бы не строки, а числа int[], и отдельно словарь с элементами инвентаря Dictionary<int, string>, где по айдишнику элемента можно получить название элемента и возможно другие его характеристики. Так же, если вы захотите переименовать элемент, или использовать локализации, например интерфейс игры на русском и английском, тогда это не затронет сам инвентарь.

В случае с хранением айдишников int - все будет вполне однозначно, 0 - пусто, или -1 - пусто, просто выберите, что вам больше по душе. К тому же в C# числовые операции сравнения работают значительно быстрее, чем строковые. А еще строка занимает больше памяти. Представьте 1000 инвентарей, хранящих строки, содержащие одни и те же элементы, зачем столько 9000 копий повторяющихся строк хранить в памяти, если можно хранить только по одному экземпляру каждой строки, и добывать ее с помощью числового индекса, хранящегося в инвентаре.

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

6
  • Если что в C# есть интернизация строк, так что проблемы "9000 копий повторяющихся строк хранить в памяти" при желании можно избежать )
    – CrazyElf
    1 сен 2020 в 8:55
  • @CrazyElf никто не спорит, можно и кеш реализовать, но словарь это, кмк, более простое и интуитивно понятное решение. Его и в БД при желании достаточно легко положить, он расширяем до Int32.MaxValue значений. К тому же, это не обязательно должен быть словарь строк, он может быть словарь экземпляров класса, представляющего игровой айтем со всеми своими прибамбасами.
    – aepot
    1 сен 2020 в 9:25
  • 1
    Да я по остальным вопросам с вами и не спорил, просто небольшое замечание внёс )
    – CrazyElf
    1 сен 2020 в 9:45
  • По поводу хранения айдишников одно маленькое замечание: вместо 0 или -1 как пустой ячейки я бы сделал null, а массив бы стал типа int?[]. Как лично мне кажется, с точки зрения работы с инвентарем в коде такой подход более понятен будет 1 сен 2020 в 10:03
  • Либо и вовсе на бизнес-слое делать инвентарь как массив классов ячеек инвенторя, где каждая ячейка имеет булево состояние inventorySlot.HasItem. Но это уже немного другая история 1 сен 2020 в 10:05
0

Возможно Вам больше такой код понравится:

public class Inventory
{
    public const int inventorySlotsLimit = 9;

    public List<string> inventorySlots = new List<string>();

    public void AddItem(string name)
    {
        if(inventorySlots.Count + 1 > inventorySlotsLimit)
        {
            throw new Exception("Inventory overflow");
        }
        inventorySlots.Add(name);
    }
}
0
-1

Код не проверял, но как-то так

public class Inventory
{
    public string[] inventorySlots = new string[9];

    public void InventorySorter(string name) =>
        inventorySlots.Where(x => x == null).ForEach(x => x = name);
}

Не очень оптимально по работе, зато красиво :)

Оптимизировать можно в том случае если понадобится, что наврядли в даном случае.

4
  • наврядли в даном случае Судя по названию класса, производительность важна, так как это скорее всего игра.
    – aepot
    31 авг 2020 в 22:16
  • только судя по уровню спрашивающего, это вероятнее всего проэкт-тренировка и в релиз никогда не выйдет. А других значительно более медленных мест там будет просто завались и это решение просто особо ни на что не повлияет) Оптимизировать ВСЕ И ВСЮДУ как оптимизация ради оптимизации - это вообще довольно глупая затея. Оптимизировать нужно только по мере надобности. 1 сен 2020 в 6:16
  • 1
    Ну тогда хотя-бы тогда .Where вместо тернаршины. Только если строка пустая, условие не сработает, надо исправить.
    – aepot
    1 сен 2020 в 6:34
  • 1
    А вот здесь уже согласен, спасибо) 1 сен 2020 в 9:31

Ваш ответ

By clicking “Отправить ответ”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Всё ещё ищете ответ? Посмотрите другие вопросы с метками или задайте свой вопрос.