How to remove repeating code in this method?

Question

private static Game[] getMostPlayedGamesDo(int Fetch, int CategoryID)
{
    Game[] r;
    using (MainContext db = new MainContext())
    {
        if (CategoryID == 0)
        {
            var q = db.tblArcadeGames.OrderByDescending(c => c.Plays).Take(Fetch);
            r = new Game[q.Count()];
            int i = 0;
            foreach (var g in q)
            {
                r[i] = new Game(g);
                i++;
            }
        }
        else
        {
            var q = db.tblArcadeGames.Where(c=>c.CategoryID == CategoryID).OrderByDescending(c => c.Plays).Take(Fetch);
            r = new Game[q.Count()];
            int i = 0;
            foreach (var g in q)
            {
                r[i] = new Game(g);
                i++;
            }
        }
    }
    return r;
}

I can't seem to define q outside the scope of the if, and I can't insert the returned values to the array outside the scope of the if! Not sure how to remove repeating code in this simple instance?

Can you not make q a concrete type? What is q in both instances? Must have a common base? — JamesSugrue, Nov 19 '11 at 04:52
@Kiwi sorry I don't really understand it too well, I've always defined my linq to sql queries as var types — Tom Gullen, Nov 19 '11 at 04:57
@Tom, what is the Class for tblArcadeGames generated in your dbml? If it's tblArcadeGame then the type of q can be List or IQueryable. Look at Kirks Woll's answer and mine. — kazinix, Nov 19 '11 at 05:02
Setting a break point after the call and inspecting the type should tell you what q is... — JamesSugrue, Nov 19 '11 at 05:03

Kirk Woll · Accepted Answer · 2011-11-19T05:03:43.687

It's not clear what the type of q is -- but deducing from your usage:

db.tblArcadeGames.OrderByDescending(...)

Presumably it's an entity class from Linq-To-Sql or Entity Framework. In that case, you do have a concrete entity defined, presumably named tblArcadeGame. Therefore, move q out of the scope by not using var:

IQueryable<tblArcadeGame> q;
if (CategoryID == 0)
{
    q = db.tblArcadeGames.OrderByDescending(c => c.Plays).Take(Fetch);
}
else
{
    q = db.tblArcadeGames.Where(c=>c.CategoryID == CategoryID).OrderByDescending(c => c.Plays).Take(Fetch);
}
r = new Game[q.Count()];
int i = 0;
foreach (var g in q)
{
    r[i] = new Game(g);
    i++;
}

As you can see, the repeated code is now seen only once.

P.S. Tools like ReSharper are fantastic for this sort of thing. Using it, with one keystroke you can toggle between the var version and that using explicitly named types.

score 1 · Answer 2 · answered Nov 19 '11 at 05:02

You should really just explicitly type q. But this may let you get away without it (ternary operator will enforce it for you).

var q = CategoryID == 0 ? db.tblArcadeGames.OrderByDescending(c => c.Plays).Take(Fetch)
                        : db.tblArcadeGames.Where(c=>c.CategoryID == CategoryID).OrderByDescending(c => c.Plays).Take(Fetch);

r = new Game[q.Count()];
int i = 0;
foreach (var g in q)
{
    r[i] = new Game(g);
    i++;
}

score 1 · Answer 3 · answered Nov 19 '11 at 05:09

I assume q is type IQueryable.

private static Game[] getMostPlayedGamesDo(int Fetch, int CategoryID)
{
    var q = db.tblArcadeGames;
    if (CategoryID != 0)
    {
        q = q.Where(c => c.CategoryID == CategoryID);
    }
    q = q.OrderByDescending(c => c.Plays).Take(Fetch);
    return q.Select(g => new Game(g)).ToArray();
}

kazinix · Answer 4 · 2011-11-19T05:10:44.800

0

    List<tblArcadeGame> q;
    /* object q; */


    if (CategoryID == 0)
    {
        q = db.tblArcadeGames.OrderByDescending(c => c.Plays).Take(Fetch).ToList();
    }
    else
    {
        q = db.tblArcadeGames.Where(c=>c.CategoryID == CategoryID).OrderByDescending(c => c.Plays).Take(Fetch).ToList();
    }

        r = new Game[q.Count()];
        int i = 0;
        foreach (var g in q)
        {
            r[i] = new Game(g);
            i++;
        }

I'll assume q is List<tblArcadeGame>

edited Nov 19 '11 at 05:10

answered Nov 19 '11 at 04:48

kazinix

28,987
33
107
157

Two things. You can't have a var without a rhs, and even if you could in this example q would be three different variables – JamesSugrue Nov 19 '11 at 04:51
Doesn't work, `Implicitly-typed local variables must be initialized` on the first `var q;`. This is what's causing me difficulty as it's easy to remove repeating code but not so when linq is involved. – Tom Gullen Nov 19 '11 at 04:51
I don't know... I assume all codes he provided are correct. I just reconstructed it. – kazinix Nov 19 '11 at 04:51

How to remove repeating code in this method?

4 Answers4