2
\$\begingroup\$

I created a simple command line application to read CSV files in a specific folder and find the same values on the specify column which exists on all files. (Here's the complete source code.)

Here's the core of the logic:

func csvReader(inputColumn, fileLocation string) {
    startTime := time.Now()

    // get all the files from the specific folder
    files, err := ioutil.ReadDir(fileLocation)
    if err != nil {
        log.Fatal(err)
    }

    noColumn := make(chan struct{}, len(files))
    results := make(chan map[string]bool, len(files))

    // reading all the files from dir concurrently
    for _, file := range files {
        wg.Add(1)
        // and read it concurrently to get the data from specific column
        go func(file os.FileInfo) {
            defer wg.Done()
            f, err := os.Open(dir + file.Name())
            if err != nil {
                log.Fatal(err)
            }
            datas, ok := readFile(inputColumn, f)

            if !ok {
                noColumn <- struct{}{}
                return
            }

            results <- datas

        }(file)

    }

    wg.Wait()

    // check if we got the column or not
    select {
    case <-noColumn:
        fmt.Printf("The column name = %v doesnt exist\n", inputColumn)
        return

    default: // do nothing and continue
    }

    close(results)
    close(noColumn)

    //receive results and determine which size datas is the smallest
    theSameValue := getSameValues(&results)

    fmt.Printf("final result = %+v\n", theSameValue)
    fmt.Printf("final result size = %+v\n", len(theSameValue))
    fmt.Printf("time consume = %+v\n", time.Since(startTime).Seconds())

}

The function is finding the files on specified fileLocation and then concurrently reading the file to get the values from the given column inputColumn, and passing the values to channel to save the results from reading the CSV files.

The channel results := make(chan []map[string]bool) is passed to a function the begin searching the values that exist on all files.

theSameValue := getSameValues(&results)

Notice that the function getSameValues() receives the pointer of results. I did this because I don't' want the results channel to be copied to the functions argument which takes resource of memory.

And here is the getSameValues() function:

// getting the same value from all the datas
func getSameValues(results *chan map[string]bool) []string {
    var datas = make([]map[string]bool, len(*results))
    minIndex := -1
    minSize := int(MaxUint >> 1)
    i := 0
    for values := range *results {
        sizeValues := len(values)
        if sizeValues < minSize && sizeValues > 0 {
            minSize = sizeValues
            minIndex = i
        }
        datas[i] = values
        i++
    }

    // getting the same value from all the datas
    var theSameValue []string
    for value, _ := range datas[minIndex] {
        isExistAll := false
        for _, data := range datas {
            if data[value] {
                isExistAll = true
            } else {
                isExistAll = false
            }
        }

        if isExistAll {
            theSameValue = append(theSameValue, value)
        }
    }

    return theSameValue
}

Hopefully this would help others and if you find something that can be improved please give some suggestion.

\$\endgroup\$
2
\$\begingroup\$

Pointers to chan

Channel variables are references to an underlying data structure. As such, in terms of resources, I don't think you gain anything at all by passing a *chan instead of the chan itself. I re-read the section on channels in Effective Go, and there's no mention about saving resources by passing around pointers, and it's not logical. So I think you can safely stop using a pointer for this and simplify the code.

Likely bug

This can be simplified:

isExistAll := false
for _, data := range datas {
    if data[value] {
        isExistAll = true
    } else {
        isExistAll = false
    }
}

You can use the value of data[value] directly:

isExistAll := false
for _, data := range datas {
    isExistAll = data[value]
}

But I don't think this code does what you intended. The value of isExistAll will be the value of the last data[value]. I think you want it to be true only of all values are true:

isExistAll := true
for _, data := range datas {
    isExistAll = isExistAll && data[value]
}

Constructing paths

Instead of concatenating strings like dir + file.Name() to create paths, it's better to use path.Join(dir, file.Name()).

\$\endgroup\$
  • \$\begingroup\$ I was thinking the same thing when using pointer to chan. Now I have the answer thanks to you. \$\endgroup\$ – Gujarat Santana Jun 4 '17 at 3:07
  • 2
    \$\begingroup\$ @GujaratSantana ah yes, I meant isExistAll := true, fixed it now, thanks! \$\endgroup\$ – janos Jun 4 '17 at 5:56

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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