Merge pull request #1 from leninhasda/improve

Improvement and fix

README.md

@@ -93,6 +93,8 @@ func main() {
 
 ```
 
+### **Contribution**
+If you are interested to make the package better please send pull requests or create an issue so that others can fix. Read the [contribution guide here](CONTRIBUTING.md). 
 
 ### **License**
 The **retry** is an open-source software licensed under the [MIT License](LICENSE.md).

retry.go

@@ -28,7 +28,11 @@ func DoFunc(attempt int, sleep time.Duration, fn func() error) error {
 
 // Do try to execute the function by its value, function can take variadic arguments and return multiple return.
 // You must put error as the last return value so that DoFunc can take decision that the call failed or not
-func Do(attempt int, sleep time.Duration, fn interface{}, args ...interface{}) ([]interface{}, error) {
+func Do(attempt uint, sleep time.Duration, fn interface{}, args ...interface{}) ([]interface{}, error) {
+
+	if attempt == 0 {
+		return nil, errors.New("retry: attempt should be greater than 0")
+	}
 
 	vfn := reflect.ValueOf(fn)
 
@@ -53,26 +57,27 @@ func Do(attempt int, sleep time.Duration, fn interface{}, args ...interface{}) (
 		in[k] = reflect.ValueOf(a)
 	}
 
-	// call the fn with arguments
-	out := []interface{}{}
-	for _, o := range vfn.Call(in) {
-		out = append(out, o.Interface())
-	}
+	var lastErr error
+	for attempt > 0 {
+		// call the fn with arguments
+		out := []interface{}{}
+		for _, o := range vfn.Call(in) {
+			out = append(out, o.Interface())
+		}
 
-	// if the last value is not error then return an error
-	err, ok := out[len(out)-1].(error)
-	if !ok && out[len(out)-1] != nil {
-		return nil, errors.New("retry: fn return's right most value must be an error")
-	}
+		// if the last value is not error then return an error
+		err, ok := out[len(out)-1].(error)
+		if !ok && out[len(out)-1] != nil {
+			return nil, errors.New("retry: fn return's right most value must be an error")
+		}
 
-	if err != nil {
-		attempt--
-		if attempt > 0 {
-			time.Sleep(sleep)
-			return Do(attempt, sleep, fn, args...)
+		if err == nil {
+			return out[:len(out)-1], nil
 		}
-		return out, err
+		lastErr = err
+		attempt--
+		time.Sleep(sleep)
 	}
 
-	return out[:len(out)-1], nil
+	return nil, lastErr
 }

retry_test.go

@@ -61,37 +61,46 @@ func TestDo(t *testing.T) {
 		return false
 	}
 
+	multiRet := func() (int, bool, error) {
+		return 1, false, nil
+	}
+
 	testcases := []struct {
 		Tag           string
 		Func          interface{}
 		Args          []interface{}
 		Result        interface{}
+		Len           int
 		ExpectedError bool
 	}{
 		{
 			Tag:    "Add 1 to 4 and expected result 10",
 			Func:   sum,
 			Args:   []interface{}{1, 2, 3, 4},
 			Result: 10,
+			Len:    1,
 		},
 		{
 			Tag:    "Add 1 to 5 and expected result 15",
 			Func:   sum,
 			Args:   []interface{}{1, 2, 3, 4, 5},
 			Result: 15,
+			Len:    1,
 		},
 		{
 			Tag:    "Div 9.0/3.0 and expected result 3.0",
 			Func:   div,
 			Args:   []interface{}{9.0, 3.0},
 			Result: 3.0,
+			Len:    1,
 		},
 		{
 			Tag:           "Div 9.0/0.0 and expected result 0 with error",
 			Func:          div,
 			Args:          []interface{}{9.0, 0.0},
 			Result:        0.0,
 			ExpectedError: true,
+			Len:           1,
 		},
 		{
 			Tag:           "As div is not a variadic func, if args mismatch we expect error",
@@ -105,16 +114,23 @@ func TestDo(t *testing.T) {
 			ExpectedError: true,
 		},
 		{
-			Tag:           "As 'voidFunc' is does not return anything we expect an error from retry package",
+			Tag:           "As 'voidFunc' does not return anything we expect an error from retry package",
 			Func:          voidFunc,
 			ExpectedError: true,
 		},
 		{
-			Tag:           "As 'noErrorFunc' is does not return error we silently try to execute the func only once",
+			Tag:           "As 'noErrorFunc' does not return error we silently try to execute the func only once",
 			Func:          noErrorFunc,
 			Result:        false,
 			ExpectedError: true,
 		},
+		{
+			Tag:           "As 'multiRet' returns back two data we try to check length of out with error false",
+			Func:          multiRet,
+			Result:        1,
+			ExpectedError: false,
+			Len:           2,
+		},
 	}
 
 	for _, tc := range testcases {
@@ -125,9 +141,19 @@ func TestDo(t *testing.T) {
 		}
 
 		if !tc.ExpectedError && out != nil {
+			if len(out) != tc.Len {
+				t.Errorf("Failed: %s \nExpected length: %v \nGot: %v", tc.Tag, tc.Len, len(out))
+			}
 			if out[0] != tc.Result {
 				t.Errorf("Failed: %s \nExpected: %v \nGot: %v", tc.Tag, tc.Result, out[0])
 			}
 		}
 	}
 }
+
+func TestDoAttempt(t *testing.T) {
+	_, err := Do(0, 1*time.Millisecond, func() {})
+	if err == nil {
+		t.Errorf("Failed: expected attempt 0 error")
+	}
+}