Refactoring package #7
Conversation
add tests for coverage up
add new type and new file for credit cart
and add helpers function, which check slice string type to has element
done concept
|
I sent PR before merge with master branch |
|
It's look like your PR not past the test in travis |
faker.go
Outdated
| var ErrValueNotPtr = "Not a pointer value" | ||
|
|
||
| // Error when tah not supported | ||
| var ErrTagNotSupported = "String Tag not unsupported" |
faker.go
Outdated
| "time" | ||
| ) | ||
|
|
||
| var src = rand.NewSource(time.Now().UnixNano()) | ||
| var mu sync.Mutex |
There was a problem hiding this comment.
your mutex is still nil. This can cause nil pointer dereference
There was a problem hiding this comment.
got any suggestions?
I can insert in every function constructor and init
There was a problem hiding this comment.
Maybe you just define like src
mu = sync.Mutex{}
as the package variable
There was a problem hiding this comment.
do not understan...
maybe there is an example?
There was a problem hiding this comment.
var mutex = &sync.Mutex{}
It will available and can used in the entire package
There was a problem hiding this comment.
aaa now I understand
internet.go
Outdated
| r := rand.New(src) | ||
| size := 4 | ||
| ip := make([]byte, size) | ||
| for i := 0; i < size; i++ { |
There was a problem hiding this comment.
please use a better variable. This is can make readers confused.
you define the function owned by the Internet
func (i Internet)
But you use the i variable in different role here as the iteration counter.
internet.go
Outdated
| func (i Internet) MacAddress() string { | ||
| r := rand.New(src) | ||
| ip := make([]byte, 6) | ||
| for i := 0; i < 6; i++ { |
internet.go
Outdated
| r := rand.New(src) | ||
| size := 16 | ||
| ip := make([]byte, size) | ||
| for i := 0; i < size; i++ { |
internet.go
Outdated
| "strings" | ||
| ) | ||
|
|
||
| var tld = []string{"com", "com", "biz", "info", "net", "org", "ru"} |
There was a problem hiding this comment.
any reason why use duplicates com
internet_test.go
Outdated
| } | ||
| } | ||
| func TestUserName(t *testing.T) { | ||
| getNetworker().UserName() |
There was a problem hiding this comment.
add assertion, if it's not an empty string
|
Well I'll fix all comments |
| func Contains(slice []string, item string) bool { | ||
| set := make(map[string]struct{}, len(slice)) | ||
| for _, s := range slice { | ||
| set[s] = struct{}{} |
There was a problem hiding this comment.
I can't find this function used. Maybe you can remove this.
*ps: Any reason why using map. Why just not return if s == item directly ?
There was a problem hiding this comment.
it is used internet_test.go - 24
|
The test in travis still not passed. |
Codecov Report
@@ Coverage Diff @@
## master #7 +/- ##
=====================================
Coverage 100% 100%
=====================================
Files 2 4 +2
Lines 212 245 +33
=====================================
+ Hits 212 245 +33
Continue to review full report at Codecov.
|
Hello!
I left the old api, but I added the possibility of more flexible extension.
The ability to add a custom implementation depending on the localization
mapperTagLook, what are your thoughts on this ?