Code Review / Refactor for .csv import

I am importing a .csv file and map the .csv columns to a specific database column. You can change the array key to change the mapping for the database.

This is my current code:


        /*
         * Get the uploaded .csv file
         */
        $inputFileName = __DIR__ . '/data.csv';

        /*
         * Create an instance of the .csv Reader and load the
         * uploaded .csv file
         */
        $spreadsheet = IOFactory::load($inputFileName);

        /*
         * Create an array from the given instance
         */
        $sheetData = $spreadsheet->getActiveSheet()->toArray(null, true, true, true);

        /*
         * We're setting an empty array to map our
         * .csv columns
         */
        $mapping = array();

        /*
         * Loop trough $sheetData and put the columns
         * to the specific array key
         */
        foreach ($sheetData as $column) {
            /*
             * Create the mapping array to modify the columns
             * which are inserted to the database
             */
            $mapping [] = array(
                "A" => $column['A'],
                "B" => $column['B'],
                "C" => $column['C'],
                "D" => $column['D'],
                "E" => $column['E'],
                "F" => $column['F'],
                "G" => $column['G'],
                "H" => $column['H'],
                "I" => $column['I'],
                "J" => $column['J'],
                "K" => $column['K'],
                "L" => $column['L']
            );
        }

Is there any cleaner way to archive this, or is this fine?

lostdreamer_nl

7 years ago

Level 53

I'd probably go for less comments and more expressive code.

    public function handle()
    {
        $sheetData = $this->getSpreadsheetDataFromCsv(__DIR__ . '/data.csv');
        
        $mappedData = array_map([$this, 'mapDataFromRow'], $sheetData);
        
        dd($mappedData);
    }

    private function getSpreadsheetDataFromCsv($filePath)
    {
        $spreadsheet = IOFactory::load($filePath);
        return $spreadsheet->getActiveSheet()->toArray(null, true, true, true);
    }

    private function mapDataFromRow($row)
    {
        return [
            "A" => $row['A'],
            "B" => $row['B'],
            "C" => $row['C'],
            "D" => $row['D'],
            "E" => $row['E'],
            "F" => $row['F'],
            "G" => $row['G'],
            "H" => $row['H'],
            "I" => $row['I'],
            "J" => $row['J'],
            "K" => $row['K'],
            "L" => $row['L']
        ];
    }

Not much change, but by having every method really small, it makes your main code almost as readable as english.

Also, you dont have to change you comments every time you change your code.

1 like

Please or to participate in this conversation.