Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Offer slices of rows and cells or other flexible iteration instead of/in addition to ForEachRow/ForEachCell #665

Closed
gbnyc26 opened this issue Jan 21, 2021 · 1 comment

Comments

@gbnyc26
Copy link

gbnyc26 commented Jan 21, 2021

The ForEachRow() and ForEachCell() iterators introduce complexity into intuitive loop control flow that is built-in to Go.

Please offer publicly accessible Rows and Cells slices (even if via a method Rows() or Cells(), so simple iterations can be implemented inline with out the overhead of an anonymous function. In your current implementation I can't easily implement something like:

for i, rows := sheet.Rows {
    if i == 0 {
       continue // skip header row
    }

   // rest of loop
}

Or, if you insist on using an iterator pattern, because perhaps you don't want to overload the client memory with slices full of data, consider making your iterator similar to the iterator in the sql package for ResultSets (see database/sql):

func (rs *Rows) Next() bool

Where I can still use a for statement to control my loop flow:

rows := sheet.Rows()

for i := 0; rows.Next(); i++ {
    if i == 0 {
        continue // skip header row
    }
    cells := rows.Cells()
    
    for j := 0; cells.Next(); j++ {
       values := cells.Values()
       log.Println(values[0])... etc etc 
       // OR
       log.Println(cells.GetValue(0))
      // OR
      if cells.GetValue(j) == cells.GetValue(j+1) {
        log.Println("found: adjacent values are the same in this row")
        break     
    }
}

In my opinion with your current iterator implementation, you are excluding the builtin control flow features that a loop provides (such as break or continue, or look ahead to the next element by using cell[i+1]) and forcing a user to figure out how to create unique control flow functions for each type element of your iteration, which, by your implementation, must be placed in a handler function away from the enclosing function (this impacts readability; anonymous functions are marginally better). As I said, I could implement what I need using your approach, but not easily. Your iterator obscures loop control, and may not be as performant as a simple for loop as you have handler functions.

@github-actions
Copy link

Stale issue message

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant