HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

CardManager class and a data access layer for it

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
layercardmanagerdataforandclassaccess

Problem

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 ListCardsToShow(int _pageNumber, string _queryString, string _rarity, string _type, string _color, out int _totalCount)
    {
        List listToReturn = mCardDAL.ListCardsToShow(_pageNumber, _queryString, _rarity, _type, _color, out _totalCount);

        LinkListCardDisplayData(listToReturn);

        return listToReturn;
    }

    /// 
    /// Method links the card set with each cards of the list.
    /// 
    /// 
    private static void LinkListCardDisplayData(IEnumerable _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 ListCardsToShow(int _pageNumber, string _queryString, string _rarity, string _type, string _color, out int _totalCount)
{
using (DatabaseEntity db = new DatabaseEntity())
{
db.Database.Connection.Open();

List cardData;

List listCards;

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

Solution

You could use a private nested class:

public class CardManager
{
    public CardManager()
    {
        var card = new CardData();
    }

    private class CardData
    {
    }
}


CardData will not be able to be instantiated outside CardManager. For example, this will not compile

public class Other
{
    public Other()
    {
        CardData card = new CardData();
    }
}


I don't understand why CardData is a class. It's got one method, no fields, no properties. Its one method, at 103 lines, is almost certainly doing too much and should be split up into smaller methods.

Parameter names should not start with an underscore, by convention.

try
    {
        foreach (CardDisplay item in _listToReturn)
        {
            LinkCardDisplayData(item);
        }
    }
    catch (Exception ex)
    {
        throw new Exception(ex.Message);
    }


This has lost the stack trace. The try-catch should be removed entirely.

Code Snippets

public class CardManager
{
    public CardManager()
    {
        var card = new CardData();
    }

    private class CardData
    {
    }
}
public class Other
{
    public Other()
    {
        CardData card = new CardData();
    }
}
try
    {
        foreach (CardDisplay item in _listToReturn)
        {
            LinkCardDisplayData(item);
        }
    }
    catch (Exception ex)
    {
        throw new Exception(ex.Message);
    }

Context

StackExchange Code Review Q#65087, answer score: 7

Revisions (0)

No revisions yet.