LINQ queries improvement - repetition of query

I have a DAL class full of LINQ queries which I would like to improve as I found that I'm repeating myself couple of times.

As you can see the part below is repeated couple of times and I was wondering if it is possible to reduce the number of repetitions (passing the query somehow?) and/or improve the queries overall.


.Where(x => x.Active == true && x.Variable.Active == true && x.Variable.Question.Active == true)

private DashboardEntities db;

    public AnswerSelectLabelRepository(DashboardEntities _db)
    {
        this.db = _db;
    }

    public AnswerSelectLabel GetById(string idAnswerSelectLabel)
    {
        return db.AnswerSelectLabel.Find(idAnswerSelectLabel);
    }

    public IEnumerable GetAll()
    {
        return db.AnswerSelectLabel
            .Where(x => x.Active == true && x.Variable.Active == true && x.Variable.Question.Active == true)
            .OrderBy(x => x.DisplayOrder)
            .ThenBy(x => x.Value);
    }

    public IEnumerable GetByVariable(string idVariable)
    {
        return GetAll().Where(x => x.idVariable == idVariable);
    }

    public IEnumerable GetByVariable(IEnumerable respondents, string idVariable)
    {
        return respondents.SelectMany(x => x.AnswerSelect)
            .Select(x => x.AnswerSelectLabel)
            .Where(x => x.Active == true && x.Variable.Active == true && x.Variable.Question.Active == true)
            .Where(x => x.idVariable == idVariable);
    }

    public IEnumerable GetByQuestion(string idQuestion)
    {
        return GetAll().Where(x => x.Variable.idQuestion == idQuestion);
    }

    public IEnumerable GetByQuestion(IEnumerable respondents, string idQuestion)
    {
        return respondents.SelectMany(x => x.AnswerSelect)
            .Select(x => x.AnswerSelectLabel)
            .Where(x => x.Active == true && x.Variable.Active == true && x.Variable.Question.Active == true)
            .Where(x => x.Variable.idQuestion == idQuestion);
    }

The first thing (and this is very minor) is that the expression a == true and simply a are identical. Thus you don't need all those == true's.

The simplest, most obvious way is to simply extract a private method for your filter:

private boolean isActive(AnswerSelectLabel answer)
{
  return answer.Active
      && answer.Variable.Active 
      && answer.Variable.Question.Active
}

public IEnumerable GetAll()
{
  return db.AnswerSelectLabel
      .Where(isActive) //you'll pardon me if this is syntax error, I've been in java land too long now, where this would be this::isActive
      .OrderBy(x => x.DisplayOrder)
      .ThenBy(x => x.Value);
}

public IEnumerable GetByVariable(IEnumerable respondents, string idVariable)
{
  return respondents.SelectMany(x => x.AnswerSelect)
      .Select(x => x.AnswerSelectLabel)
      .Where(isActive)
      .Where(x => x.idVariable == idVariable);
}

and so on.

Slightly more sophisticated (and almost certainly overkill), is to extract some kind of interface on your answer, variable, and Question, classes. Something like

interface IActivatable
{
  public Active { get; }
}

If you make this change, you can start to play games with exactly what you're asking to be active:

public IEnumerable GetByVariable(IEnumerable respondents, string idVariable)
{
  return respondents.SelectMany(x => x.AnswerSelect)
    .Select(x => x.AnswerSelectLabel)
    .Where(x => new IActivatable[]{x, x.Answer, x.Answer.Question}.all(x => x.Active))
    .Where(x => x.idVariable == idVariable);
}

which, again, we can extract a method for. But this time we're parameterized on the actual things we want to check, which is nice because it means we can pick-and-choose what we want to be active. In your case it looks like this is always answer, question, and variable, but if that's not the case this is likely helpful:

public IEnumerable GetByVariable(IEnumerable respondents, string idVariable)
{
  return respondents.SelectMany(x => x.AnswerSelect)
    .Select(x => x.AnswerSelectLabel)
    .Where(x => isActive(x, x.Answer, x.Answer.Question))
    .Where(x => x.idVariable == idVariable);
}

private boolean isActive(params IActivateable activatables){
  return activatables.all(x => x.Active);
}

Of course, if this isn't the case, this is just useless speculative code, so don't do it.

If I could make a couple more notes:

• using strings as a catch-all for ID's has gotten me in trouble, so now I push to create specialized ID classes (where a simple GUID wont do). Don't let this burn you too!

-
GetByQuestion(IEnumerable respondents, string idQuestion) and GetByVariable(IEnumerable respondents, string idVariable) are suffering from a sort of linq-feature-envy. They don't use your db field at all, so why are they on this class? Do these methods exist so that I can do something like this:

var respondents = dal.GetAllRespondents();
respondents = respondents.Where(callerSpecificFilter);
var asnwers = GetByVariable(respondents, "1234");

If this is the case, and you can afford the potential performance hit, you can remove the respondents parameter and take a Func filter instead. Then simply apply the filter yourself.

StackExchange Code Review Q#111880, answer score: 10