Czy dobrze uczę się tworzenia klas?

0

Witam! Uczę się programowania od niedawna i potrzebuję informacji zwrotnej czy w poprawny sposób modeluje klasy (aplikacje konsolowe). Dla przykładu wstawiam klasę jednego z moich programów (w podobny sposób robię inne). Co mógłbym poprawić? Może kod jest zbyt mało czytelny? Czy dobrze zrobiłem, że propertisy ustawiam oddzielnymi metodami sprawdzającymi poprawność danych? A i jeszcze jedno. Na razie roboczo gdy dane są niepoprawne rzucam po prostu wyjątkiem. W jaki sposób najlepiej, najpoprawniej dać użytkownikowi możliwość poprawy gdyby to on wprowadzał dane? Np. gdy poda złą nazwę sklepu, tworzyć metodę w tej samej klasie która będzie pozwalała skorygować mu błąd i wykorzystać ją w setShopName();? Czy może oddzielna klasa odpowiedzialna za możliwość korekty pomyłek użytkownika? Pytania pewnie banalne ale na początku drogi nie chcę wyrabiać złych nawyków.

using System;

namespace ShopManager
{
    public class Shop
    {
        public string ShopName { get; private set; }
        public string LocationId { get; private set; }
        public int Id { get; private set; }
        public bool IsOpen { get; }

        public Shop(string shopName, string locationId, int id) 
        {
            setShopName(shopName);
            setLocationId(locationId);
            setId(id);
            IsOpen = setIsOpen();
        }

        public void setShopName(string shopName) 
        {
            if (string.IsNullOrWhiteSpace(shopName)) 
            {
                throw new ArgumentException($"{shopName} is null or white spaces.");
            }

            ShopName = shopName;
        }

        public void setLocationId(string locationId)
        {
            if (string.IsNullOrWhiteSpace(locationId))
            {
                throw new ArgumentException(nameof(locationId));
            }
            else if (locationId.Length != 3) 
            {
                throw new ArgumentException(nameof(locationId));
            }

            LocationId = locationId;
        }

        public void setId(int id)
        {
            if (id <= 0)
            {
                throw new ArgumentOutOfRangeException($"{id} is lower than 0");
            }

            Id = id;
        }

        //method sets the property to true when hour is between 7 and 21.
        //the shop is closed on Sundays then return false when day of week is 0 (Sunday) 
        private bool setIsOpen() 
        {
            if((int)DateTime.Now.DayOfWeek == 0) 
            {
                return false;
            }

            if (!(DateTime.Now.Hour >= 7 && DateTime.Now.Hour <=21)) 
            {
                return false;
            }

            return true;
        }

        public override string ToString()
        {
            string ShopInformation = "";
            if (IsOpen == true) 
            {
                ShopInformation = "ShopName: " + "\nLocationId: " + LocationId + "\nId: " + Id + "\nIsOpen: Yes"; 
            }
            else 
            {
                ShopInformation = "ShopName: " + "\nLocationId: " + LocationId + "\nId: " + Id + "\nIsOpen: No";
            }
            return ShopInformation;
        }
    }
}
3

Po co ci te metody set, skoro masz właściwości? Rzucać wyjątkami jeszcze można przy tworzeniu obiektu, ale już później to tak chyba średnio. Metodę tostring możesz uprościć do jednej linijki przy użyciu : https://docs.microsoft.com/pl[...]perators/conditional-operator

3
  1. W C# nazwy metod powinny się zaczynać od dużej litery.
  2. Twoja metoda setIsOpen niczego nie ustawia tylko zwraca wynik boolean. Zamiast tego powinieneś po prostu użyć read only właściwości IsOpen i tak to obliczać. Ewentualnie wystawić publiczna metodę IsOpen zwracającą bool.
  3. Ogólnie to tak jak napisał @kzkzg większość z tego możesz zamienić na właściwości i usunąć te metody.
0

@Aventus: Czyli lepiej byłoby to zrobić w taki sposób?

private string _shopName;

public string ShopName 
        {
            get => _shopName;
            set 
            {
                if (string.IsNullOrWhiteSpace(value))
                {
                    throw new ArgumentException($"{value} is null or white spaces.");
                }
                _shopName = value;
            }
        }
2

W takich prostych przypadkach tak.

0

@Aventus: A co gdyby walidacja obejmowała dużo rzeczy do sprawdzenia? Przy większej ilości kodu ma sens tworzenie metody SetShopName(), która to wszystko posprawdza dla tego pola, tak jak zrobiłem to na początku?
A no i jeszcze co z ewentualną możliwością korekty danych dla użytkownika? Pisać w tej samej klasie metody które pozwolą użytkownikowi podać jeszcze raz nazwę sklepu, czy może oddzielna klasa, która będzie za takie coś odpowiadała? Z tego co rozumiem klasa powinna być odpowiedzialna za jedną rzecz, więc skłaniałbym się wydzielić inną klasę pozwalającą na korektę i ewentualnie wykorzystać ją w set później. Pytanie czy taki tok rozumowania jest prawidłowy?

4
Abachaczi napisał(a):

@Aventus: A co gdyby walidacja obejmowała dużo rzeczy do sprawdzenia? Przy większej ilości kodu ma sens tworzenie metody SetShopName(), która to wszystko posprawdza dla tego pola, tak jak zrobiłem to na początku?
A no i jeszcze co z ewentualną możliwością korekty danych dla użytkownika? Pisać w tej samej klasie metody które pozwolą użytkownikowi podać jeszcze raz nazwę sklepu, czy może oddzielna klasa, która będzie za takie coś odpowiadała? Z tego co rozumiem klasa powinna być odpowiedzialna za jedną rzecz, więc skłaniałbym się wydzielić inną klasę pozwalającą na korektę i ewentualnie wykorzystać ją w set później. Pytanie czy taki tok rozumowania jest prawidłowy?

Wtedy najlepiej wynieść walidację do osobnej klasy. Można zacząć od zrobienia wszystkich właściwości z automatycznymi getterami i setterami.

public class Shop
{
    public string ShopName { get; set; }
    public string LocationId { get; set; }
    public int Id { get; set; }
    public bool IsOpen { get; set; }
}

Interfejs dla walidatora oraz klasa błędu walidacji:

public class ValidationError
{
    public ValidationError(string propertyName, string errorMessage)
    {
        PropertyName = propertyName;
        ErrorMessage = errorMessage;
    }

    public string PropertyName { get; }
    public string ErrorMessage { get; }
}

public interface IValidator<in T>
{
    IReadOnlyList<ValidationError> Validate(T model);
}

Oraz walidator dla klasy Shop:

public class ShopValidator : IValidator<Shop>
{
    public IReadOnlyList<ValidationError> Validate(Shop model)
    {
        var result = new List<ValidationError>();

        if (string.IsNullOrWhiteSpace(model.ShopName)) 
        {
            result.Add(new ValidationError(nameof(model.ShopName), "shop name cannot be null"));
        }

        // ...
        // other validations

        return result;
    }
}

Na szczęście żeby nie robić tego wszystkiego samemu ręcznie można wykorzystać jakąś gotową bibliotekę do walidacji. Polecam hasło w google validation library c#. Najpopularniejsza jest FluentValidation i ja jej najczęściej używam.

3

Są różne patterny do tworzenia obiektów np. Builder, wtedy taką klasę udostępniasz jedynie poprzez Builder (prywatne konstruktory), a Builder zapewnia że dane są poprawne.

var buildingResult = new ShopBuilder()
                     .SetName("xd")
                     .SetLocation("fg")
                     .TryBuild();

if (!buildingResult.Succeed)
    ...
2

Co do takich prostych walidacji to tak jak napisali koledzy wyżej. Jeśli chodzi o egzekwowanie zasad biznesowych to ja jestem za tym żeby trzymać to najbliżej źródła, czyli w samych obiektach domenowych/biznesowych. W przeciwnym razie obiekty te stają się tylko zwykłymi pojemnikami na dane, co samo w sobie jest przeciwne do tego czym OOP miało być w założeniu. Obiekty powinny posiadać dane, ale i zachowanie (ang. behaviour).

0

W sumie zastanawiam się jeszcze czy na początku nauki, we w miarę prostych aplikacjach konsolowych jest sens implementować bardziej złożone rzeczy. Chyba zrobię to w miarę jak najprościej się da na tym etapie aby potestować rzeczy których się uczę z językaa gdy przejdę do nauki ASP.NET Core zacznę tego się uczyć naprawdę mocno i przyłożę do tego większą uwagę. Ciężko w tych aplikacjach konsolowych jakoś implementować takie bardziej złożone mechanizmy, wydaje mi się, że tworząc np. Stronę internetową, czy jakiś backend do systemu logowania, sprzedaży biletów będzie łatwiej to odwzorować wszystko :D

1 użytkowników online, w tym zalogowanych: 0, gości: 1, botów: 0