Skip to content

UnifiedContainer clone fix #57

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 456 commits into from
Closed

UnifiedContainer clone fix #57

wants to merge 456 commits into from

Conversation

avid
Copy link

@avid avid commented Dec 23, 2011

Предлагаю diff по issue #7.
Переопределен метод __clone и добавлен тест на этот случай.

@dovg
Copy link
Member

dovg commented Dec 23, 2011

А это нормально, что мы клонируем dao, которое реализует паттерн Singleton?

@AlexeyDsov
Copy link
Member

Справедливости ради надо отметить что то dao которое кланируется совсем не синглтон.

@suquant
Copy link
Member

suquant commented Dec 23, 2011

Ну оно в контексте Singleton-а юзается )))

@AlexeyDsov
Copy link
Member

Что за контекст такой синглтона?

@dovg
Copy link
Member

dovg commented Dec 26, 2011

Вот этот пример #57 (comment) я считаю правильным.

Ниже ИМХО:
Мне очевидно, что оба контейнера ссылаются на одну и ту же копию объекта.

@crazedr0m
Copy link
Contributor

а если ....
$container1 = $products->getVariants();
$list1 = $container1->getList();

$products->dropVariants();

$container2 = $products->getVariants()->setCriteria(
Criteria::create()->add(
bla-bla-bla-expression(s)
)
);
$list2 = $container2->getList();

где
Product::dropVariants() {
$this->variants = null;
}

или сделайте фильтр на уже выбраное в $list1. я так понимаю там все а в $list2 - что-то.

На мой взгляд не удачный кейс.
и никто не отменял $products->getVariants()->clean() опять же после $list1 = $container1->getList();

@AlexeyDsov
Copy link
Member

Где во всех этих примерах выше клонирование?

@crazedr0m
Copy link
Contributor

я к тому, что пока не понял зачем вообще клонировать контейнер

@AlexeyDsov
Copy link
Member

По большому счету контейнер - это просто простой способ получить список объектов которые ссылаются на объект - владелец контейнера а так же сохранить-модифицировать этот список. При этом сейчас в нем использовать setCriteria опасно, т.к. в одном месте сделаешь что б список доставался с дополнительными условиями, а удалить потом не удалишь и в другом месте там где не ожидаешь и где не надо будешь получать список с этими дополнительными условиями...

Кстати, тут походу нужно в таком случае дописать метод __clone также и у UnifiedContainerWorker'а

@crazedr0m
Copy link
Contributor

@Neerrar @soloweb Если еще актуально, сделайте отдельную ветку и с неё пулл реквест.

@AlexeyDsov
Copy link
Member

Не могу сказать за авторов, но я все хочу добраться до этого кейса и все никак не добираюсь.

@avid
Copy link
Author

avid commented Jan 15, 2013

@crazedr0m новый pull request: #175

klkvsk and others added 18 commits June 7, 2013 11:50
Исправить пагинацию в списке магазинов, чтобы при переключении фильтра статуса сбрасывалась страница в пагинаторе
Поиск по названию или offer_id в списке магазинов выдаёт ошибку
В списке магазинов сделать фильтр статуса "Все", аналогично списку yml-файлов
В списке магазинов подсвечивать строку, над которой находится курсор
git-subtree-dir: lib/Fenom
git-subtree-split: 5c2685f
950175c fix FORCE_COMPILE should have priority and ignore storage
7a39a85 fix Render::isValid() should return true when times are equal
c8d668f fix _load(): check if cached Render is valid if AUTO_RELOAD is on

git-subtree-dir: lib/Fenom
git-subtree-split: 950175ce7325ffdb12304182a4b138191b6150c2
@dovg
Copy link
Member

dovg commented Nov 10, 2016

Этот pullrequest доставляет много писем в мою почту. Можно я его закрою?

@avid
Copy link
Author

avid commented Nov 10, 2016

Да, закрывай.
Сам уже закрыл.

@avid avid closed this Nov 10, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants