String converting and parsing a few rows of a .csv file

Question

There are not many available resources for refactoring go code. I'm a novice gopher who would appreciate any feedback on a small program which reads a .csv file, string converts and parses a few rows in order to perform a sub method.

package main
import (
 "encoding/csv"
 "fmt"
 "io"
 "log"
 "os"
 "strconv"
)
const file = "csvdata/csvtest.csv"
// what should stay in main()? What should be refactored?
func main() {
 f, err := os.Open(file)
 if err != nil {
 log.Fatalf("Error reading all lines: %v", err)
 }
 defer f.Close()
 reader := csv.NewReader(f)
 reader.Comma = ';'
 for {
 record, err := reader.Read()
 if err == io.EOF {
 break
 } else if err != nil {
 log.Print(err)
 os.Exit(-1)
 }
 // need to refactor variables, could I use a type struct?
 var num string = (record[2])
 var name string = (record[3])
 var eia string = (record[5])
 var cia string = (record[6])
 var inc_percent = (record[7])
 var estIncTxn string = (record[8])
 var inc_diff float64
 // I would like to turn this chunk into it's own function
 for i := 0; i < len(record[i]); i++ {
 estInc, err := strconv.ParseFloat(eia, 64)
 actInc, err := strconv.ParseFloat(cia, 64)
 inc_diff = (actInc - estInc)
 if err == nil {
 //How could I turn the following into a template?
 fmt.Println("============================================================\n")
 fmt.Printf("Account: %+s - %+s exceeded the IncAmt by %+v same as $%+v\n", num, name, inc_percent, inc_diff)
 fmt.Printf("over the monthly incoming amount of $%+v. Currently, the declared\n", actInc)
 fmt.Printf("profile is established at $%+v with an expectancy of (%+v).\n", estInc, estIncTxn)
 } else {
 log.Fatalf("Error converting strings: +v", err)
 }
 }
 fmt.Println()
 }
}

Here is a single line of the csv file.

30;4;102033657;COCACOLA;53764;15000.00;73010.58;387%;4;30;650%;15000.00;89558.96;497%;3;5;66%;DDA

Sample of the output:

==============================================================================

Account: 102033657 - COCACOLA exceeded the IncAmt by 387% same as 58010ドル.58 over the monthly incoming amount of 73010ドル.58. Currently, the declared profile is established at 15000ドル with an expectancy of (4).

==============================================================================

Eventually I would be capturing the outgoing amount as well for each record (just to give everyone an idea of where I'm going with this). I'm assuming a template would be the best way to approach this situation? How could this be refactored?

Answer 1

The first thing I would do is tidy up the code and fix obvious bugs. For example,

package main
import (
 "encoding/csv"
 "fmt"
 "io"
 "log"
 "os"
 "strconv"
)
const file = "csvdata/csvtest.csv"
type Amounts struct {
 EstimatedAmt float64
 ActualAmt float64
 Percent string
 EstimatedTxn string
}
type Account struct {
 Num string
 Name string
 In *Amounts
 Out *Amounts
}
const reportSeparator = "============================================================\n"
func printAccountMonth(a *Account) error {
 fmt.Printf(
 reportSeparator,
 )
 fmt.Printf(
 "Account: %+s - %+s exceeded the incoming amount by %+v, the same as $%+v\n",
 a.Num, a.Name, a.In.Percent, a.In.ActualAmt-a.In.EstimatedAmt,
 )
 fmt.Printf(
 "over the monthly incoming amount of $%+v. Currently, the declared\n",
 a.In.ActualAmt,
 )
 fmt.Printf(
 "profile is established at $%+v with an expectancy of (%+v).\n",
 a.In.EstimatedAmt, a.In.EstimatedTxn,
 )
 return nil
}
func readAmounts(r []string) (a *Amounts, err error) {
 a = new(Amounts)
 est := r[0]
 a.EstimatedAmt, err = strconv.ParseFloat(est, 64)
 if err != nil {
 return nil, fmt.Errorf("Error converting string: +v", err)
 }
 act := r[1]
 a.ActualAmt, err = strconv.ParseFloat(act, 64)
 if err != nil {
 return nil, fmt.Errorf("Error converting string: +v", err)
 }
 a.Percent = r[2]
 a.EstimatedTxn = r[3]
 return a, nil
}
func accountMonth(record []string) error {
 var err error
 var a Account
 a.Num = record[2]
 a.Name = record[3]
 a.In, err = readAmounts(record[5 : 5+6])
 if err != nil {
 return err
 }
 a.Out, err = readAmounts(record[11 : 11+6])
 if err != nil {
 return err
 }
 err = printAccountMonth(&a)
 return err
}
func main() {
 f, err := os.Open(file)
 if err != nil {
 log.Fatalf("Error opening file: %v", err)
 }
 defer f.Close()
 rdr := csv.NewReader(f)
 rdr.Comma = ';'
 for {
 record, err := rdr.Read()
 if err != nil {
 if err == io.EOF {
 break
 }
 log.Fatal(err)
 }
 err = accountMonth(record)
 if err != nil {
 log.Fatal(err)
 }
 fmt.Printf(reportSeparator)
 }
}

Input:

Record:
[30 4 102033657 COCACOLA 53764 15000.00 73010.58 387% 4 30 650% 15000.00 89558.96 497% 3 5 66% DDA]
0 30
1 4
2 102033657
3 COCACOLA
4 53764
5 15000.00
6 73010.58
7 387%
8 4
9 30
10 650%
11 15000.00
12 89558.96
13 497%
14 3
15 5
16 66%
17 DDA

Output:

============================================================
Account: 102033657 - COCACOLA exceeded the incoming amount by 387%, the same as 58010ドル.58
over the monthly incoming amount of 73010ドル.58. Currently, the declared
profile is established at 15000ドル with an expectancy of (4).
============================================================

Obviously it needs a lot more work, but it's a start.

• Alex: this is definitely more idiomatic go code. I would upvote this answer right now but I have not been granted access.

Answer 2

 for i := 0; i < len(record[i]); i++ {

... I don't think that does what you intended. It seems like just luck from how the data is that it works, or did I miss something? :-) I think you can just take the for {} loop out altogether.

I'd move the ParseFloat() calls up to where you assign the record[] values to named variables. Be sure to check the err return from both calls.

Your idea of using a struct is also good.

type SomeRecord struct {
 Num int
 Name string
 Eia float
 // etc
}

The for moving some of the code to a function: Have the function take a SomeRecord as a parameter and do the output.

I don't know if using a template is necessary for such a short string, but if you want to then outside of the loop read the template file with text/template, specifically template.ParseFiles and then execute the template when appropriate with the record data or a new data structure if it needs data not in the record. If this is all the program does it might make most sense to have the SomeRecord struct have the data that the template needs (so include IncDiff).

• Alex: Thanks for your observations. Well in this case the for loop is superfluous because I only listed one record entry as a csv sample. However, my actual file contains over several thousand records.
• Alex: Actually, what I would like to do is create individual markdown files of the output for each record. I will be adding more implementation to the current template. What packages would you recommend I look at for this task?
• Ask Bjørn Hansen: I meant the second loop (for i := 0; i < len(record[i]); i++ { ... that doesn't seem to do anything (useful/correctly). For Markdown have a look at blackfriday: github.com/russross/blackfriday