yeboahnanaosei / go Goto Github PK

• What is the reasoning behind the repo name. go as a repo name doesn't necessarily reflect what the repo does.

• go/cval/validage.go

  • You might also want to investigate reading file in chunks to avoid loading entire files into memory.
  • L17 : there is no validation on the file input argument to check if its a CSV file, that can go a long way to help
  • L17 : the return signature is a little bit complex. For me i like the first argument returned by a validate function to be the error cos, bar evertthing else, when it is nil, i know my input is invalid. you can also just return a list of line numbers and the errors on those lines becaus the otherwise will be a valid line. No need to return multiple structures for both.
  • L28: No checking the length of a slice before accessing it will always panic if the reader is empty. Same thing for L37
  • L43: Why is an empty field deemed invalid?
  • L45: WIth your current implementation rather than append to a list which is already efficient, you can create a new slice with the exact length needed since the file is deterministic.

• go/fayasms/checks.go

  • L25: This data structure is a bit too complex.
  • L48: You can create a type aliase for conditionalFields
  • L58: Since we always know the type of the value in the field map as string, we can use a string specific verb.

• go/fayasms/endpoints.go

  • L11 - we can some enum(iotas) as keys.
  • L48 - You should never store context in a struct, it should be passed around in a function. and any function that depends on a context should accept it as its first argument.
  • L53: to make network requests, you should always specify a new client with a timeout. Avoid using the default network request for go.
  • L62 defer close should always come after the error check to the function that returns the closable ie. b/w L53 & L54
  • 64: I dont know about returning a string but isnt there a specific structure you can decode into? That should be the case since this is an API

• go/fayasms/fayasms.go

  • L20: A context object should never be put in a struct.
  • L33: A url.Values object is a map with string keys. You dont need to apend fields that are not readily available, you can alsways add them later.
  • L102: Sshcedule should take a time.Time argument and do the parsing within.

• go/fayasms/fayasms_test.go

  • L22: I dont know why you are using deep equal to compare string values, but it feels like overkill.
  • L26: I dont get the point of this particular test. What logic is being tested here.
  • L32: i think this tests the same functions as the one above. it can be combined with it.

It looks like all your test dont test behaviour of your code they just test url.Values in a convoluted ways. Tests are supposed to cover specific behavior you implemented.