refactoring golang code

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 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 would a Professional approach this code to refactor it? I'm really looking on insight.

peterSO · Accepted Answer · 2013-04-09 20:43:20Z

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.

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

Ask Bjørn Hansen · Answer 2 · 2013-04-09 05:06:32Z

    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).

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.
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?
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

asked	1 month ago
viewed	62 times
active	1 month ago

refactoring golang code

2 Answers

Your Answer

Not the answer you're looking for? Browse other questions tagged go csv or ask your own question.

Community Bulletin

refactoring golang code

2 Answers

Your Answer

Sign up or log in

Post as a guest

Not the answer you're looking for? Browse other questions tagged go csv or ask your own question.

Community Bulletin

Related