gotchaphpMinor
Steam Community Market Strange Part Scraper
Viewed 0 times
marketcommunitypartsteamstrangescraper
Problem
A while back I wrote a simple little PHP script that searches the Steam community market for any TF2 strange weapons with strange parts on the first page of results for that weapon type. It works by retrieving the listings for each weapon using
I'd like to get some feedback on how I can make it cleaner/clearer/better. In particular, the big
curl, runs a regex to retrieve the internal JS variable containing the item metadata, and parses the JSON and looks in the descriptions for any strange part strings:#!/usr/bin/php
"Scattergun",
1 => "Pistol",
2 => "Rocket%20Launcher",
3 => "Direct%20Hit",
4 => "Shotgun",
5 => "Flame%20Thrower",
6 => "Flare%20Gun",
7 => "Reserve%20Shooter",
8 => "Axtinguisher",
9 => "Grenade%20Launcher",
10 => "Loch-n-Load",
11 => "Stickybomb%20Launcher",
12 => "Eyelander",
13 => "Minigun",
14 => "Tomislav",
15 => "Dalokohs%20Bar",
16 => "Killing%20Gloves%20of%20Boxing",
17 => "Pomson%206000",
18 => "Widowmaker",
19 => "Rescue%20Ranger",
20 => "Frontier%20Justice",
21 => "Wrangler",
22 => "Sniper%20Rifle",
23 => "Bazaar%20Bargain",
24 => "SMG",
25 => "Jarate",
26 => "Bushwacka",
27 => "Revolver",
28 => "Ambassador",
29 => "Knife",
);
for($wep_counter = 0; $wep_counter
I'd like to get some feedback on how I can make it cleaner/clearer/better. In particular, the big
$weapons array at the beginning is super ugly; is there a way I could have that be more compact? Perhaps something with an explode?Solution
Not exactly a PHP expert but your
You do not need an associative array for this. In fact, I would discourage this. Although there may be no good reason to do this, imagine you want to add a weapon in the middle, then you have to change a lot of numbers.
Similarly, I do not know how PHP internally represents associative arrays, but they may be slower to access than a standard array. C++ implementations often use Red-Black Trees to implement something similar to associative arrays and other languages may use other data structures.
Just use the standard array:
Use a foreach loop:
becomes:
There could be other issues but I'm not too much of a PHP expert, but these issues pertain to a lot of languages. I would definitely be sure to keep these in mind when using other languages as well.
$weapons array:$weapons = array(
0 => "Scattergun",
1 => "Pistol",
2 => "Rocket%20Launcher",
3 => "Direct%20Hit",
.
.
.
29 => "Knife",
);You do not need an associative array for this. In fact, I would discourage this. Although there may be no good reason to do this, imagine you want to add a weapon in the middle, then you have to change a lot of numbers.
Similarly, I do not know how PHP internally represents associative arrays, but they may be slower to access than a standard array. C++ implementations often use Red-Black Trees to implement something similar to associative arrays and other languages may use other data structures.
Just use the standard array:
$weapons = array(
"Scattergun",
"Pistol",
"Rocket%20Launcher",
"Direct%20Hit",
.
.
.
"Knife",
);Use a foreach loop:
for($wep_counter = 0; $wep_counter < count($weapons); $wep_counter++) {
$url = "http://steamcommunity.com/market/listings/440/Strange%20". $weapons[$wep_counter]; # get listing for that weapon typebecomes:
foreach ($weapons as $weapon) { # I don't believe you are mutating a value so I don't believe the "&" prefix is necessary.
$url = "http://steamcommunity.com/market/listings/440/Strange%20". $weapon;There could be other issues but I'm not too much of a PHP expert, but these issues pertain to a lot of languages. I would definitely be sure to keep these in mind when using other languages as well.
Code Snippets
$weapons = array(
0 => "Scattergun",
1 => "Pistol",
2 => "Rocket%20Launcher",
3 => "Direct%20Hit",
.
.
.
29 => "Knife",
);$weapons = array(
"Scattergun",
"Pistol",
"Rocket%20Launcher",
"Direct%20Hit",
.
.
.
"Knife",
);for($wep_counter = 0; $wep_counter < count($weapons); $wep_counter++) {
$url = "http://steamcommunity.com/market/listings/440/Strange%20". $weapons[$wep_counter]; # get listing for that weapon typeforeach ($weapons as $weapon) { # I don't believe you are mutating a value so I don't believe the "&" prefix is necessary.
$url = "http://steamcommunity.com/market/listings/440/Strange%20". $weapon;Context
StackExchange Code Review Q#127113, answer score: 2
Revisions (0)
No revisions yet.