ns.Model.removeData() #177

Open
chestozo opened this Issue Nov 13, 2013 · 16 comments

Projects

None yet

3 participants

@chestozo
Member

Нужен метод для удаления данных по jpath.
Просто сделать model.set('.prop', undefined) недостаточно, ибо for in пройдётся по полю prop.

Сигнарута:

ns.Model.prototype.removeData = function(jpath, options) {};

Видимо, надо также триггерить ns-model-changed события (как делается в set()).

@edoroshenko
Contributor

ибо for in пройдётся по полю prop

Ром, напиши, пожалуйста подробнее

@chestozo
Member
var a = { prop: 7 };
for (var key in a) {
  console.log(key);
}
// "prop"

a.prop = undefined;
for (var key in a) {
  console.log(key);
}
// "prop"

delete a.prop;
for (var key in a) {
  console.log(key);
}
// ничего

Ну т.е. если я хочу удалить какие-то данные из модели по-настоящему - мне нужен delete.
Сейчас его нет (через set() не добиться реального удаления).

@edoroshenko
Contributor

Ок, согласен.
Только название немножко неконсистентное. Есть метод setData, который принимает данные. А removeData у тебя принимает jpath, как метод set. Так что логичнее назвать его просто remove. Но тут проблемка: remove уже есть у modelCollection и работает он с вложенными моделями. Вот такая задачка )

Либо менять api modelCollection, либо придумать ещё какие-то правильные названия

@chestozo
Member

Ну или:

  • setData() заменить на data()
  • set() заменить на setData() )
@vitkarpov
Member

Возможно, имеет смысл оставить тот интерфейс, который уже есть — set, только изменить реализацию самого метода. Я имею ввиду, что в клиентском коде я могу писать model.set('.prop', undefined), но при этом в таком случае реально будет происходить delete.

@edoroshenko
Contributor

Тоже тема. Думаю, что у нас в проекте это ничего не поломает, т.к. мы не сетим осознанно в поле undefined.

@chestozo
Member

@vitkarpov думал про это тоже, но посчитал это неявным действием (а вдруг ты захочешь вот прямо, чтобы там было undefined? хотя, понятно, редкий кейс).

@vitkarpov
Member

Есть другой тип данных, который может быть использован для «обнуления», а не удаления значения — null. По-моему, достаточно просто принять, что undefined работает для set именно так и задекларировать, чтобы если этот странный кейс случился программист понимал, что происходит: чем в отличаются null и undefined в этом контексте.

@vitkarpov
Member

@chestozo по-моему, в данном случае полиморфизм — это только плюс. Идеальный интерфейс — когда его нет, а его функция выполняется.

@chestozo
Member

задекларировать

Ты имеешь ввиду коммент написать? )

@vitkarpov
Member

Ну да, я имею ввиду написать в «документацию», в раздел про методы моделей ;)

@vitkarpov
Member

Шучу. Просто договориться, разумеется :)

@chestozo
Member

Это да, но, как ты понимаешь, самое крутое, что может быть, это когда метод делает то, что ты ожидаешь и не более.
В принципе, в jquery уже есть места, где это работает (кажется, $('div').css('display', null) просто сбросит свойство display).
Так что, может и ок, если мы просто будем реагировать на value == null (и на null и на undefined, в общем).

@edoroshenko
Contributor

а как вам метод clear?

@chestozo
Member

Мне кажется лучше remove / delete, или removeData / deleteData.
clear / clean это что-то про чистку, а у мы хотим удалить, вроде как.

@vitkarpov
Member

Я не против ничего, разумеется :)

Почему расширение поведения метода set мне показалось логичным, попробую объяснить на примере:

var foo = {};
foo.bar = 1;
console.log(foo.bar); // 1
delete foo.bar;
console.log(foo.bar); // undefined

Кажется логичным, что когда ты хочешь затереть какое-то поле в структуре модели, нужно просто присвоить ему значение undefined. Разумеется, все понимают, что в джаваскрипте нельзя просто присвоить значение undefined какой-либо переменной, чтобы полностью стереть ее из памяти. Это как раз и может сбивать с толку — в этом минус.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment