Tell me what's wrong with my class architecture; <code> enter image description here </ code> `

using System; using System.Collections.Generic; using System.Threading; namespace ConsoleApplication97 { abstract class Securities { string fullName; string shortName; string issuerName; DateTime dateOfEmission; double volume; public Securities() { } public Securities(string f, string sh, string iss, DateTime de, double v) { FullName = f; ShortName = sh; IssuerName = iss; DateOfEmission = de; Volume = v; } protected string FullName { get { return fullName; } set { fullName = value; } } protected string ShortName { get { return shortName; } set { shortName = value; } } protected string IssuerName { get { return issuerName; } set { issuerName = value; } } protected DateTime DateOfEmission { get { return dateOfEmission; } set { dateOfEmission = value; } } protected double Volume { get { return volume; } set { volume = value; } } public abstract void ChangeCurrentPrice(double cp); public abstract string ToStringFields(); } class Share : Securities { string typeShare; double currentPrice; double balanceOfCost; double lotSize; public Share(string f, string sh, string iss, DateTime de, double v, string tysh, double cp, double bc, double ls) : base(f, sh, iss, de, v) { TypeShare = tysh; CurrentPrice = cp; BalanceOfCost = bc; LotSize = ls; } public Share() { } public string TypeShare { get { return typeShare; } set { typeShare = value; } } public double CurrentPrice { get { return currentPrice; } set { currentPrice = value; } } public double BalanceOfCost { get { return balanceOfCost; } set { balanceOfCost = value; } } public double LotSize { get { return lotSize; } set { lotSize = value; } } public override void ChangeCurrentPrice(double cp) { currentPrice = cp; } public override string ToStringFields() { return FullName + "\n" + ShortName + "\n" + IssuerName + "\n" + DateOfEmission + "\n" + Volume + "\n" + TypeShare + "\n" + CurrentPrice + "\n" + BalanceOfCost + "\n" + LotSize + "\n-----------------------------"; } public double CalcAmountOfDevidends(bool t) { if (typeShare == "Обычная") { return balanceOfCost * 0.05; } else { if (t) { return balanceOfCost * 0.05; } else { return 0; } } } } class Bond : Securities { string typeBond; double ratedCost; double currentPrice; DateTime datePerformance; double percentCoupon; public Bond(string f, string sh, string iss, DateTime de, double v, string tyB, double rC, double cp, DateTime dP, double pc) : base(f, sh, iss, de, v) { TypeBond = tyB; RatedCost = rC; CurrentPrice = cp; DatePerformance = dP; PercentCoupon = pc; } public Bond() { } public string TypeBond { get { return typeBond; } set { typeBond = value; } } public double RatedCost { get { return ratedCost; } set { ratedCost = value; } } public double CurrentPrice { get { return currentPrice; } set { currentPrice = value; } } public DateTime DatePerformance { get { return datePerformance; } set { datePerformance = value; } } public double PercentCoupon { get { return percentCoupon; } set { percentCoupon = value; } } public override void ChangeCurrentPrice(double cp) { CurrentPrice = cp; } public override string ToStringFields() { return FullName + "\n" + ShortName + "\n" + IssuerName + "\n" + DatePerformance + "\n" + Volume + "\n" + TypeBond + "\n" + RatedCost + "\n" + CurrentPrice + "\n" + DatePerformance + "\n" + PercentCoupon + "\n-----------------------------"; } public double CalcSumCoupon() { if (TypeBond == "Купон") { 05.06.16 return PercentCoupon * RatedCost; } else { return 0; } } } class Program { static void Main(string[] args) { List<Securities> list = new List<Securities>(); list.Add(new Bond("ПУБЛИЧНОЕ АКЦИОНЕРНОЕ ОБЩЕСТВО «ГОРНО - МЕТАЛЛУРГИЧЕСКАЯ КОМПАНИЯ» «НОРИЛЬСКИЙ НИКЕЛЬ»", "ПАО ГОРНО - МЕТАЛЛУРГИЧЕСКАЯ КОМПАНИЯ НОРИЛЬСКИЙ НИКЕЛЬ", "ОАО «Российское акционерное общество «Норильский никель», ОАО «РАО «Норильский никель»", new DateTime(2016, 10, 1), 3500, "Дисконтная", 150, 45, new DateTime(2016, 11, 1), 0.15)); list.Add(new Share("Ростелеком(ПАО) АО", "Ростел-AO", "RTKM", new DateTime(2013, 5, 6), 6000, "Обычная", 47, 45, 48)); list.Add(new Share("акц.пр. ОАО АК Транснефть", "Транснф АП", "TRNFP", new DateTime(2013, 5, 6), 6500, "Привилегированная", 45, 36, 46)); list.Add(new Bond("АКЦИОНЕРНОЕ ОБЩЕСТВО «АЛЬФА-БАНК»", "АО «АЛЬФА-БАНК»", "Альфа-Банк", new DateTime(2015, 1, 1), 5000, "Купонная", 200, 15, new DateTime(2015, 2, 1), 0.11)); list.Insert(0, new Bond("Публичное акционерное общество «Сбербанк России»", "АО «АЛЬФА-БАНК»", "ПАО Сбербанк", new DateTime(2014, 1, 1), 4000, "Купонная", 400, 25, new DateTime(2014, 3, 1), 0.08)); //доб.в начало списка list.RemoveAt(1); //удаление из списка 2 элемента while (true) { for (int i = 0; i < list.Count; i++) { if (new Random().Next() % 2 == 1) { if (typeof(Bond) == list[i].GetType()) { list[i].ChangeCurrentPrice(((Bond)(list[i])).CurrentPrice + new Random().NextDouble()); } else { list[i].ChangeCurrentPrice(((Share)(list[i])).CurrentPrice + new Random().NextDouble()); } } } Thread.Sleep(new Random().Next(1000, 10000)); foreach (var sec in list) { Console.WriteLine(sec.ToStringFields()); } } Console.ReadKey(); } } } 

Closed due to the fact that it is necessary to reformulate the question so that it was possible to give an objectively correct answer by the participants of D-side , VenZell , aleksandr barakin , user194374, Kromster 9 Jun '16 at 5:30 .

The question gives rise to endless debates and discussions based not on knowledge, but on opinions. To get an answer, rephrase your question so that it can be given an unambiguously correct answer, or delete the question altogether. If the question can be reformulated according to the rules set out in the certificate , edit it .

  • 3
    What exactly does not suit the teacher, to clarify all the same better with him. In classes like this, nothing criminal is immediately visible. - Monk
  • one
    I do not see anything criminal in architecture, only a couple of shortcomings in the types and use of the capabilities of the language. What does not suit the teacher, it is necessary to clarify with him. - Alex Krass
  • one
    There are no explicit access modifiers, errors in the code, non-uniform indents, indistinct namespace, and no use of the language features. You can get to anything. Without a comment, the teacher will not help you here. - Alexis

2 answers 2

You have everything protected in the base class, it will not allow you to contact them outside. Either it is necessary in each class to write getters / setors for each property (which is not quite logical), or to rewrite it as something like this:

 public string FullName { get { return fullName; } protected set{ fullName = value; } //так сетер будет протектнутым } 

In the reference information for your control, it most likely hints at this.

    The problem is not so much in the class hierarchy, everything is just fine in it, but in the architecture, read the code, of the classes themselves.

    1. As far as I understand, securities cannot change their type, name, issuer, and issue date, therefore these parameters should be set in the constructor and have read-only properties, without set or with private set . The constructor without parameters in your case is not applicable from the point of view of logic, since A security in principle cannot be without the above listed set of immutable properties. Accordingly, it is necessary to define constructors with the appropriate set of parameters.

    2. To output as a string, you do not need to create a separate method; it is enough to override the standard ToString method inherited from Object , since He doesn’t do anything useful in the basic version anyway. Although this is already an amateur and according to the conditions of a specific task, this option is more than suitable for a learning task.

    3. All securities from your task have a current price, so this field and method for it can be taken out to the base class, despite the fact that this is not clearly written. For this, it is also basic in order to contain common elements.

    4. The type of security, as well as the type, is better to make an enumeration, not a string, because There are a limited number of types and types, and the lines require additional validation during assignment, with enumerations it is a bit simpler, and the code looks better.

    5. For all fields, specify the access level private and provide properties with getters and setters with the necessary access levels.

    About an error with protected properties you have already written. In general, everything that I wrote to you should have been explained at lectures.