Jump to content

Help with a few functions and better coding.


xsnow
 Share

Recommended Posts

So i made a post earlier, but here is where I am. I want to track my account balance, as of right now I just have it doing deposits and withdrawls, no storing of names of each yet.

#include <String.au3>
#include <array.au3>
#include <file.au3>

Global $deposit = "C:\Documents and Settings\Parker\Desktop\BA\deposits.txt"
Global $withdrawal = "C:\Documents and Settings\Parker\Desktop\BA\withdrawals.txt"

 dim  $depositamount, $withdrawalamount, $balance, $filedeposit, $filedeposit, $line, $flag, $adddeposit, $addwithdrawl, $totaldeposits = 0, $totalwithdrawals = 0, $close, $update
 $filedeposit = FileOpen($deposit, 1)
 $filewithdrawal = FileOpen($withdrawal, 1)
 
Balance()
 
 GUICreate("Checkbook Finance",200,250)
 $adddeposit=GUICtrlCreateButton ("Deposit", 65,10,70,25)
 $addwithdrawl=GUICtrlCreateButton ("Withdrawal", 65,45,70,25)
 $update=GUICtrlCreateButton ("Update Balance", 65,80,70,25)
 $close=GUICtrlCreateButton ("Close", 65,135,70,25)
 GUICtrlCreateLabel ("Account Balance : $"&$balance, 65,165,70,75)
 
 GUISetState ()
 $msg = 0
 While $msg <> $GUI_EVENT_CLOSE
    $msg = GUIGetMsg()

    Select
         case $msg = $adddeposit
           Deposit()
         case $msg = $addwithdrawl
           Withdrawal()
         case $msg = $update
            Balance()
            GUICtrlCreateLabel ("Account Balance : $"&$balance, 65,165,70,75)
         case $msg = $close
            Exit
    EndSelect
 Wend
 
 Func Balance()
 $balance = 0
 $totaldeposits = 0
 $totalwithdrawals = 0
 $line = 0
      If Not _FileReadToArray($deposit,$line) Then
    MsgBox(4096,"Error", " Error reading log to Array    error:" & @error)
    Exit
 EndIf
 For $i = 1 to $line[0]
    $totaldeposits = $line[$i] + $totaldeposits
Next

 If Not _FileReadToArray($withdrawal,$line) Then
    MsgBox(4096,"Error", " Error reading log to Array    error:" & @error)
    Exit
 EndIf
 For $i = 1 to $line[0]
    $totalwithdrawals = $line[$i] + $totalwithdrawals
 Next
 $balance = $totaldeposits - $totalwithdrawals
 

 
 EndFunc
 
 EndFunc
 
 Func Deposit()
    $depositamount = Inputbox("Deposit", "How much?")
    If @Error = 1 Then
        MsgBox(4096, "Deposit Canceled", "No Deposit Was Entered")
    Else
        If @Error = 0 Then
            $flag = Msgbox (4, "Is the information correct?", "Deposit of $"&$depositamount)
            If $flag = 6 Then
                FileWrite($deposit, $depositamount & @CRLF)
            EndIf
        EndIf
    EndIf
 EndFunc
 
 Func Withdrawal()
    $withdrawalamount = Inputbox("Withdrawal", "How much?")
    If @Error = 1 Then
        MsgBox(4096, "Withdrawal Canceled", "No Withdrawal Was Entered")
    Else
        If @Error = 0 Then
        $flag = Msgbox (4, "Is the information correct?", "Withdrawal of $"&$withdrawalamount)
            If $flag = 6 Then
                FileWrite($withdrawal, $withdrawalamount & @CRLF)
            EndIf
        EndIf   
    EndIf
 EndFunc

What coding looks bad? And should be written more efficiently?

Edited by xsnow
Link to comment
Share on other sites

The coding looks good, only thing I would recommend is renaming some of the variables. It can get a bit confusing later on when you have many variables all named similarly. One thing some coders do is add a lowercase letter at the beginning of your variable name to denote its type. IE: $iVal would let you know that the variable is an interger, $dVal would tell you it's a double, $sVal a string, etc. Also, when declaring variables at the top, it helps to group similar virables together. Here's what a variable renaming would look like on your script:

#include <String.au3>
#include <array.au3>
#include <file.au3>

Global $fDeposit = "C:\Documents and Settings\Parker\Desktop\BA\deposits.txt"
Global $fWithdrawal = "C:\Documents and Settings\Parker\Desktop\BA\withdrawals.txt"

Dim $dDeposit, $dWithdrawal, $dBalance
Dim $hDeposit, $hWithdrawal
Dim $line, $flag
Dim $gGui, $bDeposit, $bWithdrawal, $bUpdate, $bClose
Dim $dTotalDeposits = 0, $dTotalWithdrawals = 0

$hDeposit = FileOpen($fDeposit, 1)
$hWithdrawal = FileOpen($fWithdrawal, 1)

Balance()

$gGui = GUICreate("Checkbook Finance",200,250)
$bDeposit = GUICtrlCreateButton ("Deposit", 65,10,70,25)
$bWithdrawal = GUICtrlCreateButton ("Withdrawal", 65,45,70,25)
$bUpdate = GUICtrlCreateButton ("Update Balance", 65,80,70,25)
$bClose = GUICtrlCreateButton ("Close", 65,135,70,25)
GUICtrlCreateLabel ("Account Balance : $"&$dBalance, 65,165,70,75)

GUISetState ()
$msg = 0
While $msg <> $GUI_EVENT_CLOSE
    $msg = GUIGetMsg()
    Select
         case $msg = $bDeposit
            Deposit()
         case $msg = $bWithdrawal
            Withdrawal()
         case $msg = $bUpdate
            Balance()
            GUICtrlCreateLabel ("Account Balance : $"&$dBalance, 65,165,70,75)
         case $msg = $bClose
            Exit
    EndSelect
Wend

Func Balance()
    
    $dBalance = 0
    $dTotalDeposits = 0
    $dTotalWithdrawals = 0
    $line = 0
    
    If Not _FileReadToArray($fDeposit,$line) Then
        MsgBox(4096,"Error", " Error reading log to Array     error:" & @error)
        Exit
    EndIf
    For $i = 1 to $line[0]
        $dTotalDeposits = $line[$i] + $dTotalDeposits
    Next

    If Not _FileReadToArray($fWithdrawal,$line) Then
        MsgBox(4096,"Error", " Error reading log to Array     error:" & @error)
        Exit
    EndIf
    For $i = 1 to $line[0]
        $dTotalWithdrawals = $line[$i] + $dTotalWithdrawals
    Next
    $dBalance = $dTotalDeposits - $dTotalWithdrawals
EndFunc

EndFunc

Func Deposit()
    $dDeposit = Inputbox("Deposit", "How much?")
    If @Error = 1 Then
        MsgBox(4096, "Deposit Canceled", "No Deposit Was Entered")
    Else
        If @Error = 0 Then
            $flag = Msgbox (4, "Is the information correct?", "Deposit of $"&$dDeposit)
            If $flag = 6 Then
                FileWrite($fDeposit, $dDeposit & @CRLF)
            EndIf
        EndIf
    EndIf
EndFunc

Func Withdrawal()
    $dWithdrawal = Inputbox("Withdrawal", "How much?")
    If @Error = 1 Then
        MsgBox(4096, "Withdrawal Canceled", "No Withdrawal Was Entered")
    Else
        If @Error = 0 Then
        $flag = Msgbox (4, "Is the information correct?", "Withdrawal of $"&$dWithdrawal)
            If $flag = 6 Then
                FileWrite($fWithdrawal, $dWithdrawal & @CRLF)
            EndIf
        EndIf    
    EndIf
EndFunc
[font="Impact"] I always thought dogs laid eggs, and I learned something today. [/font]
Link to comment
Share on other sites

Global $deposit = "C:\Documents and Settings\Parker\Desktop\BA\deposits.txt"
Global $withdrawal = "C:\Documents and Settings\Parker\Desktop\BA\withdrawals.txt"

is better written as the following, because you only have to change your path once if you move your folder off the desktop

similarly, if you want to add another file reference, you don't have to type the absolute path.

To access this you would just do : $path & $deposit

Global $path = "C:\Documents and Settings\Parker\Desktop\BA\"
Global $deposit = "deposits.txt"
Global $withdrawal = "withdrawals.txt"

$totaldeposits = $line[$i] + $totaldeposits

can be re-written more succinctly as

$totaldeposits += $line[$i]

Func Deposit()
    $depositamount = Inputbox("Deposit", "How much?")
    If @Error = 1 Then
        MsgBox(4096, "Deposit Canceled", "No Deposit Was Entered")
    Else
        If @Error = 0 Then
            $flag = Msgbox (4, "Is the information correct?", "Deposit of $"&$depositamount)
            If $flag = 6 Then
                FileWrite($deposit, $depositamount & @CRLF)
            EndIf
        EndIf
    EndIf
EndFunc

might be better rewritten as the following, although its slightly different, look at the comment

Func Deposit()
    $depositamount = Inputbox("Deposit", "How much?")
    If @Error = 0 Then
        $flag = Msgbox (4, "Is the information correct?", "Deposit of $"&$depositamount)
        If $flag = 6 Then
            FileWrite($deposit, $depositamount & @CRLF)
        EndIf
    Else; if @error = 1 Then
        MsgBox(4096, "Deposit Canceled", "No Deposit Was Entered")
    EndIf
EndFunc

same goes for the other function

Where they are now, you can probably turn your deposit withdrawal function into a single transaction function which takes different parameters, and then call your error messages from an array, or even from a different file. depends on how much you want to expand the program

make sure you are consistent with indentation

you have two EndFunc after the Deposit function

I'm not sure why you declare all your variables at the top, although there's nothing wrong with it. As for me, I try to define things within the scope which they are used, such as depositamount, which i would define within deposit.

You should always comment your code, and you shouldn't ever worry about optimization too early on.

I recommend making a comment at the top of each function, as well as inline comments wherever you do some process which isn't immediately obvious. Also, I'd place some linebreaks to separate some of your code to make it more readable

Oops, it double posted half way through, sorry

Edited by maxlarue
Link to comment
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now
 Share

  • Recently Browsing   0 members

    • No registered users viewing this page.
×
×
  • Create New...