hsim
hsim

Reputation: 2080

Isolate class to be instantiated only by a specific class: best practices

Ok, I am working with 2 layers of class: somekind of a "manager" class which task is to take an object, link all items attached to it, and return it to the caller, and a "data" class which task is to call the database on a specific request.

Here's an example of how I work. Here's a "manager" class:

public class CardManager
{
    private static readonly CardData mCardDAL = new CardData();

    public List<CardDisplay> ListCardsToShow(int _pageNumber, string _queryString, string _rarity, string _type, string _color, out int _totalCount)
    {
        List<CardDisplay> listToReturn = mCardDAL.ListCardsToShow(_pageNumber, _queryString, _rarity, _type, _color, out _totalCount);

        LinkListCardDisplayData(listToReturn);

        return listToReturn;
    }


    /// <summary>
    /// Method links the card set with each cards of the list.
    /// </summary>
    /// <param name="_listToReturn"></param>
    private static void LinkListCardDisplayData(IEnumerable<CardDisplay> _listToReturn)
    {
        try
        {
            foreach (CardDisplay item in _listToReturn)
            {
                LinkCardDisplayData(item);
            }
        }
        catch (Exception ex)
        {
            throw new Exception(ex.Message);
        }
    }

    private static void LinkCardDisplayData(CardDisplay _item)
    {
        _item.mMasterCardID = _item.mMasterCard.mCardID;

        ImagesManager.GetCardImages(_item);

        if (_item.mChildCard != null)
        {
            _item.mChildCardID = _item.mChildCard.mCardID;
        }
    }
}

And here's a "data" class, namely the CardData class in this occurrence:

public class CardData
{
    internal List<CardDisplay> ListCardsToShow(int _pageNumber, string _queryString, string _rarity, string _type, string _color, out int _totalCount)
    {
        using (DatabaseEntity db = new DatabaseEntity())
        {
            db.Database.Connection.Open();

            List<CARD> cardData;

            List<CardInfo> listCards;

            if (!String.IsNullOrWhiteSpace(_queryString))
            {
                var predicate = GetCardPredicate(_queryString);

                if (_rarity != "All")
                {
                    predicate = predicate.And(_item => _item.CARD_RARTY == _rarity);
                }

                if (_color != "All")
                {
                    predicate = predicate.And(
                            _item => _item.CARD_MANA_COST.Contains(_color) || _item.CARD_COLOR.Contains(_color));
                }

                if (_type != "All")
                {
                    predicate = predicate.And(_item => _item.CARD_TYPE.Contains(_type));
                }

                var cardQry = from c in db.CARD.AsExpandable().Where(predicate)
                              select c;

                _totalCount = cardQry.Count();

                int pageCount = _pageNumber - 1;

                cardData = cardQry.OrderBy(_x => _x.CARD_IDE).Skip(pageCount * 20).Take(20).ToList();

                for (int i = 0; i < cardData.Count; i++)
                {
                    CARD card = cardData[i];

                    if (cardData.Any(_item => _item.CARD_MASTER_IDE == card.CARD_IDE))
                    {
                        cardData.Remove(card);
                    }
                }

                listCards = DataConverter.ListCardDATAToListCardInfo(cardData);
            }
            else
            {
                // If we are here then the user browsed to get the 300 latest entries available.

                Expression<Func<CARD, bool>> cardPredicate = PredicateBuilder.True<CARD>();

                if (_rarity != "All")
                {
                    cardPredicate = cardPredicate.And(_item => _item.CARD_RARTY == _rarity);
                }

                if (_type != "All")
                {
                    cardPredicate = cardPredicate.And(_item => _item.CARD_TYPE.Contains(_type));
                }

                if (_color != "All")
                {
                    cardPredicate =
                        cardPredicate.And(
                            _item => _item.CARD_MANA_COST.Contains(_color) || _item.CARD_COLOR.Contains(_color));
                }

                var cardQry = (from c in db.CARD.AsExpandable().Where(_item => !_item.CARD_NAME.Contains("(Foil)"))
                                select c).OrderByDescending(_x => _x.CARD_SET.CARD_SET_RELES_DATE).Take(300);

                cardQry = cardQry.Where(cardPredicate);

                _totalCount = cardQry.Count();

                int pageCount = _pageNumber - 1;

                cardData = cardQry.Skip(pageCount * 20).Take(20).ToList();

                for (int i = 0; i < cardData.Count; i++)
                {
                    CARD card = cardData[i];

                    if (cardData.Any(_item => _item.CARD_MASTER_IDE == card.CARD_IDE))
                    {
                        cardData.Remove(card);
                    }
                }

                listCards = DataConverter.ListCardDATAToListCardInfo(cardData);
            }

            List<CardDisplay> listToReturn = MakeListCardDisplay(listCards);

            return listToReturn;
        }
    }
}

My question is not on "how" I do my code (but feel free to make a constructive comment as I love to learn), but rather on how it is structured. I'd like, for example, that a CardData class be exclusively instantiated by a "manager" class, meaning that I could not, for example, create a public static readonly CardData mCardDAL = new CardData(); object in a controller.

Is there a way to isolate my class in such a manner that anyone is "forced" to pass through the manager to get the object? And I am working in a good way, is my code good?

Upvotes: 2

Views: 153

Answers (4)

Arin Taylor
Arin Taylor

Reputation: 390

From the code provided, it looks like you may want to put the CardData class within the CardManager class and make it private so that only CardManager can access it.

public class CardManager
{
    private class CardData { ... }
}

Upvotes: 1

Patrick Hofman
Patrick Hofman

Reputation: 156948

It is possible to deny access from the outside by making CardData a nested class inside Manager, block it's constructor by making it protected, and create a private instance class:

public class Manager
{
    public class Manager
    {
        CardData c = new CardDataInternal();
    }

    private class CardDataInternal : CardData
    {
        public CardDataInternal()
        { }
    }

    public class CardData
    {
        protected CardData()
        { }
    }
}

Upvotes: 4

Robert Levy
Robert Levy

Reputation: 29073

An alternative to @Patrick's approach using interfaces:

public interface ICardData { ... }
public class Manager
{
    public class Manager()
    {
        CardData c = new CardData();
    }

    private class CardData : ICardData
    {
        public CardData() { }
    }
}

By using interfaces instead of a constructor on a nested private class, you're also set up to do unit testing with mock implementations... probably not important for CardData but if you are doing this on a class that has interesting behavior, it is very important.

Upvotes: 2

Damien_The_Unbeliever
Damien_The_Unbeliever

Reputation: 239646

You can keep both objects public by making the manager class a nested class and then give the Card class a private constructor (that nested classes are allowed to call), something like:

public class Card
{
    private Card()
    {

    }
    public class Manager
    {
        public Card Create()
        {
            return new Card();
        }
    }
}

So you can freely create new Card.Manager() but the only way to create a new Card is via the Create() method.

Upvotes: 3

Related Questions