datetime client #1
Conversation
RawanMostafa08
force-pushed
the
development
branch
from
764c985 to
3c47b5b
Compare
RawanMostafa08
force-pushed
the
development
branch
from
3c47b5b to
7bb2149
Compare
RawanMostafa08
force-pushed
the
development
branch
from
9689036 to
5370b1d
Compare
…olating http logic to the client
| baseUrl string | ||
| endpoint string | ||
| port string | ||
| contentType string |
There was a problem hiding this comment.
I don't think you need to store the content type in the client, the same client can be used to fetch more than one content type
There was a problem hiding this comment.
I think this is a matter of opinion as Abdulrahman told me that configs should have the data type so I did it this way, It was an argument to the function before
| } | ||
|
|
||
| // NewClient takes the baseUrl, endpoint, port, content-type and timeout and returns a Client object | ||
| func NewClient(baseUrl string, endpoint string, port string, contentType string, timeout time.Duration) Client { |
There was a problem hiding this comment.
timeout should be an option with default value say 10s
There was a problem hiding this comment.
what do you mean by an option? you mean optional parameter? isn't this not supported in go?
| baseUrl: baseUrl, | ||
| endpoint: endpoint, | ||
| port: port, |
There was a problem hiding this comment.
you can create and store only the request url instead of storing this data
| // GetTime creates and sends the request and uses the retry mechanism | ||
| // for maximum of 10 seconds before the request fails | ||
| // it then returns the time and an error if it failed to send for 10 seconds | ||
| func (c Client) GetTime() (time.Time, error) { |
There was a problem hiding this comment.
I think content type should be passed as an argument here
… removing some if elses, checking on status code before reading the body
Issue link: codescalersinternships/home#288
This PR implements the first version of a datetime client