MS Access VBA Code Audit

Hankwembo Christopher,FCCA,FZICA,CIA,MAAT,B.A.Sc
Hankwembo Christopher,FCCA,FZICA,CIA,MAAT,B.A.Sc used Ask the Experts™
on
Dear All;

Happy new year 2020!

I just want to be corrected on the Ms Access/VBA code below if I have gotten it right or there is an error somewhere , the quality may not be as per your expectation , but due to the impending scheduled quality check  by the taxman on their tax gadget (Rs 232 serial port) coming on Monday next week , I have decided to use this platform to audit the code below:
    Dim json As String
    Dim intPortID As Integer ' Ex. 1, 2, 3, 4 for COM1 - COM4
    Dim lngStatus As Long
    Dim strError  As String
    Dim strData   As String
    Dim lngSize As Long
    intPortID = 2
    ' Initialize Communications
    lngStatus = CommOpen(intPortID, "COM" & CStr(intPortID), _
        "baud=115200 parity=N data=8 stop=1")
    
    If lngStatus <> 0 Then
    ' Handle error.
        lngStatus = CommGetError(strError)
    MsgBox "COM Error: " & strError
    End If
    

    ' Set modem control lines.
    lngStatus = CommSetLine(intPortID, LINE_RTS, True)
    lngStatus = CommSetLine(intPortID, LINE_DTR, True)

    ' Write data to serial port.
    strData = JsonConverter.ConvertToJson(transaction, Whitespace:=3) [b]‘the intention here is to put ‘the Json data into the data string called    strData[/b]
    lngSize = Len(strData)
    lngStatus = CommWrite(intPortID, strData)
    If lngStatus <> lngSize Then
    ' Handle error.
    End If
Exit_CmdConertJson_Click:
Exit Sub
Err_Handler:
Resume Exit_CmdConertJson_Click

' Read maximum of 14400 bytes from serial port.

Dim JSONS As Object

    lngStatus = CommRead(intPortID, strData, 14400)

Set rs = db.OpenRecordset("tblEfdReceipts") [b]‘here I’m opening the table as a record source so that I ‘put the data received as per below deserialization[/b]
    If lngStatus > 0 Then
    ElseIf lngStatus < 0 Then
        ' Handle error.
        On Error Resume Next
    End If
        ' Process data.
  Set JSONS = JsonConverter.ParseJson(strData)[b]’ Here I have assigned the Json to put data into object ‘called JSONS[/b]
    Z = 2
  For Each item In JSONS [b]‘Here Im now unpacking received json data into the table called  ‘("tblEfdReceipts")[/b]
           With rs
         
            .AddNew
            rs![TPIN] = item("TPIN")
            rs![TaxpayerName] = item("TaxpayerName")
            rs![Address] = item("Address")
            rs![ESDTime] = item("ESDTime")
            rs![TerminalID] = item("TerminalID")
            rs![InvoiceCode] = item("InvoiceCode")
            rs![InvoiceNumber] = item("InvoiceCode")
            rs![FiscalCode] = item("FiscalCode")
            rs![TalkTime] = item("TalkTime")
            rs![Operator] = item("Operator")
            rs![Taxlabel] = item("TaxItems")("TaxLabel")
            rs![CategoryName] = item("TaxItems")("CategoryName")
            rs![Rate] = item("TaxItems")("Rate")
            rs![TaxAmount] = item("TaxItems")("TaxAmount")
            rs![VerificationUrl] = item("TaxItems")("VerificationUrl")
            rs![INVID] = Me.InvoiceID
            rs.Update
        End With
        Z = Z + 1
    Next
      
      rs.Close
      Set rs = Nothing
      Set db = Nothing
      Set JSONS = Nothing
    
    ' Reset modem control lines.
    lngStatus = CommSetLine(intPortID, LINE_RTS, False)
    lngStatus = CommSetLine(intPortID, LINE_DTR, False)

    ' Close communications.
    Call CommClose(intPortID)

Open in new window

Comment
Watch Question

Do more with

Expert Office
EXPERT OFFICE® is a registered trademark of EXPERTS EXCHANGE®
Infotrakker Software
Most Valuable Expert 2012
Top Expert 2014
Commented:
If the code works, then you don't have any errors.

Most audits of this nature have to do with security, and it's tough to determine if you're handling that correctly. In many cases, data you're streaming over the internet should be encrypted, and it's hard to say if you're doing that since we don't actually see the code you're using to do that (looks like it's in a function named CommWrite, or perhaps another).
ste5anSenior Developer

Commented:
As I already wrote in this thread, especially these points are normally a show stopper:

- It uses hard coded values, which are clearly configuration.
The COM port configuration settings (parity, speed, stop bit) should be stored in configuration table. Any other setting requires a new audit, cause the code must be changed.
 
- No data validation.
The data read from the device must be validated before parsing and storing it.

- [..] Error handling and logging is not as granular as it could and (imho) should be.
There is no error handling and logging.
Very correct I'm using the commwrite

I was just concerned on the assignments of the following whether they are correctly done:

 strData = JsonConverter.ConvertToJson(transaction, Whitespace:=3) [b]‘the intention here is to put ‘the Json data into the data string called    strData[/b]

Open in new window



Set rs = db.OpenRecordset("tblEfdReceipts") [b]‘here I’m opening the table as a record source so that I ‘put the data received as per below deserialization[/b]

Open in new window



For Each item In JSONS ‘Here Im now unpacking received json data into the table called  ‘("tblEfdReceipts")
Scott McDaniel (EE MVE )Infotrakker Software
Most Valuable Expert 2012
Top Expert 2014
Commented:
Not sure what you mean by "correctly done". If the calls to ConvertToJson and OpenRecordset work, then they're done correctly. If you're storing them in your tables correctly, then they're done correctly.

If you're asking if your database is properly normalized, or if your application is correctly done, then the answer is we don't really know, since we don't know the full scope of your project. At first glance it seems to be stored correctly, and if it actually gets to the table, then I'd have to say it's done correctly.

One point to remember is that any data you receive from an outside source should be stored in a "staging" table, which is then used to validate your data before it gets into your live data tables. If you're using "tblEfdReceipts" as that staging table, then I'd say you're handling that correctly. If tblEfdReceipts is your LIVE data table, then I'd say you're asking for problems - there appears to be no data validation prior to inserting into that table.

Do more with

Expert Office
Submit tech questions to Ask the Experts™ at any time to receive solutions, advice, and new ideas from leading industry professionals.

Start 7-Day Free Trial