Shielding your CodeReview on GitHub

Question

In response to a feature-request on meta, I spent a few hours today creating the feature in php.

This application adds a review shield to your GitHub repository (or wherever else you want it).

It looks like the following:

Code Review

To use it, use standard image/link markdown, like this:

[![Code Review](https://www.zomis.net/codereview/shield/?qid=54737)](https://codereview.stackexchange.com/q/54737/31562)

Replace 54737 with the question id you want to direct to, and replace 31562 with your user id (so that you can get the Announcer / Booster / Publicist badges).

You can also use the Code Review Shield Creator to create your shield.

I used shields.io to get the basic SVG XML and adapted that.

There are different ways the badge can be shown:

• Unanswered questions shows the question score and a red background
• Answered questions shows the number of answers and an orange background
• Questions with accepted answer shows the view count and a green background

How the code works

Because of the daily Stack Exchange API limit of 10,000 requests, I am avoiding too many API requests by storing previous results in a database table, and only performing a new request if it has been more than an hour since the last API request for that particular question.

The code contains these functions:

• buildURL($apiCall, $site, $filter, $apiKey): Creates the URL for the Stack Exchange API call (for future use, I check if the apiCall parameter contains a '?' or not).
• apiCall($apiCall, $site, $filter): Performs a HTTP request to the Stack Exchange API using curl, returns JSON data as a pure string
• fetchQuestion($qid, $db): Uses the JSON data retrieved by apiCall as an associative array, extracts the interesting data from it, updates the database, and calls useData.
• useData($data): Takes an associative array and creates SVG XML for it
• dbOrAPI($qid, $db): Main point of entry. $qid is a Code Review question id and $db is a PDO object. Checks the database for the existence of the qid and uses it if it is somewhat up-to-date, otherwise calls fetchQuestion

This code is also available at https://github.com/Zomis/CodeReview-Shield

<?php
require 'conf.php';
function buildURL($apiCall, $site, $filter, $apiKey) {
 if (strpos($apiCall, '?') === false) {
 $apiCall = $apiCall + "?dummy";
 }
 return "https://api.stackexchange.com/2.2/" . $apiCall
 . "&site=" . $site
 . "&filter=" . $filter . "&key=" . $apiKey;
}
function apiCall($apiCall, $site, $filter) {
 global $apiKey;
 $url = buildURL($apiCall, $site, $filter, $apiKey);
 $ch = curl_init($url);
 curl_setopt($ch, CURLOPT_ENCODING, 'gzip');
 curl_setopt($ch, CURLOPT_RETURNTRANSFER, true);
 $result = curl_exec($ch);
 if ($result === false) {
 $error = curl_error($ch);
 curl_close($ch);
 throw new Exception("Error calling Stack Exchange API: $error");
 }
 curl_close($ch);
 return $result;
}
function fetchQuestion($qid, $db) {
 $filter = "!)rcjzniPuafk4WNG65yr";
 $data = apiCall("questions/$qid?order=desc&sort=activity", 'codereview', $filter);
 $json = json_decode($data, true);
 $question = $json['items'][0];
 $dbfields = array("is_answered", "view_count", "favorite_count", "answer_count", "score", "accepted_answer_id");
 $sql = 'INSERT INTO cr_badge (question_id, is_answered, favorite_count, answer_count, view_count, score, fetch_time, accepted_answer_id) ' .
 'VALUES (:qid, :is_answered, :favorite_count, :answer_count, :view_count, :score, :time, :accepted_answer_id) ON DUPLICATE KEY UPDATE ' .
 'is_answered = :is_answered, favorite_count = :favorite_count, answer_count = :answer_count, view_count = :view_count, score = :score, fetch_time = :time, accepted_answer_id = :accepted_answer_id ;';
 $stmt = $db->prepare($sql);
 $sql_params = array();
 foreach ($dbfields as $field_name) {
 if (isset($question[$field_name])) {
 $sql_params[':' . $field_name] = $question[$field_name];
 } else {
 $sql_params[':' . $field_name] = 0;
 }
 }
 $sql_params[':qid'] = $qid;
 $sql_params[':time'] = time();
 $result = $stmt->execute($sql_params);
 if ($result) {
 useData($question);
 } else {
 print_r($stmt->errorInfo());
 }
 return $json;
}
function useData($data) {
 header('Content-type: image/svg+xml; charset=utf-8');
 $is_answered = $data['text'];
 $text = 'reviewed';
 if (isset($data['accepted_answer_id']) && $data['accepted_answer_id'] != 0) {
 $color = '97ca00';
 $mode = 'views';
 } elseif ($data['answer_count'] >= 1) {
 $color = 'ff8000';
 $right = $data['score'] . ' score';
 $mode = 'answers';
 } else {
 $color = 'e05d44';
 $text = 'reviewing';
 $mode = 'score';
 }
 if (isset($_GET['mode'])) {
 $mode = $_GET['mode'];
 }
 $data['answers'] = $data['answer_count'];
 $data['views'] = $data['view_count'];
 $right = $data[$mode] . ' ' . $mode;
 $svg = <<<END
<svg xmlns="http://www.w3.org/2000/svg" width="137" height="20">
<linearGradient id="b" x2="0" y2="100%">
<stop offset="0" stop-color="#bbb" stop-opacity=".1"/>
<stop offset="1" stop-opacity=".1"/>
</linearGradient>
<mask id="a">
<rect width="137" height="20" rx="3" fill="#fff"/>
</mask>
<g mask="url(#a)">
<path fill="#555" d="M0 0h62v20H0z"/>
<path fill="#$color" d="M62 0h75v20H62z"/>
<path fill="url(#b)" d="M0 0h137v20H0z"/>
</g>
<g fill="#fff" text-anchor="middle" font-family="DejaVu Sans,Verdana,Geneva,sans-serif" font-size="11">
<text x="31" y="15" fill="#010101" fill-opacity=".3">$text</text>
<text x="31" y="14">$text</text>
<text x="98.5" y="15" fill="#010101" fill-opacity=".3">$right</text>
<text x="98.5" y="14">$right</text>
</g>
</svg>
END;
 echo $svg;
}
function dbOrAPI($qid, $db) {
 $sql = 'SELECT is_answered, favorite_count, answer_count, view_count, score, fetch_time, accepted_answer_id FROM cr_badge WHERE question_id = :qid;';
 $stmt = $db->prepare($sql);
 $result = $stmt->execute(array(':qid' => $qid));
 if ($result) {
 $row = $stmt->fetch(PDO::FETCH_ASSOC);
 $time = $row['fetch_time'];
 if ($time < time() - 3600) { // if time was updated more than one hour ago
 // fetch data again
 fetchQuestion($qid, $db);
 } else {
 useData($row);
 }
 } else {
 print_r($stmt->errorInfo());
 }
}
if (isset($_GET['qid'])) {
 $qid = $_GET['qid'];
} else {
 die("No qid set");
}
try {
 $db = new PDO($dbhostname, $dbuser, $dbpass);
} catch (PDOException $e) {
 echo 'Connection failed: ' . $e->getMessage();
 return false;
}
dbOrAPI($qid, $db);

Primary Concerns

It's been a while since I used PHP, so I am interested to know whether I'm adhering to the PHP conventions (if there are any), and if I'm using it as it is meant to be used.

Any comments are also welcome.

• TRiG: PHP coding standards: github.com/php-fig/fig-standards. (Note: I don't like all of these, particluarly the use of Egyptian brackets.)
• nhaarman: I issued a pull request for adding a CodeReview-Shield to your CodeReview-Shield repo. Lol.
• Michael Anderson: I find the light grey text on medium green a bit hard to read. I'd go with either a darker green background, or pure green text.
• Simon Forsberg: @IsmaelMiguel You can use it already, see the GitHub repository for this, I also included usage instructions in this post. The only thing is that I am hosting the service, and not Stack Exchange / Shields.io / GitHub / whatever.
• Graham: How does this tool extend to multiple CRs on multiple commits? Are committers expected to update the question/user# with every pull request? Or would it be more appropriate to have a separate badge for each commit? If so, where would those badges live?

Answer 1

On a personal note, your code is really clean, the idea is brilliant, and I really hope to see it implemented soon.

---

Your code is well implemented and structured, but there's syntax points that could be improved.

1. I see a lot of basic if-else statements, if you're into ternaries, using them really slims down these statements, but it's at the cost of readability.

   See the following examples for usage:

   ($time < time() - 3600 ? fetchQuestion($qid, $db) : useData($row));
   ($result ? useData($question) : print_r($stmt->errorInfo()));
   

   I do believe some versions of PHP support (a == a ?: doStuff()); syntax also, however, I could be mistaken.

2. There's a few points that are either inconsistent or don't adhere to standards:

   $apiCall = $apiCall + "?dummy"; 
   

   Should be $apiCall .= "?dummy";, and you shouldn't use + when concatenating strings, it's best to use . instead.

   Switching between implementing the variable directly in the string 'words$varmorewords' or adding it like:. $var . , I would recommend the latter as it's more reader-friendly, and because the former can have issues, it's best to wrap in curly braces: 'words{$var}morewords' in place of the former.

3. Using curl instead of get_file_contents is great, I see a lot of people make that mistake, and I've even too.

4. You have two blank lines above your return $json statement in fetchQuestion(), they don't need to be there.

5. in useData(), you create the variable $is_answered and then never use it, and I'd suggest replacing its value with $data['accepted_answer_id'] so that your if loop looks cleaner.

6. You could consider keeping the SVG in another file and replacing in your variables, but I'm not 100% on its standing as a code standard / best practice.

7. In useData(), rather than doing a double check (isset: (returns true for '') and != 0), you can just compare to > 0.

8. (削除) You retrieve and store quite a few variables for each post that aren't currently used in the final image:
   "is_answered" : Not used, "view_count": Used, "favorite_count": Not used, "answer_count": Not used, "score": Not used, "accepted_answer_id": Not used.

   Although I can see future updates using these, and capturing them now is great, but, you could look at modifying that. (削除ここまで)

   On the topic of future implementations, a score counter on the button would be pretty cool too.

9. You could consider implementing a namespace and class like structure into your project so it can be used externally, easier.

• Simon Forsberg: 1. for PHP, I think I will avoid that. 2. whoops, I guess that's what happens when you copy code from Java and convert it to PHP. 3. file_get_contents didn't work, had to use curl :) 4. whoops, forgot copy-pasting new code after I removed those (was the only little change I did) 5. indeed, thanks. 6. might be a good idea, that can also lead to the possibility of using different SVG templates. I like it. 7. the JSON from SE API might not have that set, I was under the impression that PHP adds a warning or notice if I don't do it like this, but I might be wrong. 8. some more of those are used.
• Ismael Miguel: "Should be $apiCall .= "?dummy";, and you shouldn't use + when concatenating strings" --> You should say "You must use $apiCall .= "?dummy";. Using + to concatenate strings will result in summing the letters which always results in 0.. Also, you can consider this a bug.
• Ismael Miguel: Also, I've noticed that you have 'words$varmorewords' as an example. This echo words$varmorewords because variables inside single-quotes aren't expanded. If you replace the quotes, you will have an ugly error because $varmorewords is a single variable, that doesn't exist. It should be "words{$var}morewords" instead.
• Ryan: As an aside, using the . operator for string concatenation is an abomination that never should of happened.
• scrowler: Regarding point #1: there are not many scenarios where reducing readability to save character count is considered a good thing. Ternaries are a perfect example of this. Point #2 will render the string literal, you need to suggest double quotes. Point #3: what is the problem with either? They're both useful for their own purposes. Point #5: isset is important, never suggest removing it.

Answer 2

I guess this is about as beautiful as PHP gets :p (I had (削除) not (削除ここまで) fun discovering this monstrosity.)

---

One minor suggestion I have is to eliminate the duplication here:

 if (isset($question[$field_name])) {
 $sql_params[':' . $field_name] = $question[$field_name];
 } else {
 $sql_params[':' . $field_name] = 0;
 }

I'm surprised our good friend of ternarys @Quill forgot to include this prime candidate:

$sql_params[':' . $field_name] = isset($question[$field_name]) ? $question[$field_name] : 0;

---

Another thing that put me off a little bit is this mysterious piece in the middle the code:

$filter = "!)rcjzniPuafk4WNG65yr";

What is this about? Where did this value come from? Is it important?

Like all magic constants, it would be good to put it near the top of the file with a descriptive name.

---

Lastly, maybe it's not feasible at all, but it would be nice to be able to style the display using CSS, instead of the hard-coded $color values.

---

Finally, strpos($apiCall, '?') === false is cryptic enough (fault of PHP, not yours), that it might be worth encapsulating it in a helper method:

function contains($haystack, $needle) {
 return strpos($haystack, $needle) !== false;
}

It keeps the rest of your code clean and nicely readable, and isolates the, ahem, garbage.

• Simon Forsberg: 1. yeah, strpos in PHP is not fun (but then again, it is PHP, what is?). 2. I agree with that ternary. 3. the filter is used for Stack Exchange API, so that I only get the information I am somewhat interested in - saves bandwidth. I don't agree though that all constants should be declared at the top. This is only used once, it is directly associated with the SE API, I'm don't see anything good that would come from putting it at the top. 4. I'm outputting XML for SVG graphics, I doubt I could use CSS there. I'm all for making it more dynamic, but I don't think CSS is the way to go :)
• Ismael Miguel: I don't get what's wrong with strpos().... Besides the order of the arguments.
• janos: If it was 100% idea I would put it in my answer ;-) True I didn't explain enough, the problem with preg_match is that it's not idiomatic. Compared to is, strpos is still easier to remember and read. Maybe it's not inferior. But at least for me it's not good enough to suggest to change.
• Ismael Miguel: @janos I totally agree with that. Someone needs to make a decent string library for PHP.
• janos: @Simon I added one more point about strpos. And about "!)rcjzniPuafk4WNG65yr", it's an incomprehensible thing whose purpose is not obvious in the middle of your code. When something is inexplicable, I cannot trust it, and assume that someday it might change, inexplicably, and then I wouldn't want to dig it out from the middle of code. That's why I prefer such things at the top, where easy to see, with a descriptive name, maybe even a comment explaining where it came from.

Answer 3

Disclamer:

My review (削除) will be short (削除ここまで) is longer than I expected but I will only focus on the function useData().

I've read it carefully and did my best to improve it and make it more readable for you.

---

Lets get it started!

1. The first thing that pops in my head is that giant pile of un-indented SVG:
   

 $svg = <<<END
<svg xmlns="http://www.w3.org/2000/svg" width="137" height="20">
<linearGradient id="b" x2="0" y2="100%">
<stop offset="0" stop-color="#bbb" stop-opacity=".1"/>
<stop offset="1" stop-opacity=".1"/>
</linearGradient>
<mask id="a">
<rect width="137" height="20" rx="3" fill="#fff"/>
</mask>
<g mask="url(#a)">
<path fill="#555" d="M0 0h62v20H0z"/>
<path fill="#$color" d="M62 0h75v20H62z"/>
<path fill="url(#b)" d="M0 0h137v20H0z"/>
</g>
<g fill="#fff" text-anchor="middle" font-family="DejaVu Sans,Verdana,Geneva,sans-serif" font-size="11">
<text x="31" y="15" fill="#010101" fill-opacity=".3">$text</text>
<text x="31" y="14">$text</text>
<text x="98.5" y="15" fill="#010101" fill-opacity=".3">$right</text>
<text x="98.5" y="14">$right</text>
</g>
</svg>
END;
 echo $svg;

It sure need some indentation. It's a total mess! Consider this:

 $svg = <<<END
<svg xmlns="http://www.w3.org/2000/svg" width="137" height="20">
 <linearGradient id="b" x2="0" y2="100%">
 <stop offset="0" stop-color="#bbb" stop-opacity=".1"/>
 <stop offset="1" stop-opacity=".1"/>
 </linearGradient>
 <mask id="a">
 <rect width="137" height="20" rx="3" fill="#fff"/>
 </mask>
 <g mask="url(#a)">
 <path fill="#555" d="M0 0h62v20H0z"/>
 <path fill="#$color" d="M62 0h75v20H62z"/>
 <path fill="url(#b)" d="M0 0h137v20H0z"/>
 </g>
 <g fill="#fff" text-anchor="middle" font-family="DejaVu Sans,Verdana,Geneva,sans-serif" font-size="11">
 <text x="31" y="15" fill="#010101" fill-opacity=".3">$text</text>
 <text x="31" y="14">$text</text>
 <text x="98.5" y="15" fill="#010101" fill-opacity=".3">$right</text>
 <text x="98.5" y="14">$right</text>
 </g>
</svg>
END;
 echo $svg;

So much better now!

---

2. There's still an useless attribution. Lets fix that too:

echo <<<END
<svg xmlns="http://www.w3.org/2000/svg" width="137" height="20">
 <linearGradient id="b" x2="0" y2="100%">
 <stop offset="0" stop-color="#bbb" stop-opacity=".1"/>
 <stop offset="1" stop-opacity=".1"/>
 </linearGradient>
 <mask id="a">
 <rect width="137" height="20" rx="3" fill="#fff"/>
 </mask>
 <g mask="url(#a)">
 <path fill="#555" d="M0 0h62v20H0z"/>
 <path fill="#$color" d="M62 0h75v20H62z"/>
 <path fill="url(#b)" d="M0 0h137v20H0z"/>
 </g>
 <g fill="#fff" text-anchor="middle" font-family="DejaVu Sans,Verdana,Geneva,sans-serif" font-size="11">
 <text x="31" y="15" fill="#010101" fill-opacity=".3">$text</text>
 <text x="31" y="14">$text</text>
 <text x="98.5" y="15" fill="#010101" fill-opacity=".3">$right</text>
 <text x="98.5" y="14">$right</text>
 </g>
</svg>
END;

---

3. Alright, much better now. But you have 'stray' variables lost within your SVG.

To make it easier to read, consider wrapping the variables in brackets:

echo <<<END
<svg xmlns="http://www.w3.org/2000/svg" width="137" height="20">
 <linearGradient id="b" x2="0" y2="100%">
 <stop offset="0" stop-color="#bbb" stop-opacity=".1"/>
 <stop offset="1" stop-opacity=".1"/>
 </linearGradient>
 <mask id="a">
 <rect width="137" height="20" rx="3" fill="#fff"/>
 </mask>
 <g mask="url(#a)">
 <path fill="#555" d="M0 0h62v20H0z"/>
 <path fill="#{$color}" d="M62 0h75v20H62z"/>
 <path fill="url(#b)" d="M0 0h137v20H0z"/>
 </g>
 <g fill="#fff" text-anchor="middle" font-family="DejaVu Sans,Verdana,Geneva,sans-serif" font-size="11">
 <text x="31" y="15" fill="#010101" fill-opacity=".3">{$text}</text>
 <text x="31" y="14">{$text}</text>
 <text x="98.5" y="15" fill="#010101" fill-opacity=".3">{$right}</text>
 <text x="98.5" y="14">{$right}</text>
 </g>
</svg>
END;

Way better, isn't it?

---

4. But now, you want to change a color. How would you do it? Change everything by hand?

I propose the following (partial) code:

$colors = array(
 'gradient'=>'bbb',
 'mask'=>'fff',
 'back'=>array('555', 'e05d44'),
 'text'=>'010101',
 'right'=>'010101'
);
if (isset($data['accepted_answer_id']) && $data['accepted_answer_id'] != 0) {
 $color['back'][1] = '97ca00';
 $mode = 'views';
} elseif ($data['answer_count'] >= 1) {
 $colors['back'][1] = 'ff8000';
 $right = $data['score'] . ' score';
 $mode = 'answers';
} else {
 $text = 'reviewing';
 $mode = 'score';
}
// [...]
 echo <<<END
<svg xmlns="http://www.w3.org/2000/svg" width="137" height="20">
 <linearGradient id="b" x2="0" y2="100%">
 <stop offset="0" stop-color="#{$colors['gradient']}" stop-opacity=".1"/>
 <stop offset="1" stop-opacity=".1"/>
 </linearGradient>
 <mask id="a">
 <rect width="137" height="20" rx="3" fill="#{$colors['mask']}"/>
 </mask>
 <g mask="url(#a)">
 <path fill="#{$colors['back'][0]}" d="M0 0h62v20H0z"/>
 <path fill="#{$colors['back'][1]}" d="M62 0h75v20H62z"/>
 <path fill="url(#b)" d="M0 0h137v20H0z"/>
 </g>
 <g fill="#fff" text-anchor="middle" font-family="DejaVu Sans,Verdana,Geneva,sans-serif" font-size="11">
 <text x="31" y="15" fill="#{$colors['text']}" fill-opacity=".3">{$text}</text>
 <text x="31" y="14">{$text}</text>
 <text x="98.5" y="15" fill="#{$colors['right']}" fill-opacity=".3">{$right}</text>
 <text x="98.5" y="14">{$right}</text>
 </g>
</svg>
END;

Notice that I've removed the color attribution on the variable $colors on the else, and made it the default color.

---

5. You have the following code:

if (isset($_GET['mode'])) {
 $mode = $_GET['mode'];
}
$data['answers'] = $data['answer_count'];
$data['views'] = $data['view_count'];
$right = $data[$mode] . ' ' . $mode;

Do you smell that? I smell code injection!

Please, always validate your input.

Simply use this if instead:

if (isset($_GET['mode']) && in_array($_GET['mode'], array('views','answers','score'))) {
 $mode = $_GET['mode'];
}

---

6. This point is purely subjective.

You are blindly trusting that your code has no output before this function.

Instead of this:

header('Content-type: image/svg+xml; charset=utf-8');

Consider using this:

if (!headers_sent()) {
 header('Content-type: image/svg+xml; charset=utf-8');
}

In case you happen to have an error, it will still send the SVG with the previous errors, but at least it won't be an error factory!

Due to it being subjective and not everybody agreeing on it, I've decided to remove it from the final code.

---

7. As pointed out before, you have an useless variable ($is_answered). I've removed it as well, since it isn't doing anything there.

---

8. A very picky point would be to change echo <<<END to echo <<<SVG.

This shows what the echo is all about and what is that huge block, without reading more than 12 characters.

---

Final result:

This is what the code looks like, with additional lines to increase readability:

function useData($data) {
 header('Content-type: image/svg+xml; charset=utf-8');
 $is_answered = $data['text'];
 $text = 'reviewed';
 $colors = array(
 'gradient'=>'bbb',
 'mask'=>'fff',
 'back'=>array('555', 'e05d44'),
 'text'=>'010101',
 'right'=>'010101'
 );
 if (isset($data['accepted_answer_id']) && $data['accepted_answer_id'] != 0) {
 $color['back'][1] = '97ca00';
 $mode = 'views';
 } elseif ($data['answer_count'] >= 1) {
 $colors['back'][1] = 'ff8000';
 $right = $data['score'] . ' score';
 $mode = 'answers';
 } else {
 $text = 'reviewing';
 $mode = 'score';
 }
 if (isset($_GET['mode']) && in_array($_GET['mode'], array('views','answers','score'))) {
 $mode = $_GET['mode'];
 }
 $data['answers'] = $data['answer_count'];
 $data['views'] = $data['view_count'];
 $right = $data[$mode] . ' ' . $mode;
 echo <<<SVG
<svg xmlns="http://www.w3.org/2000/svg" width="137" height="20">
 <linearGradient id="b" x2="0" y2="100%">
 <stop offset="0" stop-color="#{$colors['gradient']}" stop-opacity=".1"/>
 <stop offset="1" stop-opacity=".1"/>
 </linearGradient>
 <mask id="a">
 <rect width="137" height="20" rx="3" fill="#{$colors['mask']}"/>
 </mask>
 <g mask="url(#a)">
 <path fill="#{$colors['back'][0]}" d="M0 0h62v20H0z"/>
 <path fill="#{$colors['back'][1]}" d="M62 0h75v20H62z"/>
 <path fill="url(#b)" d="M0 0h137v20H0z"/>
 </g>
 <g fill="#fff" text-anchor="middle" font-family="DejaVu Sans,Verdana,Geneva,sans-serif" font-size="11">
 <text x="31" y="15" fill="#{$colors['text']}" fill-opacity=".3">{$text}</text>
 <text x="31" y="14">{$text}</text>
 <text x="98.5" y="15" fill="#{$colors['right']}" fill-opacity=".3">{$right}</text>
 <text x="98.5" y="14">{$right}</text>
 </g>
</svg>
SVG;
}

---

Side-notes:

Before you say anything, this is primarly opinion-based and not objective!

---

I don't think that useData is a good name.

I strongly recommend to change it to lowercase_and_underscore (a.k.a. snake_case).

Why is that? If you write usedata by mistake, you will have an hard time to look into "Where in the living fudge is this declared???" just to notice that you have misspelled the name of the function and that PHP doesn't care about casing in the function name.

If you write USE_DATA, Use_Data or any variation, it is easier to find the name. Implicitly you split the name by the _ and compare part by part.
Try this experiment:

1. Compare aVeryInterestingMethodWellSpelled with averyinterestingmethodwellspelled.
2. Compare a_Very_Interesting_Method_Well_Spelled with a_very_interesting_method_well_spelled.

Which one is easier to compare?

---

I disagree with https://softwareengineering.stackexchange.com/questions/196416/whats-the-dominant-naming-convention-for-variables-in-php-camelcase-or-undersc on using camelCase for this exact point.

Also, PHP itself doesn't follow this! Look at all the function names.

---

But, even if you change the name to use_data, it will be a bad name.

Why is that? Well, the name gives the idea that you are trying to use some data to do something. But what is it doing? I don't know, I have to read the whole function to know.

My recomendation: print_svg.
It shows preciselly what the code does: it outputs SVG. Simple.

• CodesInChaos: I strongly disagree with your 6th suggestion, if there is a bug that outputs something before you want to set the header, triggering another error/warning is better than choosing to malfunction silently.
• Ismael Miguel: @CodesInChaos I've changed it. I still keep the 6th point there, but it isn't part of the final code anymore. It's still a valid opinion. I'm just not 'enforcing' it anymore.
• Dmitri Zaitsev: Brilliant point about _ vs CamelCase - much cleaner and much more readable!