locked
Using bool as params and get the same results in if else conditions RRS feed

  • Question

  • User-501297529 posted

    I'm trying to get archived and current documents using the method below but I get current documents to show as the results when I run the application. When I run through the code the IsArchived param is always set to false and therefore I will always see current documents when I should see archived documents. It never hits the if (IsArchived) true condition

    I'm calling two methods (GetArchivedDocs and GetCurrentDocs and setting them to the GetDocuments method below

    private IQueryable<DocEntity> GetDocuments(bool isArchived, int? committeeTypeId)
            {
                var filteredDocs = DbContext
                    .Set<DocEntity>()
                    .AsNoTracking()
                    .Where(p =>  !p.IsDeleted);
    
                if (committeeTypeId != null)
                {
                    filteredDocs = filteredDocs
                        .Where(p => (p.CommitteeTypeId == committeeTypeId));
                }
    
                if (isArchived)
                {
                    filteredDocs = filteredDocs
                        .Where(p => p.IsArchived == true && !p.IsCurrent);
                }
                else
                {
                    filteredDocs = filteredDocs.Where(p => (p.IsArchived == false || p.IsArchived == null) && p.IsCurrent);
                }
    
                return filteredDocs;
            }

    public IEnumerable<DocEntity> GetArchivedDocs(int? committeeTypeId) =>
    GetDocuments(true, committeeTypeId)

    public IEnumerable<PacketEntity> GetCurrentDocs(int? committeeTypeId) =>
    GetDocuments(false, null);

    Thursday, July 30, 2020 3:58 AM

Answers

  • User753101303 posted

    And then this outer GetArchivedDocs method is called from where ? In short my first move would be to make 100% sure that the test page or whatever is really calling GetArchivedDocs which ends up calling GetDocuments(true).

    For now all happens as if you are calling GetCurrentDocs instead causing GetDocuments(false) to be called. BTW what if you just try for example :

     public IEnumerable<DocEntity> GetArchivedDocs(int? committeeTypeId) =>
    GetDocuments(true, committeeTypeId)

    public IEnumerable<PacketEntity> GetCurrentDocs(int? committeeTypeId) =>
    GetDocuments(true, committeeTypeId); // HACK: pass true and see if you see now true in GetDocuments

    Now passing false to GetDocuments should be just not possible at all. If it "fixes" the problem it would show you really end up in calling GetCurrentDocs rather than GetArchivedDocs.

    If it doesn't change anything then it seems GetDocuments(false) could be called from somewhere else.

    Also could you confirm you really see that isArchived is false or do you have some unexpected behavior that just make you think it could be caused by isArchived being false ?

    • Marked as answer by Anonymous Thursday, October 7, 2021 12:00 AM
    Thursday, July 30, 2020 1:37 PM

All replies

  • User1120430333 posted

    You can always pass in an int 1 or 0.

    https://www.wyzant.com/resources/answers/626873/1-false-and-0-true

    Thursday, July 30, 2020 5:02 AM
  • User-775646050 posted

    Can you post the code that calls GetArchivedDocs? Have you set a breakpoint in GetArchivedDocs to see if it is ever actually called?

    Thursday, July 30, 2020 5:23 AM
  • User-501297529 posted

    smtaz

    Can you post the code that calls GetArchivedDocs? Have you set a breakpoint in GetArchivedDocs to see if it is ever actually called?

    I did it's this

     public IEnumerable<DocEntity> GetArchivedDocs(int? committeeTypeId) =>
                GetDocuments(true, committeeTypeId)

    I can't see anything because it never gets into that code. It should get into this code

     if (isArchived)
                {
                    filteredDocs = filteredDocs
                        .Where(p => p.IsArchived == true && !p.IsCurrent);
                }

    Thursday, July 30, 2020 5:26 AM
  • User-501297529 posted

    How would I do that? I read that link and it neither applies or makes sense to what I'm doing 

    Thursday, July 30, 2020 5:28 AM
  • User-775646050 posted

    You posted the GetArchivedDocs method but you didn't post how it is called. It may not be actually being called which is why the isArchived is always false. Place a breakpoint on this line inside of your GetArchivedDocs method:

    GetDocuments(true, committeeTypeId)

    If the code does not reach this, you will need to figure out why it is not called.

    Thursday, July 30, 2020 5:40 AM
  • User-501297529 posted

    smtaz

    You posted the GetArchivedDocs method but you didn't post how it is called. It may not be actually being called which is why the isArchived is always false. Place a breakpoint on this line inside of your GetArchivedDocs method:

    GetDocuments(true, committeeTypeId)

    If the code does not reach this, you will need to figure out why it is not called.

    Not sure what you mean by placing a break point inside the line. There's nothing after or inside that method. It calls the GetsDocuments method.

    This is all there is for that method

    public IEnumerable<DocEntity> GetArchivedDocs(int? committeeTypeId) =>
                GetDocuments(true, committeeTypeId)
    

    Do you mean place a break point on the  GetDocuments(true, committeeTypeId) line? 

    Thursday, July 30, 2020 5:44 AM
  • User-775646050 posted
    public IEnumerable<DocEntity> GetArchivedDocs(int? committeeTypeId) =>
                GetDocuments(true, committeeTypeId)

    is just shorthand for this:

    public IEnumerable<DocEntity> GetArchivedDocs(int? committeeTypeId)
    { GetDocuments(true, committeeTypeId) }

    Place the breakpoint on the line calling GetDocuments inside your GetArchivedDocs method. Now when you run your code does execution stop at the breakpoint? If not, then the method is never being called.

    Thursday, July 30, 2020 5:54 AM
  • User-501297529 posted

    smtaz

    public IEnumerable<DocEntity> GetArchivedDocs(int? committeeTypeId) =>
                GetDocuments(true, committeeTypeId)

    is just shorthand for this:

    public IEnumerable<DocEntity> GetArchivedDocs(int? committeeTypeId)
    { GetDocuments(true, committeeTypeId) }

    No it's not.

    smtaz

    public IEnumerable<DocEntity> GetArchivedDocs(int? committeeTypeId)
    { GetDocuments(true, committeeTypeId) }

    The GetDocuments method is combining the GetArchiveDocs and GetCurrentDocs. Instead of having if conditions in those two methods they are in one method that's why I am using =>. . THe committeeTypeId check is the constant in GetDocuments method before I check if it is archived or current. I can only get one (current) to work. I do this because if I need to make an updated down the road I only have to do it in one place, GetDocuments method and not in two GetArchived/GetCurrentDocs.

    It  hits the bolded section of the code, but not the italicized section.

    private IQueryable<DocEntity> GetDocuments(bool isArchived, int? committeeTypeId)
            {
                var filteredDocs = DbContext
                    .Set<DocEntity>()
                    .AsNoTracking()
                    .Where(p =>  !p.IsDeleted);
    
                if (committeeTypeId != null)
                {
                    filteredDocs = filteredDocs
                        .Where(p => (p.CommitteeTypeId == committeeTypeId));
                }
    
                if (isArchived)
                {
                    filteredDocs = filteredDocs
                        .Where(p => p.IsArchived == true && !p.IsCurrent);
                }
                else
                {
                    filteredDocs = filteredDocs.Where(p => (p.IsArchived == false || p.IsArchived == null) && p.IsCurrent);
                }
    
                return filteredDocs;
            }

    Does this make sense?

    Thursday, July 30, 2020 6:00 AM
  • User-775646050 posted

    Read the section Expression body definition here. When you call GetArchivedDocs or GetCurrentDocs you are calling GetDocuments. What you are not understanding is that GetArchivedDocs may never be called. This is why you need to set breakpoints on the GetDocuments(true, committeeTypeId) line inside your GetArchivedDocs method. Can you post the code that calls GetArchivedDocs?

    Thursday, July 30, 2020 6:09 AM
  • User-501297529 posted
    That part is not inside the method. Do you see the lamba => sign in the GetArchivedDocs method. I did post the code for that method in my original post and a few other times.
    Thursday, July 30, 2020 6:38 AM
  • User-775646050 posted

    You are not understanding what an Expression bodied function is. Like I mentioned earlier, => is equivalent to this {}. Just set the breakpoint inside GetArchivedDocs and see if execution stops. I'm betting it won't. No one needs to see the GetDocuments method because the problem is outside of that. We need to see what is calling GetArchivedDocs to help.

    Thursday, July 30, 2020 6:47 AM
  • User-501297529 posted
    I get it. I will try that. So you want to see what is calling GetArchivedDocs. Will that be by having the breakpoint on that line when stepping through the code and then seeing the values in that line of code?

    I'm not sure what you mean by see if the execution stops. It will keep going but not get into the if condition for isArchived. Is that what you mean?
    Thursday, July 30, 2020 6:55 AM
  • User753101303 posted

    Hi,

    Or use "Find all references" or the "call stack" when being in GetDocuments.

    The point is that if you really see that is Archived is false then it means likely that GetCurrentDocs is called rather than GetArchivedDocs. Do you cann one or the other based maybe on a checkbox ?

    Edit: not directly related but having both isArchived and isCurrent looks weird. They won't always have opposite values? Also when processing null as another value I prefer to use not null columns (ie here to have isArchived being always either true or false).

    Thursday, July 30, 2020 7:40 AM
  • User-501297529 posted

    Hi,

    Or use "Find all references" or the "call stack" when being in GetDocuments.

    The point is that if you really see that is Archived is false then it means likely that GetCurrentDocs is called rather than GetArchivedDocs. Do you cann one or the other based maybe on a checkbox ?

    Edit: not directly related but having both isArchived and isCurrent looks weird. They won't always have opposite values? Also when processing null as another value I prefer to use not null columns (ie here to have isArchived being always either true or false).

    Is this what you need to see. This is the method that calls GetArchivedDocs. Do I need to add the parameter for isArchived true in bolded line?

     public IEnumerable<Document> GetArchivedDocs(CommitteeType committeeType)
            {
                var docEntities = _docsRepository.GetArchivedDocs(committeeType.ToCommitteeTypeId());
                var docs = _mapper.Map<IEnumerable<DocEntity>,
                    IEnumerable<Document>>(docEntities);
                return docs;
            }

    Thursday, July 30, 2020 12:44 PM
  • User1120430333 posted

    bootzilla

    DA924

    You can always pass in an int 1 or 0.

    https://www.wyzant.com/resources/answers/626873/1-false-and-0-true

    How would I do that? I read that link and it neither applies or makes sense to what I'm doing 

    This is inexperience talking here.  How would you save the boolean value to a database table column representing boolean? It's going to be an int or a binary column of 1 or 0. You can read the value and set some boolean variable to true or false or just use the numeric value of 0 = truer or 1 = false

    public IEnumerable<DocEntity> GetArchivedDocs(int? committeeTypeId) =>
    GetDocuments(0, committeeTypeId)

    public IEnumerable<PacketEntity> GetCurrentDocs(int? committeeTypeId) =>
    GetDocuments(1, null);

    if (val == 0)
    {
    // then it's true
    }
    else
    {
    // then it's 1 representing false.
    }

    This is some software developer 101 stuff here.
    Thursday, July 30, 2020 1:17 PM
  • User753101303 posted

    And then this outer GetArchivedDocs method is called from where ? In short my first move would be to make 100% sure that the test page or whatever is really calling GetArchivedDocs which ends up calling GetDocuments(true).

    For now all happens as if you are calling GetCurrentDocs instead causing GetDocuments(false) to be called. BTW what if you just try for example :

     public IEnumerable<DocEntity> GetArchivedDocs(int? committeeTypeId) =>
    GetDocuments(true, committeeTypeId)

    public IEnumerable<PacketEntity> GetCurrentDocs(int? committeeTypeId) =>
    GetDocuments(true, committeeTypeId); // HACK: pass true and see if you see now true in GetDocuments

    Now passing false to GetDocuments should be just not possible at all. If it "fixes" the problem it would show you really end up in calling GetCurrentDocs rather than GetArchivedDocs.

    If it doesn't change anything then it seems GetDocuments(false) could be called from somewhere else.

    Also could you confirm you really see that isArchived is false or do you have some unexpected behavior that just make you think it could be caused by isArchived being false ?

    • Marked as answer by Anonymous Thursday, October 7, 2021 12:00 AM
    Thursday, July 30, 2020 1:37 PM