3
\$\begingroup\$

iextrading has two APIs (v1, v2) which provide financial market data. Following script records a JSON file with their equity information (using a CRON job).

Would you be kind and review it for any possible improvement or anything that may be incorrect?

<?php


date_default_timezone_set("UTC");
ini_set('max_execution_time' , 0);
ini_set('memory_limit','-1');
set_time_limit(0);

$i = new I();
I::scrapeAllStocks($i);

Class I
{


    /**
    *
    * @var an integer for version of iextrading api
    */

    private $version; 

    /**
    *
    * @var a string of iextrading symbols
    */

    private $symbolsPath; 

    /**
    *
    * @var a string of our symbols json directory
    */

    private $symbolsDir; 

    /**
    *
    * @var a string of target path and query
    */

    private $targetQuery; 

    /**
    *
    * @var a string of iextrading base URL
    */

    private $baseUrl; 

    /**
    *
    * @var a string of iextrading end point
    */

    private $endPoint; 

    // For version 1, tokens are not required. 
    // SECRET TOKEN: *********************************
    // PUBLISHABLE TOKEN: *********************************

    ## https://cloud.iexapis.com/beta/iex/tops?symbols=AAPL&token=*********************************

    function __construct()
    {
        $this->version = 2;

        $this->symbolsPath = __DIR__ . "/../../config/symobls.md";
        $this->symbolsDir = __DIR__ . "/../../a/path/to/your/dir/symbols";
        $this->targetQuery = "stock/market/batch?symbols=";

        // baseUrl for version 1

        $this->baseUrl = "https://api.iextrading.com/1.0/";
        // baseUrl for version 2
        // $this->baseUrl = "https://cloud.iexapis.com/beta/";

        // endPoint for version 1
        $this->endPoint = "&types=quote,chart&range=1m&last=10";
        // endPoint for version 2 
        // $this->endPoint = "&token=*********************************&types=quote,chart&range=1m&last=10";

        echo "YAAAY! Class I is running 💚\n";
        return true;
    }



    public static function symbs($i){
        $fnm= $i->symbolsPath;
        $cnt= file_get_contents($fnm);
        $sym=preg_split('/\r\n|\r|\n/', $cnt);
        $child=array();
        $mother=array();
        $c=100;
        foreach ($sym as $k=>$v){
            $c=$c-1;
            $sym=preg_split('/[\t]/', $v);
            array_push($child,$sym);

            if($c<=0){
                $c=100;
                array_push($mother, $child);
                $child=array();
            }

        }
      return $mother;
    }

    public static function scrapeAllStocks($i){
        $vStocks=I::symbs($i);
        $baseUrl=$i->baseUrl.$i->targetQuery;

        $currentTime=date("Y-m-d-H-i-s");
        $allStocks=array();
        foreach ($vStocks as $k=>$v) {
            $s=array_column($v, 0);
            $stockUrl=$baseUrl . implode(",", $s) . $i->endPoint;
            $rawStockJson=file_get_contents($stockUrl);
            $rawStockArray=json_decode($rawStockJson, true);
            $allStocks=array_merge($allStocks, $rawStockArray);
        }

        $allStocksJson=json_encode($allStocks);

        // Write the raw file
        $symbolsDir= $i->symbolsDir;

        if (!is_dir($symbolsDir)) {mkdir($symbolsDir, 0755,true);}

        $rawStockFile=$symbolsDir . "/" . $currentTime . ".json";
        $fp=fopen($rawStockFile, "x+");
        fwrite($fp, $allStocksJson);
        fclose($fp);
        echo "YAAAY! stock large json file updated successfully! 💚 \n";
    }


}

?>

Example of symobls.md:

A   2019-01-04  AGILENT TECHNOLOGIES INC
AA  2019-01-04  ALCOA CORP
AAAU    2019-01-04  PERTH MINT PHYSICAL GOLD ETF
AABA    2019-01-04  ALTABA INC
AAC 2019-01-04  AAC HOLDINGS INC
AADR    2019-01-04  ADVISORSHARES DORSEY WRIGHT
AAL 2019-01-04  AMERICAN AIRLINES GROUP INC
AAMC    2019-01-04  ALTISOURCE ASSET MANAGEMENT
AAME    2019-01-04  ATLANTIC AMERICAN CORP
AAN 2019-01-04  AARON'S INC
AAOI    2019-01-04  APPLIED OPTOELECTRONICS INC
AAON    2019-01-04  AAON INC
AAP 2019-01-04  ADVANCE AUTO PARTS INC
AAPL    2019-01-04  APPLE INC
AAT 2019-01-04  AMERICAN ASSETS TRUST INC
AAU 2019-01-04  ALMADEN MINERALS LTD - B
AAWW    2019-01-04  ATLAS AIR WORLDWIDE HOLDINGS
AAXJ    2019-01-04  ISHARES MSCI ALL COUNTRY ASI
AAXN    2019-01-04  AXON ENTERPRISE INC
AB  2019-01-04  ALLIANCEBERNSTEIN HOLDING LP
ABAC    2019-01-04  RENMIN TIANLI GROUP INC
ABB 2019-01-04  ABB LTD-SPON ADR
ABBV    2019-01-04  ABBVIE INC
ABC 2019-01-04  AMERISOURCEBERGEN CORP
ABCB    2019-01-04  AMERIS BANCORP
ABDC    2019-01-04  ALCENTRA CAPITAL CORP
ABEO    2019-01-04  ABEONA THERAPEUTICS INC
ABEOW   2019-01-04  
ABEV    2019-01-04  AMBEV SA-ADR
ABG 2019-01-04  ASBURY AUTOMOTIVE GROUP
ABIL    2019-01-04  ABILITY INC
ABIO    2019-01-04  ARCA BIOPHARMA INC
ABM 2019-01-04  ABM INDUSTRIES INC
ABMD    2019-01-04  ABIOMED INC
ABR 2019-01-04  ARBOR REALTY TRUST INC
ABR-A   2019-01-04  
ABR-B   2019-01-04  
ABR-C   2019-01-04  
ABT 2019-01-04  ABBOTT LABORATORIES
ABTX    2019-01-04  ALLEGIANCE BANCSHARES INC
ABUS    2019-01-04  ARBUTUS BIOPHARMA CORP
AC  2019-01-04  ASSOCIATED CAPITAL GROUP - A
ACA 2019-01-04  ARCOSA INC
ACAD    2019-01-04  ACADIA PHARMACEUTICALS INC
ACB 2019-01-04  AURORA CANNABIS INC
ACBI    2019-01-04  ATLANTIC CAPITAL BANCSHARES
ACC 2019-01-04  AMERICAN CAMPUS COMMUNITIES     
\$\endgroup\$
  • 2
    \$\begingroup\$ I rolled back your last edit. After getting an answer you are not allowed to change your code anymore. This is to ensure that answers do not get invalidated and have to hit a moving target. If you have changed your code you can either post it as an answer (if it would constitute a code review) or ask a new question with your changed code (linking back to this one as reference). Refer to this post for more information \$\endgroup\$ – Sᴀᴍ Onᴇᴌᴀ Feb 26 at 19:53
1
\$\begingroup\$

Why instantiate an object of class I if the only methods called are static? It appears that only private member variables that are never mutated are accessed. Instead of creating an object of class I just to pass to the static methods, you could use class constants or if you really want them kept private, use make those private variables static.

That way, the line the instance (i.e. $i = new I();) can be removed and the parameters (e.g. $i) can be removed from the method signatures, since they are no longer needed. It is up to you if you keep the constructor and destructor (e.g. if you end up needing to instantiate objects of that type then maybe you want those).


The class name I is a little vague/non-descriptive. Perhaps a more appropriate name would be something like StockController or StockScraper. Similarly the method name symbs could be better named - perhaps something like getSymbols.


What does the value 100 signify when used in that symbs method? Were you using 100 or more lines in your symbols.md file? If 100 is a special value, it should be stored in a constant or private static property.

I tried using the sample file which has 47 lines and thus when I tried the code, nothing was added to the array returned by symbs(). Perhaps the logic needs to be updated to handle array lengths lower than 100.


I presume there is a typo on this line:

$this->symbolsPath = __DIR__ . "/../../config/symobls.md";

given you gave a sample

Example of symobls.md:

So perhaps that line should be:

$this->symbolsPath = __DIR__ . "/../../config/symbols.md";

The last few lines of scrapeAllStocks() uses fopen(), fwrite() and fclose() to write the output file - is there a reason not to use file_put_contents() instead of all of those? Maybe you aren't familiar with that function, or else there is some write issue I am unaware of.

\$\endgroup\$
1
\$\begingroup\$
  1. You cannot use a return in a __construct() method. If you need to check for any "badness", you can use throw/try/catch.
    Relevant resources: Return false from __constructor & PHP - constructor function doesn't return false (In the end, I support Sam's advice about writing class constants.)
  2. Regarding your regex patterns, they can be simplified.
    • /\r\n|\r|\n/ is more simply expressed as /\R/ (I expect you will never need to contend with a solitary \r -- you could have also used /\r?\n/.)
    • /[\t]/ does not need to be expressed inside of a character class, so the square brackets can be omitted. (/\t/)
  3. Now I am going to recommend that you completely abandon the contents of your symbs() method. Using regex is a sub-optimal tool for parsing your tab-delimited .md files. Using a combination of file() with iterative calls of str_getcsv() and a declaration of the your delimiting character makes for a concise and robust one-liner. So brief that you may choose to avoid writing the dedicated method entirely.

    public static function symbs($i) {
        return array_map(function($line){ return str_getcsv($line, "\t"); }, file($i->symbolsPath)));
    }
    // I tested this code locally using your .md file to make sure it worked as expected
    
  4. Try to avoid single-use variable declarations. There are some cases where declaring variables improves the readability, but something straight forward like a datetime string generator is pretty safe to just inject into its final location in your script.

  5. It is best to keep your variable declarations in reasonable proximity to the place(s) they are used. This will save you and other developers from having to do too much scanning to find what they need.
  6. It would be a good idea for you to get familiar with PSR Coding Standards. Most relevant to this question, please obey the PSR-2 recommendations.
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.