Crypto monitor josantonius #9
Conversation
| $provider = 'coinapi'; | ||
|
|
||
| $apiKey = 'your-coin-api-key'; | ||
| $apiKey = '4E861687-19D6-4894-87B9-E785B1EE3900'; |
There was a problem hiding this comment.
Is this API key free-trial? If it's, I think it's fine to expose this key here.
|
|
||
| $params = self::$providers[$config->provider][$config->type]; | ||
|
|
||
| switch ($config->type) { |
There was a problem hiding this comment.
How about enhancing this type condition?
I think using the switch...case is not the proper way to do.
If we have the many types to do the different action, the swittch condition will be very long.
| class GetAllRate implements ProviderResource | ||
| { | ||
| private $apiKey; | ||
| public $config; |
There was a problem hiding this comment.
Let $config be public and it means we can modify this at any time from this class instance.
How about using the private/protected $config?
And we can also add the setConfig and getConfig to allow us to modify this private/protected $config :).
peter279k
left a comment
peter279k
left a comment
There was a problem hiding this comment.
Most of changes aree good for me, but the Travis CI build is broken.
This build work need to be fixed and I also have some request chnages.
Please review them at your available time 😄 .
This is the new branch that @josantonius creates.
I just create this PR for helping us to do the code review :).
@josantonius, can you fix the Travis CI build and
README.mdconflicts? Thanks.