Skip to content

Commit 0ec9a3e

Browse files
authored
Change the Transform coding pattern from Next() -> Read() to just Read() (#110)
Found this `Next()->Read()` pattern pretty silly and unnecessary. Also current `Next()` has a useless `for {}` wrapping loop. Probably from long time ago, but it's not needed.
1 parent b9cbdad commit 0ec9a3e

File tree

11 files changed

+98
-79
lines changed

11 files changed

+98
-79
lines changed

README.md

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -80,10 +80,12 @@ Take a detailed look at samples here:
8080
if err != nil { ... }
8181
transform, err := schema.NewTransform("input-name", strings.NewReader("..."), &transformctx.Ctx{})
8282
if err != nil { ... }
83-
if !transform.Next() { ... }
84-
b, err := transform.Read()
85-
if err != nil { ... }
86-
fmt.Println(string(b))
83+
for {
84+
b, err := transform.Read()
85+
if err == io.EOF { break }
86+
if err != nil { ... }
87+
fmt.Println(string(b))
88+
}
8789
```
8890
- Output:
8991
```
@@ -115,7 +117,7 @@ Take a detailed look at samples here:
115117
## Requirements
116118
- Golang 1.14
117119
118-
## Recent Feature Additions
120+
## Recent Major Feature Additions/Changes
119121
- Added EDI file format support in omniv2 handler.
120122
- Major restructure/refactoring
121123
- Upgrade omni schema version to `omni.2.1` due a number of incompatible schema changes:

cli/cmd/serverCmd.go

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package cmd
33
import (
44
"encoding/json"
55
"fmt"
6+
"io"
67
"io/ioutil"
78
"log"
89
"net/http"
@@ -136,8 +137,11 @@ func httpPostTransform(w http.ResponseWriter, r *http.Request) {
136137
return
137138
}
138139
var records []string
139-
for t.Next() {
140+
for {
140141
b, err := t.Read()
142+
if err == io.EOF {
143+
break
144+
}
141145
if err != nil {
142146
writeBadRequest(w, fmt.Sprintf("bad request: transform failed. err: %s", err))
143147
return

cli/cmd/transformCmd.go

Lines changed: 23 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -77,30 +77,36 @@ func doTransform() error {
7777
return err
7878
}
7979

80-
fmt.Println("[")
81-
doOne := func() error {
80+
doOne := func() (string, error) {
8281
b, err := transform.Read()
8382
if err != nil {
84-
return err
83+
return "", err
8584
}
86-
fmt.Print(
87-
strings.Join(
88-
strs.NoErrMapSlice(
89-
strings.Split(jsons.BPJ(string(b)), "\n"),
90-
func(s string) string { return "\t" + s }),
91-
"\n"))
85+
return strings.Join(
86+
strs.NoErrMapSlice(
87+
strings.Split(jsons.BPJ(string(b)), "\n"),
88+
func(s string) string { return "\t" + s }),
89+
"\n"), nil
90+
}
91+
92+
record, err := doOne()
93+
if err == io.EOF {
94+
fmt.Println("[]")
9295
return nil
9396
}
94-
if transform.Next() {
95-
if err := doOne(); err != nil {
96-
return err
97+
if err != nil {
98+
return err
99+
}
100+
fmt.Printf("[\n%s", record)
101+
for {
102+
record, err = doOne()
103+
if err == io.EOF {
104+
break
97105
}
98-
for transform.Next() {
99-
fmt.Print(",\n")
100-
if err := doOne(); err != nil {
101-
return err
102-
}
106+
if err != nil {
107+
return err
103108
}
109+
fmt.Printf(",\n%s", record)
104110
}
105111
fmt.Println("\n]")
106112
return nil

extensions/omniv21/samples/csv/csv_test.go

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package csv
22

33
import (
44
"bytes"
5+
"io"
56
"io/ioutil"
67
"testing"
78

@@ -48,8 +49,11 @@ func Benchmark1_Weather_Data_CSV(b *testing.B) {
4849
if err != nil {
4950
b.FailNow()
5051
}
51-
for transform.Next() {
52+
for {
5253
_, err := transform.Read()
54+
if err == io.EOF {
55+
break
56+
}
5357
if err != nil {
5458
b.FailNow()
5559
}

extensions/omniv21/samples/customfileformats/jsonlog/sample_test.go

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package jsonlog
22

33
import (
4+
"io"
45
"os"
56
"path/filepath"
67
"strings"
@@ -76,8 +77,11 @@ func TestSample(t *testing.T) {
7677
assert.NoError(t, err)
7778

7879
var records []string
79-
for transform.Next() {
80+
for {
8081
recordBytes, err := transform.Read()
82+
if err == io.EOF {
83+
break
84+
}
8185
assert.NoError(t, err)
8286
records = append(records, string(recordBytes))
8387
}

extensions/omniv21/samples/customparse/sample_test.go

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package customparse
22

33
import (
4+
"io"
45
"os"
56
"path/filepath"
67
"strconv"
@@ -45,12 +46,15 @@ func TestSample(t *testing.T) {
4546
},
4647
})
4748
assert.NoError(t, err)
48-
tfm, err := schema.NewTransform(inputFileBaseName, inputFileReader, &transformctx.Ctx{})
49+
transform, err := schema.NewTransform(inputFileBaseName, inputFileReader, &transformctx.Ctx{})
4950
assert.NoError(t, err)
5051

5152
var records []string
52-
for tfm.Next() {
53-
recordBytes, err := tfm.Read()
53+
for {
54+
recordBytes, err := transform.Read()
55+
if err == io.EOF {
56+
break
57+
}
5458
assert.NoError(t, err)
5559
records = append(records, string(recordBytes))
5660
}

extensions/omniv21/samples/edi/edi_test.go

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package edi
22

33
import (
44
"bytes"
5+
"io"
56
"io/ioutil"
67
"testing"
78

@@ -45,8 +46,11 @@ func Benchmark1_CanadaPost_EDI_214(b *testing.B) {
4546
if err != nil {
4647
b.FailNow()
4748
}
48-
for transform.Next() {
49+
for {
4950
_, err := transform.Read()
51+
if err == io.EOF {
52+
break
53+
}
5054
if err != nil {
5155
b.FailNow()
5256
}

extensions/omniv21/samples/json/json_test.go

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package json
22

33
import (
44
"bytes"
5+
"io"
56
"io/ioutil"
67
"testing"
78

@@ -58,8 +59,11 @@ func Benchmark2_Multiple_Objects(b *testing.B) {
5859
if err != nil {
5960
b.FailNow()
6061
}
61-
for transform.Next() {
62+
for {
6263
_, err := transform.Read()
64+
if err == io.EOF {
65+
break
66+
}
6367
if err != nil {
6468
b.FailNow()
6569
}

extensions/omniv21/samples/testCommon.go

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package samples
22

33
import (
4+
"io"
45
"os"
56
"path/filepath"
67
"strings"
@@ -30,11 +31,13 @@ func SampleTestCommon(t *testing.T, schemaFile, inputFile string) string {
3031
assert.NoError(t, err)
3132

3233
var records []string
33-
for transform.Next() {
34+
for {
3435
recordBytes, err := transform.Read()
36+
if err == io.EOF {
37+
break
38+
}
3539
assert.NoError(t, err)
3640
records = append(records, string(recordBytes))
3741
}
38-
3942
return "[" + strings.Join(records, ",") + "]"
4043
}

transform.go

Lines changed: 31 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,6 @@
11
package omniparser
22

33
import (
4-
"io"
5-
64
"github.com/jf-tech/omniparser/errs"
75
"github.com/jf-tech/omniparser/schemahandler"
86
)
@@ -11,56 +9,50 @@ import (
119
// operation. An instance of a Transform must not be shared and reused among different
1210
// input streams. An instance of a Transform must not be used across multiple goroutines.
1311
type Transform interface {
14-
// Next indicates whether the ingestion/transform is completed or not.
15-
Next() bool
1612
// Read returns a JSON byte slice representing one ingested and transformed record.
13+
// io.EOF should be returned when input stream is completely consumed and future calls
14+
// to Read should always return io.EOF.
15+
// errs.ErrTransformFailed should be returned when a record ingestion and transformation
16+
// failed and such failure isn't considered fatal. Future calls to Read will attempt
17+
// new record ingestion and transformations.
18+
// Any other error returned is considered fatal and future calls to Read will always
19+
// return the same error.
20+
// Note if returned error isn't nil, then returned []byte will be nil.
1721
Read() ([]byte, error)
1822
}
1923

2024
type transform struct {
21-
ingester schemahandler.Ingester
22-
curErr error
23-
curRecord []byte
25+
ingester schemahandler.Ingester
26+
lastErr error
2427
}
2528

26-
// Next calls the underlying schema handler's ingester to do the ingestion and transformation, saves
27-
// the resulting record and/or error, and returns whether the entire operation is completed or not.
28-
func (o *transform) Next() bool {
29-
// ErrTransformFailed is a generic wrapping error around all handlers' ingesters'
29+
// Read returns a JSON byte slice representing one ingested and transformed record.
30+
// io.EOF should be returned when input stream is completely consumed and future calls
31+
// to Read should always return io.EOF.
32+
// errs.ErrTransformFailed should be returned when a record ingestion and transformation
33+
// failed and such failure isn't considered fatal. Future calls to Read will attempt
34+
// new record ingestion and transformations.
35+
// Any other error returned is considered fatal and future calls to Read will always
36+
// return the same error.
37+
// Note if returned error isn't nil, then returned []byte will be nil.
38+
func (o *transform) Read() ([]byte, error) {
39+
// errs.ErrTransformFailed is a generic wrapping error around all handlers' ingesters'
3040
// **continuable** errors (so client side doesn't have to deal with myriad of different
31-
// types of benign continuable errors. All other errors: non-continuable errors or ErrEOF
41+
// types of benign continuable errors. All other errors: non-continuable errors or io.EOF
3242
// should cause the operation to cease.
33-
if o.curErr != nil && !errs.IsErrTransformFailed(o.curErr) {
34-
return false
43+
if o.lastErr != nil && !errs.IsErrTransformFailed(o.lastErr) {
44+
return nil, o.lastErr
3545
}
36-
37-
for {
38-
record, err := o.ingester.Read()
39-
switch {
40-
case err == nil:
41-
o.curRecord = record
42-
o.curErr = nil
43-
return true
44-
case err == io.EOF:
45-
o.curErr = err
46-
o.curRecord = nil
47-
// No more processing needed.
48-
return false
49-
default:
50-
o.curErr = err
46+
record, err := o.ingester.Read()
47+
if err != nil {
48+
if o.ingester.IsContinuableError(err) {
5149
// If ingester error is continuable, wrap it into a standard generic ErrTransformFailed
5250
// so caller has an easier time to deal with it. If fatal error, then leave it raw to the
5351
// caller so they can decide what it is and how to proceed.
54-
if o.ingester.IsContinuableError(err) {
55-
o.curErr = errs.ErrTransformFailed(err.Error())
56-
}
57-
o.curRecord = nil
58-
return true
52+
err = errs.ErrTransformFailed(err.Error())
5953
}
54+
record = nil
6055
}
61-
}
62-
63-
// Read returns the current ingested and transformed record and/or the current error.
64-
func (o *transform) Read() ([]byte, error) {
65-
return o.curRecord, o.curErr
56+
o.lastErr = err
57+
return record, err
6658
}

0 commit comments

Comments
 (0)