[PHP]"problem plecakowy" - usprawnienie kodu Opcje

[mike287]

Cześć,
dostałem zadanie ostatnio do rozwiązania tzw. "problem plecakowy", poniżej zamieszczam kod. Usłyszałem że jest zbyt mało obiektowy... czy macie jakieś wskazówki co można poprawić, albo jak do tego podejść ?
Chodzi mi o jakieś wskazówki, bo patrze na to i nie wiem co mam rozumieć przez za mało obiektowy kod...

informacje o przedmiotach jakie mam "wsadzic do plecaka zaciagam z pliku csv, gdzie w pierwszym wierszu sa nazwy kolumn, ktorych nie potrzebuje wiec dlatego napisane jest w kodzie tak by to pominąć.

[PHP] pobierz, plaintext 
 
<?php
 
class BagTask
{
    private $arr;
    private $count;
    private $finalArr = [];
    private $weight;
    private $bagArr;
 
    public function __construct($arr, $weight)
    {
        $this->arr = file($arr);
        $this->weight = $weight;
 
        $this->countOfItemsInArray();
        $this->prepareArray();
        $this->changeKeyNames();
        $this->sort();
        $this->toBag();
        $this->summary();
        $this->showBag();
 
//        var_dump($this->bagArr);
    }
 
/**
 * Metoda countOfItemsInArray zwraca ilosc wierszy w tablyc arr (tam są przedtrzymywane dane z pliku csv)
 */
 
    public function countOfItemsInArray()
    {
        $this->count = count($this->arr);
        return $this->count;
    }
 
/**
 * Metoda prepareArray tworzy tablice finalArr, wyrzuca "," między elementami i tworzy nowy wspólczynnik waga/wartość na 3 pozycji.
 */
    public function prepareArray()
    {
 
        $partArr = [];
        $finalArr = [];
 
        for($i = 1; $i < $this->countOfItemsInArray(); $i++)
        {
            array_push($partArr, explode(",", $this->arr[$i]));
            array_push($finalArr, $partArr[$i-1]);
 
            array_push($finalArr[$i-1], $partArr[$i-1][2] / $partArr[$i-1][1]);
 
        }
        return $this->finalArr = $finalArr;
    }
 
/**
 * Metoda changeKeyNames nadaje kluczom nazwy, zamiast liczb.
 */
    public function changeKeyNames(){
 
        $this->finalArr = array_map(function($arr) {
 
            return array(
                'id' => $arr[0],
                'weight' => $arr[1],
                'value' => $arr[2],
                'W/V' => $arr[3]
            );
        }, $this->finalArr);
 
    }
 
/**
 * Metoda sort sortuje tablice tablic po współczynniku W/V od największego.
 */
    public function sort(){
 
        usort($this->finalArr, function ($item1, $item2) {
            if ($item1['W/V'] == $item2['W/V']) return 0;
            return $item1['W/V'] > $item2['W/V'] ? -1 : 1;
        });
 
    }
 
/**
 * Metoda toBag uzupełnia tablice plecaka pobierając z posegregowanej tablicy finalArr elementy
 * na wejściu sprawdzając wagę elementu i zestawia ją z limitem jaki pozostał.
 */
    public function toBag()
    {
        $bagLimit = 0;
        $bagArr = [];
        for ($i = 0; $i < $this->countOfItemsInArray()-1; $i++)
        {
            if($this->finalArr[$i]['weight'] <= $this->weight)
            {
               $bagLimit += $this->finalArr[$i]['weight'];
 
                    if ($bagLimit <= $this->weight)
                    {
                        array_push($bagArr, $this->finalArr[$i]);
                    }
            }
        }
 
        return $this->bagArr = $bagArr;
    }
 
/**
 * Metoda countOfItemsInTheBag, podaje ilość elementów wsadzonych do plecaka
 */
 
    public function countOfItemsInTheBag()
    {
        $countOfItemsInTheBag = count($this->bagArr);
 
        return $countOfItemsInTheBag;
    }
 
/**
 * Metoda weightOfBag, zwraca wagę plecaka
 */
 
    public function weightOfBag()
    {
        $weight = 0;
        foreach ($this->bagArr as $key)
        {
            $weight += $key['weight'];
        }
            return $weight;
    }
 
/**
 * Metoda valueOfBag, zwraca wartość plecaka
 */
    public function valueOfBag()
    {
        $value = 0;
        foreach ($this->bagArr as $key)
        {
            $value += $key['value'];
        }
        return $value;
    }
 
/**
 * Metoda showBag, zwraca informacje wyjściowe na temat plecaka
 */
    public function showBag()
    {
        $i = 0;
        foreach ($this->bagArr as $value)
        {   $i++;
 
              echo "przedmiot ".$i." o id ".$value['id']." jego waga - ".$value['weight']." wartość to ".$value['value']."<br>";
 
        }
    }
 
/**
 * Metoda summary, wyświetla informacje na temat przedmiotów w plecaku
 */
    public function summary()
    {
        echo "liczba elementów znajdujących się w plecaku to ".$this->countOfItemsInTheBag()."<br>";
        echo "limit plecaka = ".$this->weight."<br>";
        echo "wykorzystana waga plecaka = ".$this->weightOfBag()."<br>";
        echo "całkowita wartość plecaka = ".$this->valueOfBag()."<br>";
 
    }
 
}
 
$bag = new BagTask('csvArray.csv', 1);
 
[PHP] pobierz, plaintext 

[Tomplus]

Swoją drogą, problem plecakowy dotyczy bardziej pojemności kontenerów, a nie ilości przedmiotów w kontenerze.

Ja w takich przypadkach posługuję się gotową klasą:
https://github.com/dvdoug/BoxPacker

[mike287]

zastanawia mnie co w tym kodzie jest takiego ze jest "zbyt mało obiektowy", widzialem tez roznego rodzaju rozwiazania problemu plecakowego, ale napisalem swoj i chcialem sie poradzic co tu mozna zmienic zeby byl lepszy

[rad11]

Poczytaj o https://pl.m.wikipedia.org/wiki/SOLID_(prog...anie_obiektowe) te wiadomosci napewno pozwola Ci zrobic z Twojego kodu bardziej obiektowy

[emillo91]

Możliwe, że chodzi tutaj o dużą ilość pętli. Na przykład metoda toBag mogłaby być od razu implementowana w metodzie countOfItemsInTheBag(). Wszystko wyświetlasz w konstruktorze i może o to chodzi.

[markuz]

Masz 1 klasę a na szybko narzucają się przynajmniej 2 - Bag, Element (chociaż nazwa element jest trochę za ogólna i przydało by się ją skrócić do konkretnego problemu np. Apple jeżeli w Bag możemy trzymać tylko jabłka, ew. przedmiot).

W konstruktorze jako arr podajesz nazwę pliku CSV - czyli przy zmianie sposobu wczytywaniu danych trzeba zmieniać twoją klasę - tam mógłby być jakiś interfejs np. Source + implementacja Source z CSV np. CSVSource dzięki czemu masz już 4 klasy.

Co tam robi zakomentowany var_dump?
Dlaczego nie używasz PHP 7.1?

Nie ma żadnego wyjątku a jak np. bag się przepełni to powinien zostać rzucony wyjątek.

Patrząc na wnętrze konstruktora już wiadomo, że ten kod coś nie halo