Что не так с моими уроками?

Question

Представлены эти два класса (вместе с бизнес-логикой и логикой доступа к данным) для проверки кода.Рецензент засмеялся, глядя на код, и посоветовал мне заново сделать уроки.Он сказал мне, чтобы я провел какое-то исследование и не дал мне никаких подсказок.Я не мог найти ничего плохого.Немного неловко выглядит, что «подменю» - это базовый класс, а не меню.Но мое требование таково.,,меню состоит только из двух уровней (под-подменю никогда не будет).Что в них смешного?Был бы признателен, если бы кто-нибудь показал мне путь для улучшения этих классов.

[Serializable]
public class Menu: SubMenu
{
    private List<SubMenu> _subMenu;
    public List<SubMenu> SubMenu
    {
        get
        {
            if (_subMenu == null)
                _subMenu = new List<SubMenu>();
            return _subMenu;
        }
        set
        {
            _subMenu = value;
        }
    }
}

[Serializable]
public class SubMenu
{
    public int ID { get; set; }
    public string MenuText { get; set; }
    public string MenuURL { get; set; }
    public string ImageURL { get; set; }
}

Answer 1

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

Тем не менее, я думаю, что они увидели в вашем дизайне тот факт, что вы вообще используете наследование.Вам нужно наследство здесь?Кажется, вам лучше обслужить класс Menu, который содержит список SubMenus без родительско-дочерних отношений или, возможно, только один объект MenuItem, который может содержать список самого себя.

Я бы предпочел второй подход на самом деле, что-то вроде этого может быть:

[Serializable] 
public class MenuItem
{ 
    public MenuItem()
    {
        MenuItems = new List<MenuItem>();
    }

    public int ID { get; set; } 
    public string MenuText { get; set; } 
    public string MenuURL { get; set; } 
    public string ImageURL { get; set; } 

    // I'm not bothering with the code to protect access to the list
    // in this example but you might want that too...
    public List<MenuItem> MenuItems { get; set; }
} 

Answer 2

Общая конструкция ваших объектов хороша, например, ленивый экземпляр, хотя, как показывают другие ответы, я думаю, что ваше наследование немного ошибочно;но это не проблема!

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

Я создал свою концепцию элемента меню, которая является лишь предложением, но, надеюсь, это поможет.Вот некоторые вещи, на которые стоит обратить внимание.

1. Я удалил базовый объект
2. Теперь пункт меню больше использует ленивый экземпляр, но при создании создает список
3. Появились новые конструкторы
4. Пункты меню могут иметь бесконечные уровни глубины.

и вот как это выглядит:

[Serializable]
public class MenuItem
{
    public int Id { get; set; }
    public string MenuText { get; set; }
    public string MenuUrl { get; set; }
    public string ImageUrl { get; set; }

    public List<MenuItem> Children { get; set; }

    public MenuItem(int id, string text, string url)
        : this(id, text, url, String.Empty)
    {
    }

    public MenuItem(int id, string text, string url, string imageUrl)
    {
        this.Id = id;
        this.MenuText = text;
        this.MenuUrl = url;
        this.ImageUrl = imageUrl;

        this.Children = new List<MenuItem>();
    }
}

Answer 3

Я согласен с вышеуказанными комментаторами.чтобы ваш рецензент просто смеялся и не давал конструктивных советов, это грубо и контрпродуктивно.мой ответ будет: «, вернитесь и получите профессиональную конструктивную консультацию » сначала от рецензента.Если этого не произойдет, то, боюсь, похоже, что рецензент руководствуется эго, а не управлением и принципами руководства и образования.

Answer 4

Рецензент не полезен или профессионал. Будучи вежливым, вы должны понимать, что такой обзор не имеет смысла. Те, у кого есть знания / опыт, обязаны делиться, когда их об этом спрашивают.

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

Также путаница между наследованием от SubMenu (вводит в заблуждение, поскольку вы обычно наследуете от базового класса или суперкласса в зависимости от предпочитаемой вами терминологии и того, что наследуете, является подклассом) и имеет свойство, которое также называется SubMenu и Свойство представляет собой набор, а не один экземпляр.

Посмотрите на реализацию меню, например, в winforms - за ними проще следовать

Answer 5

Представлены эти два класса (вместе с бизнес-логикой и логикой доступа к данным) для проверки кода.Рецензент засмеялся, глядя на код, и посоветовал мне заново сделать уроки.Он сказал мне, чтобы я провел какое-то исследование и не дал мне никаких подсказок.

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

Я не смог найти ничего плохого.Немного неловко выглядит, что «подменю» - это базовый класс, а не меню.Но мое требование таково ... меню состоит только из двух уровней (под-подменю никогда не будет).

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

Если вы требуете, чтобы в меню было только дваУровни, не создавайте код, расширяющий это требование.Однако, если у вас есть возможность очень легко открыть свой код для будущего расширения, то почему бы не сделать это?Там нет затрат, просто выгода.И в вашем случае, я думаю, что «открытие для будущего расширения» действительно довольно просто.

Что в них смешного?Был бы признателен, если бы кто-то показал мне путь для улучшения этих классов.

Это не обязательно смешно или неправильно.Я бы сделал это иначе: если меню может содержать меню, не будет ли это композицией?И если да, в чем разница между меню и подменю?Подменю - это меню, в котором есть родитель, но должно ли оно заботиться?Должен ли он знать?На самом ли деле оно отличается от самого меню?

class Menu
  +addSubmenu( Menu menu ) : Menu
  +addItem( Item item ) : Menu

Так будет выглядеть моя попытка, вероятно.

Answer 6

Учитывая ваши требования, единственное, что я считаю неправильным, это проблема с именами:

public List<SubMenu> SubMenu

Это коллекция, она должна быть во множественном числе. А если серьезно, то тут не над чем смеяться ..

Теперь, если вы вернетесь к своему ****** рецензенту, подготовьте свои аргументы:

• Наследование PLUS Коллекция необходима, потому что Меню само является Подменю, так как Меню имеет Текст / Url / Изображение / ID, и у него есть коллекция подменю, которые находятся ниже его.

• Объясните требование, где SubMenu является базовым классом для Menu (который на самом деле не так прост).

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

Answer 7

Похоже, у вас есть меню, которое может содержать два типа элементов

1) «Простой элемент меню» (Id, Text, url, image)2) Другое меню (подменю)

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

Кажется, что шаблон составного дизайна (http://en.wikipedia.org/wiki/Composite_pattern)быть в хорошей форме.

hth,Алан.

Answer 8

Я не уверен, что я прав с помощью следующего утверждения (и, пожалуйста, исправьте меня, если я ошибаюсь), но если класс наследует от базового класса, должен ли производный класс содержать список элементов своего базового класса ? И в каком случае это может быть полезно (я не могу думать ни о чем прямо сейчас). Если я думаю, например, о «Лошади» и «Животном»: должен ли класс «Лошадь» содержать список животных? Звучит немного странно ..