I have this code but it takes over a minute to run. I'm looking at around 400 entries being brought back for the first loop. Any ideas on how it could be improved for speed?
<?php
$context = stream_context_create(array(
'http' => array(
'method' => 'GET',
'header' => "Authorization: Basic abc123\r\nContent-Type: application/json\r\n"//,'content' => json_encode($postData)
)
));
// Send the request
$response = file_get_contents('https://localhost', FALSE, $context);
// Check for errors
if($response === FALSE){
die('Error');
}
// Decode the response
$responseData = json_decode($response, TRUE);
// Print the date from the response
foreach($responseData as $row) {
foreach($row as $k) {
$response = file_get_contents('https://localhost/answers'.$k['id'], FALSE, $context);
$responseDataAns = json_decode($response, TRUE);
echo $responseDataAns['summary']." / ".$responseDataAns['solution']."\r\n";
}
}
?>
This is the REST API I am using specifically. This is used to retrieve all answers then this to get an instance.
I want to use the REST API to list all questions, allow someone to go into a question and then update it.
2 Answers 2
It seems to me that you are making multiple requests to 'https://localhost/answers'.$k['id']
inside a loop and that you have one route for each answers
, such as:
https://localhost/answers100
https://localhost/answers101
https://localhost/answers102
My suggestion is to abandon that approach and combine them into one route that can handle multiple ids. For instance:
https://localhost/answers?id[]=100&id[]=101&id[]=102
Then grab the id like this:
$ids = $_GET['id'];
which is an array. Process each id one by one and combine the result and send as JSON. For instance:
{
"100": {
"summary": "",
"solution": ""
},
"101": {
"summary": "",
"solution": ""
},
"102": {
"summary": "",
"solution": ""
}
}
So, instead of making 400 requests, you only need to make 1 request. Much faster.
To answer your question about performance: you are doing 400 requests to an external service. What did you expect? Even if your requests are returned in 30ms, it will still take you 2 minutes.
Now on to the codereview:
Coding style
Your coding style is artistic. Try to stick to a well known coding standard like psr-1 and psr-2. This will make your code easier to read for others and yourself.
Note that most IDE's come built-in with this code style and they do the formatting for you.
Seperation of concern
Your code lacks any kind of seperaration; Everything is written in one go, making it very hard to understand and/or maintain. Depending on your choice. Given the size of the code, creating a couple of functions might be usefull:
function retrieveAnswer($answerId);
function retrieveAnswerList($answerId);
function retrieveObject($endpoint);
this would clean up your code a bit:
``` $answers = retrieveAnswerList('/');
foreach($answers as $answerRow) {
foreach($answerRow as $answerDefinition) {
$answer = retrieveAnswer($answerDefinition['id']);
printAnswer($answer);
}
}
```
We now seperated your "business logic" from the actual implementations. This code doesn't care if the api returns json or xml or ...
Variable names
As seen above, I changed the variable names to have more meaning. Avariable called $responseData
doesnt say anything. What kind of data? what kind of response? Do we even care the data came from a response? Choosing variables is hard, but it will make your code read more like natural language.
Comments
comments should explain the why, not the what.
// Decode the response
. Yes, I can see your are decoding something, thats what json_decode means. I know that. But why?
Small code remark
I love that you use contest + file_get_contents. Not many developers know that trick. But you can have an even neater syntax:
$context = stream_context_create(array(
'http' => array(
'method' => 'GET',
'header' => array(
"Authorization" => "Basic abc123",
"Content-Type" => "application/json",
),
'content' => json_encode($postData)
)
));
$response
? What are those rows? Also, why are you usingob_flush();
? I don't seeob_start();
anywhere. Also, wouldn't be easier to make a single cache file to which you would be storing the data you are now generating every single time? Its kinda hard to know what this actually does and whats in the files. You have to provide a bit more to this question. \$\endgroup\$