Версия для печати темы
Нажмите сюда для просмотра этой темы в оригинальном формате
Форум программистов > .NET для новичков > Правильность кода и наследование от класса carrier


Автор: mastermedia 17.1.2012, 21:53
Я реализовал класс транспортное средство и два класса автомобиль и поезд, которые наследуются от класса транспортное средство. Я использовал в программе абстрактные классы, методы и виртуальные методы. Проверьте, пожалуйста, насколько корректно я реализовал данную программу.

Класс Carrier
Код

  abstract class Carrier
    {
        protected string Model { get; set; }
        protected int Speed;
        abstract public int AverageSpeed { get; set; }
        protected bool checkSpeed;
        
        public Carrier(string model)
        {
            Model = model;
        }

        protected bool isSpeedCorrect(int speed)
        {
            if (speed <= 0)
                return false;

            else
                return true;
        }

        public void validateSpeed()
        {
            Console.WriteLine("Введите скорость.");
            while (!checkSpeed)
            {
                AverageSpeed = Convert.ToInt32(Console.ReadLine());
            }
        }
        
        abstract public void DescribeCarrier();
        abstract public void ShowSpeed();
    }

Класс Car
Код
class Car : Carrier 
    {
        private string TypeCar { get; set; }
        
        public Car(string typeCar, string model)
            : base(model)
        {
            TypeCar = typeCar;
        }

        public override int AverageSpeed
        {
            get
            {
                return Speed;
            }
            
            set
            {
                if (isSpeedCorrect(value))
                {
                    Speed = value;
                    checkSpeed = true;
                }
                else
                {
                    Console.WriteLine("Скорость отрицательная или правильную скорость. Введите правильную скорость.");
                    checkSpeed = false;
                }
            }
        }

        public override void DescribeCarrier()
        {
            Console.WriteLine("Это " + TypeCar + " автомобиль модели " + Model + ".");
        }
        
        public override void ShowSpeed()
        {
            Console.WriteLine("Средняя скорость: "+ AverageSpeed + " км/час.");
        }
    }

Класс Train
Код
class Train : Carrier 
    {
        private string TrainType { get; set; }
        
        public Train(string model, string trainType)
            : base(model)
        {
            TrainType = trainType;
        }

        public override int AverageSpeed
        {
            get
            {
                return Speed;
            }
            set
            {
                if (isSpeedCorrect(value))
                {
                    checkSpeed = true;
                    Speed = value;
                }

                else
                {
                    checkSpeed = false;
                    Console.WriteLine("Скорость отрицательна или равна 0. Введите правильную скорость.");
                }
            }
        }
        public override void DescribeCarrier()
        {
            Console.WriteLine("Это " + TrainType + " поезд.");
        }

        public override void ShowSpeed()
        {
            Console.WriteLine("Средняя скорость: " + AverageSpeed + " км/час.");
        }
    }

Класс Program
Код
class Program
    {
        static void Main(string[] args)
        {
            Car c = new Car("легковой", "BMW");
            Train t = new Train("ЧМЭ3", "дизельный");
            c.validateSpeed();
            c.DescribeCarrier();
            c.ShowSpeed();
            t.validateSpeed();
            t.DescribeCarrier();
            t.ShowSpeed();
        }
    }

Автор: Cheloveck 17.1.2012, 23:17
Цитата(mastermedia @  17.1.2012,  22:53 Найти цитируемый пост)
        protected string Model { get; set; }
        protected int Speed;

Почему первое -- свойство, а второе -- переменная.
Защищённые переменные в базовом классе -- плохая практика. Используй свойства и методы.
Цитата(mastermedia @  17.1.2012,  22:53 Найти цитируемый пост)
protected bool checkSpeed;

Туда же. плюс начинает меняться нотация имён -- очень не профессионально.
Цитата(mastermedia @  17.1.2012,  22:53 Найти цитируемый пост)
            if (speed <= 0)
                return false;
            else
                return true;

Код

return speed > 0; // чуть короче...


Цитата(mastermedia @  17.1.2012,  22:53 Найти цитируемый пост)
        public void validateSpeed()
        {
            Console.WriteLine("Введите скорость.");
            while (!checkSpeed)
            {
                AverageSpeed = Convert.ToInt32(Console.ReadLine());
            }
        }

1. Никаких вопросов этот класс, тем более этот метод задавать не должен. Это противоречит логике.
2. Имя метода говорит: "проверить скорость". На практике мы видим бесконечный цикл. Если метод называется ValidateXXX, то ему должно передаться значение, которое он должен валидировать, но никак уж не запрашивать какие-то данные у пользователя.

Цитата(mastermedia @  17.1.2012,  22:53 Найти цитируемый пост)
private string TypeCar { get; set; }

Тут вообще свойство на фиг не нужно. Должна быть просто переменная.

Цитата(mastermedia @  17.1.2012,  22:53 Найти цитируемый пост)
            set
            {
                if (isSpeedCorrect(value))
                {
                    Speed = value;
                    checkSpeed = true;
                }
                else
                {
                    Console.WriteLine("Скорость отрицательная или правильную скорость. Введите правильную скорость.");
                    checkSpeed = false;
                }
            }

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

Цитата(mastermedia @  17.1.2012,  22:53 Найти цитируемый пост)
Console.WriteLine("Скорость отрицательная или правильную скорость. Введите правильную скорость.");

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



Цитата(mastermedia @  17.1.2012,  22:53 Найти цитируемый пост)
        public override void DescribeCarrier()
        {
            Console.WriteLine("Это " + TypeCar + " автомобиль модели " + Model + ".");
        }
        
        public override void ShowSpeed()
        {
            Console.WriteLine("Средняя скорость: "+ AverageSpeed + " км/час.");
        }

Всё те же проблемы.

Цитата(mastermedia @  17.1.2012,  22:53 Найти цитируемый пост)
private string TrainType { get; set; }

Опять приватное свойство вместо переменной.

Цитата(mastermedia @  17.1.2012,  22:53 Найти цитируемый пост)
        public override int AverageSpeed
        {
            get
            {
                return Speed;
            }
            set
            {
                if (isSpeedCorrect(value))
                {
                    checkSpeed = true;
                    Speed = value;
                }
                else
                {
                    checkSpeed = false;
                    Console.WriteLine("Скорость отрицательна или равна 0. Введите правильную скорость.");
                }
            }
        }

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

Цитата(mastermedia @  17.1.2012,  22:53 Найти цитируемый пост)
        public override void ShowSpeed()
        {
            Console.WriteLine("Средняя скорость: " + AverageSpeed + " км/час.");
        }

Опять.

Я бы поставил 1 по 5-ти бальной шкале. Удачи!

Автор: mastermedia 17.1.2012, 23:41
Cheloveck, это не для преподавателя, а для себя делаю.

Цитата(Cheloveck @  17.1.2012,  23:17 Найти цитируемый пост)
Ах вот как из бесконечного цикла выходим. Оказывается, базовый класс знает об устройстве своих наследников.

Поясни этот момент, а то я не сильно понял почему это плохо?

Цитата(Cheloveck @  17.1.2012,  23:17 Найти цитируемый пост)
 То есть, при присваивании значения свойству, внезапно, появляется запрос пользователю. 

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

Автор: Cheloveck 17.1.2012, 23:48
Цитата(mastermedia @  18.1.2012,  00:41 Найти цитируемый пост)
Поясни этот момент

Когда ты пишешь класс, ты не должен полагаться на логику его наследников. Предполагается, что твой класс является самостоятельной сущностью полностью обслуживающий себя сам. Абстрактные методы нужны, как правило, для двух целей:
1. Получить данные, которыми базовый класс не может владеть.
2. Обработать данные, которые базовый класс не знает, как обработать. При этом результат обработки должен приходить из вызова этого метода (возвращаемое значение или out параметр), но никак не через изменение состояния объекта.
Состояние базового класса можно менять из дочерних только в том случае, если базовый класс этого просит явно.
Защищённые переменные способствуют излишней осведомлённости потомков, потом появляются большие проблемы.

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

Автор: mastermedia 17.1.2012, 23:50
Цитата(Cheloveck @  17.1.2012,  23:17 Найти цитируемый пост)
        public override void DescribeCarrier()
        {
            Console.WriteLine("Это " + TypeCar + " автомобиль модели " + Model + ".");
        }
        
        public override void ShowSpeed()
        {
            Console.WriteLine("Средняя скорость: "+ AverageSpeed + " км/час.");
        }

Всё те же проблемы.


Тут имеется виду вместо свойств TypeCar, Model написал обычные переменные?
Цитата(Cheloveck @  17.1.2012,  23:17 Найти цитируемый пост)
Почему первое -- свойство, а второе -- переменная.
Защищённые переменные в базовом классе -- плохая практика. Используй свойства и методы.

Шилдт у себя в примерах использовал.

Автор: Cheloveck 17.1.2012, 23:51
Цитата(mastermedia @  18.1.2012,  00:41 Найти цитируемый пост)
Это я делаю на случай, если пользователь не правильно введет данные.

Все данные вводятся в контроллер. Классы, содержащие бизнес-логику не должны общаться с пользователем. Контроллер в примитивном случае -- метод Main.
Если значение не верное -- бросай исключение, контроллер (или кто-то ещё) поймают его и всё спросят за тебя. Это проблема не логики, а уровня взаимодействия с пользователем.

Добавлено через 2 минуты и 13 секунд
Цитата(mastermedia @  18.1.2012,  00:50 Найти цитируемый пост)

Тут имеется виду вместо свойств TypeCar, Model написал обычные переменные?

Тут имеется введу чрезвычайная разговорчивость класса
Цитата(mastermedia @  18.1.2012,  00:50 Найти цитируемый пост)
Шилдт у себя в примерах использовал. 

Это не повод делать то же самое ;-)

Автор: mastermedia 18.1.2012, 00:02
В примитивном случае, в моем, я должен требовать от пользователя данные вводить данные до тех пор пока он не ведет правильные в методе Main, а из класса убрать сообщения?

Цитата(Cheloveck @  17.1.2012,  23:48 Найти цитируемый пост)
Когда ты пишешь класс, ты не должен полагаться на логику его наследников. Предполагается, что твой класс является самостоятельной сущностью полностью обслуживающий себя сам. Абстрактные методы нужны, как правило, для двух целей:
1. Получить данные, которыми базовый класс не может владеть.
2. Обработать данные, которые базовый класс не знает, как обработать. При этом результат обработки должен приходить из вызова этого метода (возвращаемое значение или out параметр), но никак не через изменение состояния объекта.
Состояние базового класса можно менять из дочерних только в том случае, если базовый класс этого просит явно.

 Вместо свойств мне использовать для изменения скорости метод?

Автор: Cheloveck 18.1.2012, 00:50
Цитата(mastermedia @  18.1.2012,  01:02 Найти цитируемый пост)
 вводить данные до тех пор пока он не ведет правильные в методе Main, а из класса убрать сообщения?

Да.

Цитата(mastermedia @  18.1.2012,  01:02 Найти цитируемый пост)
Вместо свойств мне использовать для изменения скорости метод? 

Да. Но ничего не менять там, где это не очевидно, например при валидации.

Автор: Cheloveck 18.1.2012, 02:13
Вообще, твой пример не очень хорошо демонстрирует наследование. Вот код, притянутый за уши, чтобы хоть как-то соответствовать топику, который решает твою задачу. Немного расширен, дабы не было дублирования кода. Плюс все данные вводит пользователь.
Код

using System;

namespace Demo {
    public abstract class Carrier {
        public Carrier(uint speed) {
            if(speed == 0) {
                throw new Exception("A speed must not be equals zero");
            }
            Speed = speed;
        }

        public uint Speed { get; set; }
        public abstract string Description { get; }
    }

    public class CarInfo {
        public enum CarType {
            Sedan,
            Jeep,
            Hatchback,
            Universal
        }
        public CarType Type { get; set; }
        public string Model { get; set; }
    }

    public class Car : Carrier {
        CarInfo info;

        public Car(uint speed, CarInfo info) 
            : base(speed) {
                this.info = info;
        }

        public override string  Description{
            get { 
                return String.Format(
                    "The car.{0}Model: {1}.{0}Type: {2}.{0}Average speed: {3}", 
                    Environment.NewLine, info.Model, info.Type, Speed);
            }
        }
    }

    public class TrainInfo {
        public enum TrainType {
            Cargo,
            Passenger
        }
        public TrainType Type { get; set; }
        public string Model { get; set; }
        public uint CoachCount { get; set; }
    }

    public class Train : Carrier {
        TrainInfo info;

        public Train(uint speed, TrainInfo info) 
            : base(speed) {
                this.info = info;
        }

        public override string  Description{
            get { 
                return String.Format(
                    "The train.{0}Model: {1}.{0}Type: {2}.{0}Coach count: {3}.{0}Average speed: {4}", 
                    Environment.NewLine, info.Model, info.Type, info.CoachCount, Speed);
            }
        }
    }

    class Program {
        static void Main(string[] args) {
            Car car = EnsureCreate<Car>(() => CreateCar());
            Train train = EnsureCreate<Train>(() => CreateTrain());
            Console.WriteLine("----------------");
            Console.WriteLine("The Car description:");
            Console.WriteLine(car.Description);
            Console.WriteLine("----------------");
            Console.WriteLine("The train description:");
            Console.WriteLine(train.Description);
            Console.WriteLine();
            Console.Write("Press any key to exit");
            Console.ReadKey();
        }

        delegate T Creator<T>();
        static T EnsureCreate<T>(Creator<T> creator) {
            while(true) {
                try {
                    return creator();
                } catch(Exception err) {
                    Console.WriteLine("Error occurs during creating an object.");
                    Console.WriteLine("Read follow information about error:");
                    Console.WriteLine(err.Message);
                    Console.WriteLine("Try again");
                    Console.WriteLine();
                }
            }
        }

        static Car CreateCar() {
            CarInfo info = new CarInfo();
            info.Type = AskEnumType<CarInfo.CarType>("car type");
            uint speed = AskSpeed();
            Console.Write("Type the car model and press <Enter>: ");
            info.Model = Console.ReadLine();
            return new Car(speed, info);
        }

        static Train CreateTrain() {
            TrainInfo info = new TrainInfo();
            info.Type = AskEnumType<TrainInfo.TrainType>("train type");
            info.CoachCount = AskUint("coach count");
            uint speed = AskSpeed();
            Console.Write("Type the train model and press <Enter>: ");
            info.Model = Console.ReadLine();
            return new Train(speed, info);
        }

        static T AskEnumType<T>(string name) {
            Array values = Enum.GetValues(typeof(T));
            while(true) {
                Console.WriteLine("Choose the {0}:", name);
                int i = 0;
                foreach(var val in values) {
                    Console.WriteLine("{0} - {1}", i++, val);
                }
                Console.Write(": ");
                string userValue = Console.ReadLine();
                uint index;
                if(UInt32.TryParse(userValue, out index) && index < values.Length)
                    return (T)values.GetValue(index);
                Console.WriteLine("Value is incorrect. Try again.");
            }
        }

        static uint AskSpeed() {
            return AskUint("speed");
        }

        static uint AskUint(string name) {
            while(true) {
                Console.Write("Type the {0} value and press <Enter>: ", name);
                string value = Console.ReadLine();
                uint speed;
                if(UInt32.TryParse(value, out speed))
                    return speed;
                Console.WriteLine("Value is incorrect. Try again.");
            }
        }
    }
}

Автор: mastermedia 18.1.2012, 20:17
Cheloveck,  я не пойму почему в некоторых случаях не нужно использовать свойство, а просто переменную? Например, в случае переменной Model. Ведь свойства нужны для того чтобы улучшить доступ к значению и как написанно в Шилдте это удобнее, чем использовать методы get и set.  Тем более, если при организации метода вывода вместе использовать в выводе свойства и переменные, то это как я понимаю, усложнит отладку кода.

Автор: Cheloveck 18.1.2012, 21:16
Свойства -- это ни что иное как методы доступа. Их назначение -- дать возможность контроля данных или ограничить чтение/запись. Вызовы методов всегда связаны с накладными расходами (пусть и не большими).
Также свойства предоставляют возможность сделать их виртуаьными, что зачастую упрощает синтаксис использования свойств в отличаи от классических методов.
Назначение переменных -- хранить состояние объекта. При использовании auto-implemented свойств автоматически генерируется переменная, хранящая значение свойства, хотя создаётся ложное впечатление того, что свойство само хранит значение.

Powered by Invision Power Board (http://www.invisionboard.com)
© Invision Power Services (http://www.invisionpower.com)