Locating people by roleId

Question

Basically, I'm trying to find all active people with a roleId of 3 (analyst id). But I want to exclude ids that were created as test analysts, i.e. "130", "252", etc. I see that I'm repeating the part to exclude the test ids. Also, I'm repeating the part about the roleId. Isn't there a way to optimize it? My query works fine, but I'm trying to optimize to make it more DRY.

Here's what my code looks like (I've labeled where I think the code can be optimized):

from people in xDoc.Descendants("People")
where (int?) Convert.ToInt16(people.Element("active").Value) == 1 &&
    (int?)Convert.ToInt16(people.Element("id").Value) != 130 &&  // <-- to optimize
    (int?)Convert.ToInt16(people.Element("id").Value) != 252 &&  // <-- to optimize
    (int?)Convert.ToInt16(people.Element("id").Value) != 264 &&  // <-- to optimize
    (int?)Convert.ToInt16(people.Element("id").Value) != 302 &&  // <-- to optimize
    people.Descendants("roleIds").Descendants("int")
        .Select(p => (int?)Convert.ToInt16(p.Value)).FirstOrDefault() == 3  // <-- to optimize
select new PeopleClass
{
    ResAnId = (int) Convert.ToInt16(people.Element("id").Value),
    DpInit = (string)people.Element("clientCode").Value,
    ResAnFName = (string)people.Element("firstName").Value,
    ResAnLName = (string)people.Element("lastName").Value,
    ResAnName = String.Format("{0} {1}", (string)people.Element("firstName").Value,
        (string)people.Element("lastName").Value),
    RoleId = people.Descendants("roleIds").Descendants("int")
        .Select(p => (int?) Convert.ToInt16(p.Value)).FirstOrDefault(),  // <-- to optimize
    DivisionId = (int?) Convert.ToInt16(people.Element("divisionId").Value)
}).ToList();

If you're interested, here's my XML feed:

<People>
  <id>269</id>
  <dummy>false</dummy>
  <firstName>Steve</firstName>
  <lastName>Jobs</lastName>
  <initial></initial>
  <title>CEO</title>
  <active>1</active>
  <analystPosition></analystPosition>
  <clientCode>SJ</clientCode>
  <clientCode2></clientCode2>
  <divisionId>2</divisionId>
  <updatedBy>245</updatedBy>
  <associates>
    <Associate>
      <analystId>269</analystId>
      <associateId>257</associateId>
      <rank>0</rank>
    </Associate>
  </associates>
  <roleIds>
    <int>3</int>
    <int>4</int>
    <int>13</int>
  </roleIds>
</People>

Answer 1

You can introduce range variables to hold data which you need several times in your query:

var analystRoleId = 3;
var testAnalystIds = new[] { 130, 252, 264, 302 };

var activeAnalysts = 
    from p in xdoc.Descendants("People")
    let id = (int)p.Element("id")
    let isActive = (int)p.Element("active") == 1                        
    let isAnalyst = p.Descendants("roleIds").Elements().Any(e => (int)e == analystRoleId)
    let firstName = (string)p.Element("firstName")
    let lastName = (string)p.Element("lastName")
    where isActive && isAnalyst && !testAnalystIds.Contains(id)
    select new PeopleClass {
        ResAnId = id,
        DpInit = (string)p.Element("clientCode"),
        ResAnFName = firstName,
        ResAnLName = lastName,
        ResAnName = $"{firstName} {lastName}",
        RoleId = analystRoleId,
        DivisionId = (int?)p.Element("divisionId")
    };

Also

• You can use interpolated strings instead of String.Format
• I believe id always should present in your file and it will not be null. There is no need to cast this value to nullable
• I believe you should check all role ids for verifying whether person is analyst. But you can use FirstOrDeafult as well
• You don't need to query for roleId second time - if person matched where clause then it's analyst and you know his role id
• You don't need to access Value property of XElement. It's better to use explicit cast operator. Otherwise you can get NullReferenceExceptions
• If you can change implementation of PeopleClass then start with renaming it to Person and give more descriptive names to its fields. You can also remove backing storage from ResAndName filed and make it calculated on the fly.

Answer 2

The query as shown in @Sergey Berezovskiy's answer is ok for a one time query but if you are going to create the People class more then once from XElements then you should encapsulate it and let the People class parse the XElement

class People
{
    private People(XElement xPeople)
    {
        id = (int)xPeople.Element("id");
        ..other properties
    }

    public int id { get; set; }

    .. other properties

    public static People Parse(XElement xPeople) => new People(xPeople);
}

You could then simplify the query to

var activeAnalysts = 
    from p in xdoc.Descendants("People").Select(People.Parse)    
    where p.isActive && p.isAnalyst && !testAnalystIds.Contains(p.id)
    select new p;

Other then this I support the suggestions given in the other answer.