HiveBrain v1.2.0
Get Started
← Back to all entries
patterngoMinor

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

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
rowsfilecsvfewparsingandconvertingstring

Problem

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:

Solution

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

Code Snippets

for i := 0; i < len(record[i]); i++ {
type SomeRecord struct {
    Num  int
    Name string
    Eia  float
    // etc
}

Context

StackExchange Code Review Q#24884, answer score: 2

Revisions (0)

No revisions yet.