Looking for tips on how to make more concise PHP code

Question

I'm not a formally trained developer and I tend to just make things work for silly little projects, but I would like a few tips on how to condense some PHP code to be more compact and efficient.

At the moment, my code is as follows, it works and does what it's supposed to, but surely there's a better way to compose it:

<?php
function turnoff($j) {
        $url = 'http://'.$j.'/api.cgi';
        $curl = curl_init();
        curl_setopt($curl, CURLOPT_URL, $url);
        curl_setopt($curl, CURLOPT_POST, true);
        curl_setopt($curl, CURLOPT_HTTPHEADER, array('Accept: application/json'));
        curl_setopt($curl, CURLOPT_POSTFIELDS, '{"command":"login","data":{"username":"admin","password":"admin"}}');
        curl_setopt($curl, CURLOPT_RETURNTRANSFER, true);

        $resp = curl_exec($curl);
        curl_close($curl);
        $jresp=json_decode($resp);
        $sessionID=$jresp->data->id->sessionID;

        $curl = curl_init();
        curl_setopt($curl, CURLOPT_URL, $url);
        curl_setopt($curl, CURLOPT_POST, true);
        curl_setopt($curl, CURLOPT_HTTPHEADER, array('Accept: application/json'));
        curl_setopt($curl, CURLOPT_POSTFIELDS, '{"command":"setdatapointvalue","data":{"sessionID":"'.$sessionID.'","uid":1,"value":1}}');
        curl_setopt($curl, CURLOPT_RETURNTRANSFER, true);

        $resp = curl_exec($curl);
        curl_close($curl);
}

turnoff('10.0.0.60');
?>

Basically it's two API calls, one is to authenticate and obtain a SessionID which is then used in a second API query to set a value against it.

Thanks for any improvements offered.

Welcome to Stack Overflow! If the code works and you're looking for advice on improving it, [codereview.se] is the appropriate place. But see https://codereview.meta.stackexchange.com/questions/5777/a-guide-to-code-review-for-stack-overflow-users first. — Barmar, Jun 08 '23 at 03:47
But there doesn't really seem to be anything you can do to improve this code. It's just setting a bunch of options and making the API call. There's no complexity to it, not much redundancy. — Barmar, Jun 08 '23 at 03:48
So there's no ways to nest the two calls in one curl session etc (which is what I was thinking, but couldn't quite work out how). — RodneyJ, Jun 08 '23 at 03:55
No, I don't think so. You do have some repetitive code for setting the same headers, you could create a function for those. — Barmar, Jun 08 '23 at 03:59
One thing: Don't hard-code JSON strings. Create an array and use `json_encode()` to convert it to JSON. — Barmar, Jun 08 '23 at 04:00
Do remember that compacting code to make it more efficient may not be good practice. In software engineering, readability and maintainability are more important. — fat penguin, Aug 29 '23 at 03:11

score 0 · Answer 1 · edited Aug 29 '23 at 02:59

0

One idea you can do, you write the curl call in a different function and pass the necessary parameters to it.

Then use that function twice in this turnoff($j) function. In that way your code will be little optimized. I'm sure you have think about it also.

edited Aug 29 '23 at 02:59

starball

20,030
7
43
238

answered Jun 08 '23 at 04:27

sayan0020

36
1
3
9

score 0 · Accepted Answer · answered Jun 08 '23 at 05:11

You can reuse the same cURL settings to make another call since everything is the same except for the request payload. You don't have to necessarily close the current cURL and open a new one.
Also, using json_encode to construct JSON strings is the ideal way to create a JSON string and not hardcoding the string as JSON via trial and errors.
Instead of storing the current cURL response in a variable, you can simply decode the JSON only the fly and access the sessionID.
Closing tags in PHP are also not necessary and it's better if you remove them. Refer this.

Snippet:

<?php

function turnoff($j) {
        $url = 'http://'.$j.'/api.cgi';
        $curl = curl_init();
        curl_setopt($curl, CURLOPT_URL, $url);
        curl_setopt($curl, CURLOPT_POST, true);
        curl_setopt($curl, CURLOPT_HTTPHEADER, array('Accept: application/json'));
        curl_setopt($curl, CURLOPT_POSTFIELDS, '{"command":"login","data":{"username":"admin","password":"admin"}}');
        curl_setopt($curl, CURLOPT_RETURNTRANSFER, true);
        $resp = curl_exec($curl);
        curl_setopt($curl, CURLOPT_POSTFIELDS, json_encode(['command' => 'setdatapointvalue', 'data' => ['sessionID' => json_decode($resp)->data->id->sessionID, 'uid' => 1, 'value' => 1]]));
        curl_exec($curl);
        curl_close($curl);
}

turnoff('10.0.0.60');

Looking for tips on how to make more concise PHP code

2 Answers2