Solved

Coding problem

Posted on 2014-04-22
8
222 Views
Last Modified: 2014-04-30
Hi Experts,
I have a coding below, how do I make it more simple, I know "  If Me.CPDOMVIOL = True Then" are twice,


 If IsNull(CPAgency) Then
        If Me.CPDOMVIOL = True Then
           AddrLine1 = Format(Me.CPNAME, ">")
           AddrLine2 = ""
           AddrLine3 = ""
           AddrLine4 = ""
        Else
           AddrLine1 = Format(Me.CPNAME, ">")
           AddrLine2 = Format(Me.CPADD, ">")
           AddrLine3 = Format(Me.CPCitySt, ">")
           AddrLine4 = ""
        End If
     Else
        AddrLine1 = Format(Me.AGYName, ">")
        AddrLine2 = Format(Me.AGYADDR, ">")
        AddrLine3 = Format(Me.AGYCitySt, ">")
        AddrLine4 = ""
     End If
  Else
      If IsNull(CPATTYCODE) Or CPATYLAST = "UNKNOWN" Or CPATYLAST = "UNAVAILABLE" Then
         If Me.CPDOMVIOL = True Then
           AddrLine1 = Format(Me.CPNAME, ">")
           AddrLine2 = ""
           AddrLine3 = ""
           AddrLine4 = ""
        Else
           AddrLine1 = Format(Me.CPNAME, ">")
           AddrLine2 = Format(Me.CPADD, ">")
           AddrLine3 = Format(Me.CPCitySt, ">")
           AddrLine4 = ""
        End If
      Else
         If IsNull(CPATTYName) Then
            If Not IsNull(CPATYFIRM) Then
               AddrLine1 = Format(Me.CPATYFIRM, ">")
               AddrLine2 = Format(Me.CPATTYAdd, ">")
               AddrLine3 = Format(Me.CPAttyCitySt, ">")
               AddrLine4 = ""
            End If
         Else
            If IsNull(CPATYFIRM) Then
               AddrLine1 = Format(Me.CPATTYName, ">")
               AddrLine2 = Format(Me.CPATTYAdd, ">")
               AddrLine3 = Format(Me.CPAttyCitySt, ">")
               AddrLine4 = ""
            Else
               AddrLine1 = Format(Me.CPATTYName, ">")
               AddrLine2 = Format(Me.CPATYFIRM, ">")
               AddrLine3 = Format(Me.CPATTYAdd, ">")
               AddrLine4 = Format(Me.CPAttyCitySt, ">")
            End If
         End If
        End If

I tried this, but it does not work

 If IsNull(CPAgency) Or IsNull(CPATTYCODE) Or CPATYLAST = "UNKNOWN" Or CPATYLAST = "UNAVAILABLE" Then
        If Me.CPDOMVIOL = True Then
           AddrLine1 = Format(Me.CPNAME, ">")
           AddrLine2 = ""
           AddrLine3 = ""
           AddrLine4 = ""
        Else
           AddrLine1 = Format(Me.CPNAME, ">")
           AddrLine2 = Format(Me.CPADD, ">")
           AddrLine3 = Format(Me.CPCitySt, ">")
           AddrLine4 = ""
        End If
     Else
        If Not IsNull(CPAgency) Then
           AddrLine1 = Format(Me.AGYName, ">")
           AddrLine2 = Format(Me.AGYADDR, ">")
           AddrLine3 = Format(Me.AGYCitySt, ">")
           AddrLine4 = ""
        Else
           If Not IsNull(CPATTYCODE) Then
              If IsNull(CPATTYName) Then
                  If Not IsNull(CPATYFIRM) Then
                     AddrLine1 = Format(Me.CPATYFIRM, ">")
                     AddrLine2 = Format(Me.CPATTYAdd, ">")
                     AddrLine3 = Format(Me.CPAttyCitySt, ">")
                     AddrLine4 = ""
                   End If
               Else
                   If IsNull(CPATYFIRM) Then
                      AddrLine1 = Format(Me.CPATTYName, ">")
                      AddrLine2 = Format(Me.CPATTYAdd, ">")
                      AddrLine3 = Format(Me.CPAttyCitySt, ">")
                      AddrLine4 = ""
                   Else
                      AddrLine1 = Format(Me.CPATTYName, ">")
                      AddrLine2 = Format(Me.CPATYFIRM, ">")
                      AddrLine3 = Format(Me.CPATTYAdd, ">")
                      AddrLine4 = Format(Me.CPAttyCitySt, ">")
                   End If
               End If
             End If
            End If
       End If

did I do anything wrong?

Thanks
0
Comment
Question by:urjudo
  • 3
  • 2
  • 2
  • +1
8 Comments
 
LVL 24

Expert Comment

by:SunBow
ID: 40016356
What may be best is to use non-code language to describe what you want to accomplish. Perhaps simple chart of flow or pseudo code. Also maintain count of bracketed conditions and current state, possibly through utilization of comments within code.

For example, your initial attempt also does not work as given, for two 'if-then-else-end if' are followed by an 'Else'. Two 'If' with three 'Else'. Not good.

Your simplification has redundant test for CPATYFIRM which is probably what you are seeking to correct to some test for CPATTYName. As it stands you have essentially (abbrev)

             If Not IsNull(CPATYFIRM)
                   else
              If IsNull(CPATYFIRM) Then

where the latter can only be true causing the last else to never be handled.
0
 

Author Comment

by:urjudo
ID: 40017755
I changed to this, but it only take the last Else section and the "If IsNull and If Not IsNull" those two parts the system skips.


If IsNull(CPATTYCODE) Or ATYLAST = "UNKNOWN" Or ATYLAST = "UNAVAILABLE" Then
          If IsNull(ATTYNAME) And Not IsNull(ATYFIRM) Then
             AddrLine1 = Format(Me.ATYFIRM, ">")
             AddrLine2 = "ATTORNEY FOR RESPONDENT"
             AddrLine3 = Format(Me.ATTYAdd, ">")
             AddrLine4 = Format(Me.AttyCitySt, ">")
             AddrLine5 = ""
           Else
             If Not IsNull(ATTYNAME) And IsNull(ATYFIRM) Then
                AddrLine1 = Format(Me.ATTYNAME, ">")
                AddrLine2 = "ATTORNEY FOR RESPONDENT"
                AddrLine3 = Format(Me.ATTYAdd, ">")
                AddrLine4 = Format(Me.AttyCitySt, ">")
                AddrLine5 = ""
              End If
           End If
        Else
           AddrLine1 = Format(Me.ATTYNAME, ">")
           AddrLine2 = "ATTORNEY FOR RESPONDENT"
           AddrLine3 = Format(Me.ATYFIRM, ">")
           AddrLine4 = Format(Me.ATTYAdd, ">")
           AddrLine5 = Format(Me.AttyCitySt, ">")
       End If
0
 

Author Comment

by:urjudo
ID: 40018504
I found out that the problem is in this statement,

  If IsNull(ATTYNAME) And Not IsNull(ATYFIRM) Then
             AddrLine1 = Format(Me.ATYFIRM, ">")
             AddrLine2 = "ATTORNEY FOR RESPONDENT"
             AddrLine3 = Format(Me.ATTYAdd, ">")
             AddrLine4 = Format(Me.AttyCitySt, ">")
             AddrLine5 = ""
it seems the system does not like the statement above and it went to the statement below
           AddrLine1 = Format(Me.ATTYNAME, ">")
           AddrLine2 = "ATTORNEY FOR RESPONDENT"
           AddrLine3 = Format(Me.ATYFIRM, ">")
           AddrLine4 = Format(Me.ATTYAdd, ">")
           AddrLine5 = Format(Me.AttyCitySt, ">")

I can't figure out why.
0
 
LVL 24

Expert Comment

by:SunBow
ID: 40018803
Still too much I do not know and must guess, but here are two more.

For this sequence:

If IsNull(ATTYNAME) And Not IsNull(ATYFIRM) Then
...
           Else
                 If Not IsNull(ATTYNAME) And IsNull(ATYFIRM) Then

You already tested ATTYNAME and know result of true so to simplify do not retest, leaving the latter statement as

                 If IsNull(ATYFIRM) Then

This line is not arrived at from test

          If IsNull(ATTYNAME) And Not IsNull(ATYFIRM) Then

For its 'else' is paired to by first 'if'

  If IsNull(CPATTYCODE) Or ATYLAST = "UNKNOWN" Or ATYLAST = "UNAVAILABLE" Then

If I interpret your remark about skipping, perhaps a revisit to data handing for something like CPATTYCODE.

For testing it is sometimes better to use small sample set than the main file, one that can be easily edited to switch a cell here and there back and forth between values, especially when one can have situation such as "" and " " present. Just a few rows can be sufficient, even selectively copied from database.

As you started with other compares such as for CPDOMVIOL, you may have additional code remaining that can impact result, Before using the compounded statements you may benefit temporarily by increasing number of embedded ifs.

Also consider inventing some temporary results to assess result of compares, such as for AddrLine5, make it obvious such as "Temp Test" or "AddrLine5" and temporarily remove later references to AddrLine5. For example

   If IsNull(CPATTYCODE) Or ATYLAST = "UNKNOWN" Or ATYLAST = "UNAVAILABLE" Then
          AddrLine5 = "__AddrLine5__"
          If IsNull(ATTYNAME) And Not IsNull(ATYFIRM) Then

To validate the result achieved by first if. If that new result is unattained, then debug by removing one of the three compares for that if statement. Or try them one at a time.
0
Backup Your Microsoft Windows Server®

Backup all your Microsoft Windows Server – on-premises, in remote locations, in private and hybrid clouds. Your entire Windows Server will be backed up in one easy step with patented, block-level disk imaging. We achieve RTOs (recovery time objectives) as low as 15 seconds.

 
LVL 26

Accepted Solution

by:
Nick67 earned 400 total points
ID: 40021660
You don't have a lot of comments in there, so I can't tell exactly what you are trying to do...
Which is the very definition of poor coding, no offense.

What you really need is to discover the most wonderful coding structure EVER!
Select Case True
Why is it wonderful?
Because you can test for ANY condition that will return TRUE or FALSE,and after you hit the first true condition, the block of events in that condition execute and the code exits.  It let's you escape the confusion of nested IFs, and Elseifs

So, although this is probably not the correct logic yet

Select Case True
    Case IsNull(CPAgency) =False
        AddrLine1 = Format(Me.AGYName, ">")
        AddrLine2 = Format(Me.AGYADDR, ">")
        AddrLine3 = Format(Me.AGYCitySt, ">")
        AddrLine4 = ""
    Case IsNull(CPAgency) AND Me.CPDOMVIOL = True
           AddrLine1 = Format(Me.CPNAME, ">")
           AddrLine2 = ""
           AddrLine3 = ""
           AddrLine4 = ""
    Case IsNull(CPAgency) AND Me.CPDOMVIOL = False
           AddrLine1 = Format(Me.CPNAME, ">")
           AddrLine2 = Format(Me.CPADD, ">")
           AddrLine3 = Format(Me.CPCitySt, ">")
           AddrLine4 = ""
    Case IsNull(CPATTYCODE) Or CPATYLAST = "UNKNOWN" Or CPATYLAST  "UNAVAILABLE" and Me.CPDOMVIOL = True
           AddrLine1 = Format(Me.CPNAME, ">")
           AddrLine2 = ""
           AddrLine3 = ""
           AddrLine4 = ""
    Case IsNull(CPATTYCODE) Or CPATYLAST = "UNKNOWN" Or CPATYLAST  "UNAVAILABLE" and Me.CPDOMVIOL = False
           AddrLine1 = Format(Me.CPNAME, ">")
           AddrLine2 = Format(Me.CPADD, ">")
           AddrLine3 = Format(Me.CPCitySt, ">")
           AddrLine4 = ""

'And so on

End Select

Open in new window


This is easy code to create.  You must define conditions that will be true or false, and order them so that, if one is a more specific case than the other, the first thing you WANT to have happen if two or more conditions could be true is higher in the structure.  Only ONE case will occur.

It's also easy to debug.  Put a break point on each Case statement.  Your code will walk down in order until it hits the first true statement, and then execute that block and exit.

Once you get the hang of it, you'll never nest IFs again!

You need to have the most specific case (therefore the most likely to be false) first
That's likely
Case IsNull(CPAgency)=false and  IsNull(CPATYFIRM)=false and IsNull(CPATTYName)=false
and the most general case last.  That's likely
Case IsNull(CPAgency) and Me.CPDOMVIOL = True

in the block I have above, nothing past the third case would ever occur, because a true condition will always occur in the first three cases as defined there.  Order is everything

ANY -- and I mean any -- Boolean condition you can test works for this.  It's beautiful!
0
 
LVL 34

Assisted Solution

by:PatHartman
PatHartman earned 100 total points
ID: 40023702
Good answer Nick - BUT

Case (IsNull(CPATTYCODE) Or  Or CPATYLAST = "UNKNOWN" Or CPATYLAST  = "UNAVAILABLE") and Me.CPDOMVIOL = True

The last two expressions each contain two errors.
1. there is a missing = sign
2. because of the combination of OR and AND relational operators, it will almost certainly require parentheses to be evaluated correctly.

As written, (once the = is fixed) the statement will evaluate AS
Case IsNull(CPATTYCODE) Or  Or CPATYLAST = "UNKNOWN" Or (CPATYLAST  = "UNAVAILABLE" and Me.CPDOMVIOL = True)
Which is probably incorrect.  So I moved the parentheses to enclose the "OR" operations into a single expression.
0
 
LVL 26

Expert Comment

by:Nick67
ID: 40023954
@PatHartman
Indeed the code I posted is not production-worthy, but rather a sample of what the technique would look like @urjudo's own terms.

Building such code has three components
1. Defining expressions that will evaluate to true or false
2. Ordering the CASEs so that, if more than one could evaluate to true, in some circumstances, but not in others, that the results you want to have happen in such cases occurs.
3. Coding the results and testing.

The original coded block does not actually appear to be valid compilable code.  We've got an ELSE without IF problem.

But then, that's why I don't use nested ifs, if Select Case can do the job :)

@urjudo must first define all the conditions without the nesting, and then order them.
The last bit of his code will become the first part of a Select Case True structure.
0
 

Author Closing Comment

by:urjudo
ID: 40032561
Thanks Nick67 's explained and show me another way to do a better coding.
0

Featured Post

U.S. Department of Agriculture and Acronis Access

With the new era of mobile computing, smartphones and tablets, wireless communications and cloud services, the USDA sought to take advantage of a mobilized workforce and the blurring lines between personal and corporate computing resources.

Question has a verified solution.

If you are experiencing a similar issue, please ask a related question

This article is a continuation or rather an extension from Cascading Combos (http://www.experts-exchange.com/A_5949.html) and builds on examples developed in detail there. It should be understandable alone, but I recommend reading the previous artic…
Most if not all databases provide tools to filter data; even simple mail-merge programs might offer basic filtering capabilities. This is so important that, although Access has many built-in features to help the user in this task, developers often n…
As developers, we are not limited to the functions provided by the VBA language. In addition, we can call the functions that are part of the Windows operating system. These functions are part of the Windows API (Application Programming Interface). U…
In Microsoft Access, learn how to “cascade” or have the displayed data of one combo control depend upon what’s entered in another. Base the dependent combo on a query for its row source: Add a reference to the first combo on the form as criteria i…

914 members asked questions and received personalized solutions in the past 7 days.

Join the community of 500,000 technology professionals and ask your questions.

Join & Ask a Question

Need Help in Real-Time?

Connect with top rated Experts

19 Experts available now in Live!

Get 1:1 Help Now