Будет ли забиваться память при таком коде и правильно ли организован класс?

Question

[Артур]

Подскажите правильно ли организован псевдо код? И будут ли утечки памяти при setInteraval или сборщик будет вовремя все очищать и ничего не засорится?

По всевдо задаче нужно отправлять новые блоки каждую минуту на сервер. В методе create блоки могут возвращаться каждый раз разные.

class Block {
  constructor(color) {
    this._currentBlock = null;
    this.color = color || '';
  }

  current() {
    return this._currentBlock;
  };

  create() {
    return new Promise((resolve, reject) => {
      setTimeout(() => {
        this._currentBlock = {type: 'Brick', color: this.color};
        return resolve(this._currentBlock);
      },0);
    });
  };

  prepareForSend(block) {
    return block.color = block.color.toUpperCase();
  };

  send(block) {
    fetch('http://test.com', {
      method: 'POST',
      body: JSON.stringify(block),
    })
    .catch(console.log)
  }

  init() {
    setInterval(() => {
      const block = new Block();
      block.create().then(r => {
        const preparedBlock = block.prepareForSend(block.current());
        block.send(preparedBlock);
      });
    }, 10000);
  }
}

const block = new Block();
block.init();

Обновленный вариант класса

class Block {
  constructor(color) {
    this.color = color || '';
  }

  create() {
    return new Promise((resolve, reject) => {
      setTimeout(() => {
        return resolve({type: 'Brick', color: this.color});
      },0);
    });
  };

  prepareForSend(block) {
    return block.color = block.color.toUpperCase();
  };

  initSending() {
    setInterval(() => {
      this.create().then(block => {
        const preparedBlock = this.prepareForSend(block);
        send(preparedBlock);
      });
    }, 10000);
  }
}

function send(block) {
  fetch('http://test.com', {
    method: 'POST',
    body: JSON.stringify(block),
  })
  .catch(console.log)
}

const block = new Block();
block.initSending();

Comments

[Kovalsky]

Может остальным все понятно, а для меня тут какая-то сатана необъяснимая происходит. Если отбросить глобальные вопросы типа "зачем нужно чтоб блок посылал себя сам" и "почему функция отправки не реализована вне класса", то остаётся еще много:
1. Зачем в create вообще таймаут,
2. Зачем в экземпляре хранится current_block?
3. Зачем еще один блок создаётся в init?
4. Вопрос связан с предыдущим: зачем нужно внутри метода класса создавать еще один экземпляр этого класса?
5. Зачем create отдаёт current_block который нигде не используется?

Отвечая на ваш вопрос: я вижу тут кучу вроде как лишних действий, но не вижу ничего, что бы не мог собрать GC. Есть какие-то конкретные причины думать, что GC не справится? Собственно, я даже проверил (это можно сделать за несколько секунд из консоли), память не забивается

[Артур]

Спасибо за развернутые вопросы. Как раз не мог понять в чем мои косяки.
Постараюсь ответить как я рассуждал при написании.
1. Обратил внимание что функция асинхронная и создание блока заимает время.
2. Действительно, не зачем, переписал код. (Хотел иметь возможность получить последний созданный блок)
3. Действительно, не зачем, переписал код с использованием this. (Когда писал думал что нужен экземпляр для использования методов, затупил)
4. тоже что и в 3.
5. Он писался в свойство, последнего созданного блока, по сути тоже не зачем, переписал.

Такой код будет спроектирован верно? Подскажите что еще по вашему мнению переписать?

class Block {
  constructor(color) {
    this.color = color || '';
  }

  create() {
    return new Promise((resolve, reject) => {
      setTimeout(() => {
        return resolve({type: 'Brick', color: this.color});
      },0);
    });
  };

  prepareForSend(block) {
    return block.color = block.color.toUpperCase();
  };

  initSending() {
    setInterval(() => {
      this.create().then(block => {
        const preparedBlock = this.prepareForSend(block);
        send(preparedBlock);
      });
    }, 10000);
  }
}

function send(block) {
  fetch('http://test.com', {
    method: 'POST',
    body: JSON.stringify(block),
  })
  .catch(console.log)
}

const block = new Block();
block.initSending();

[Kovalsky]

Артур, вынесу в ответ

[Артур]

Никита Полевой, спс.

обновленный код достаточен или есть еще что следует изменить?

Answer 1

[Kovalsky]

Может остальным все понятно, а для меня тут какая-то сатана необъяснимая происходит. Если отбросить глобальные вопросы типа "зачем нужно чтоб блок посылал себя сам" и "почему функция отправки не реализована вне класса", то остаётся еще много:
1. Зачем в create вообще таймаут,
2. Зачем в экземпляре хранится current_block?
3. Зачем еще один блок создаётся в init?
4. Вопрос связан с предыдущим: зачем нужно внутри метода класса создавать еще один экземпляр этого класса?
5. Зачем create отдаёт current_block который нигде не используется?

Отвечая на ваш вопрос: я вижу тут кучу вроде как лишних действий, но не вижу ничего, что бы не мог собрать GC. Есть какие-то конкретные причины думать, что GC не справится? Собственно, я даже проверил (это можно сделать за несколько секунд из консоли), память не забивается

ЗЫ: если отвечаете, то нажимайте кнопку "ответить", а то я ваш ответ чисто случайно заметил. И код лучше закатать либо в спойлеры, либо в интерактивные примеры в песочнице (codepen, jsfiddle).

Comments

[Kovalsky]

Спасибо за развернутые вопросы. Как раз не мог понять в чем мои косяки.
Постараюсь ответить как я рассуждал при написании.
1. Обратил внимание что функция асинхронная и создание блока заимает время.
2. Действительно, не зачем, переписал код. (Хотел иметь возможность получить последний созданный блок)
3. Действительно, не зачем, переписал код с использованием this. (Когда писал думал что нужен экземпляр для использования методов, затупил)
4. тоже что и в 3.
5. Он писался в свойство, последнего созданного блока, по сути тоже не зачем, переписал.

1. Так это вы её сделали асинхронной. Все используемые вами функции там синхронные, так что можно убрать таймаут без всяких проблем,

Дело в том, что я толком не знаю что происходит, поэтому вряд ли смогу подсказать что-то по делу. Например я в упор не понимаю зачем существует create, ведь сам конструктор по сути и должен заниматься созданием. Кроме того этот самый create вместо того чтоб создавать объект (что следовало бы из его названия) просто возвращает хардкодные данные. Опять же, я не знаю правильно ли я понял, но я бы сделал так:

class Block {
  constructor(data) {
    if (!data.type) {
      throw new Error('No type passed in Block constructor!');
    }

    this.color = data.color || '';
    this.type = data.type;
  }

  prepareForSending() {
    this.color = this.color.toUpperCase();

    return this;
  };
}

function send(data) {
  fetch('http://test.com', {
    method: 'POST',
    body: JSON.stringify(data),
  })
  .catch(console.log)
}

function initSending(data) {
  setInterval(() => {
    send( data );
  }, 10000);
}

const block = new Block({ type: 'brick', color: 'red' });

initSending( block.prepareForSending() );

[Артур]

Никита Полевой, в констукторе класса можно сделать запрос в другое место?
Т.е при создании нового блока, 1 свойство берется из бекенда, например цвет.
Или так нельзя делать?
Собственно для этого я и создвал метод create, чтобы в нем получить некоторые данные, затем уже манипулировать готовым блоком с полученным цветом и заданным руками типом.
Поскольку из конструктора нельзя вернуть промис (или можно?), пришлось добавить метод create.

Как только промис разрешися в create, значит блок полностью создан и его можно отправить на серв

class Block {
  constructor(data) {
    if (!data.type) {
      throw new Error('No type passed in Block constructor!');
    }

    this.type = data.type;
  }

  create() {
    fetch('http://test.com/color')
    .then(r => {
      this.color = r;
      return this;
    });
  }
}

function send(data) {
  fetch('http://test.com', {
    method: 'POST',
    body: JSON.stringify(data),
  })
  .catch(console.log)
}

function initSending(data, Construct) {
  setInterval(() => {
    const unit = new Contruct(data);
    unit.create().then(r => send(r))
  }, 10000);
}

initSending({type: 'Brick'}, Block);

(мб можно даже просто обновлять color вызывая create при каждом интервале, хотя наверно лучше создавать новый объект)

[Kovalsky]

в констукторе класса можно сделать запрос в другое место?

В целом можно. Конечно обычно все таки советуют делать init (как вы и сделали вначале), но если никаких последствий плохих не предвидится, то почему бы и нет.

Собственно для этого я и создвал метод create, чтобы в нем получить некоторые данные, затем уже манипулировать готовым блоком с полученным цветом и заданным руками типом.
Поскольку из конструктора нельзя вернуть промис (или можно?), пришлось добавить метод create.

Понял, дошло зачем нужен create.

мб можно даже просто обновлять color вызывая create при каждом интервале, хотя наверно лучше создавать новый объект

Ну вот да, на самом деле тут в принципе можно не создавать объект, тупо собирать строку, что будет быстрее и проще. Это вы практикуете ООП? Если суть просто в том, чтобы с десяток символов отправить, то можно и без класса же, а то с ним куча лишних действий добавляются:

Без классов

async function getRandomColorName() {
  const response = await fetch('http://www.colr.org/json/color/random').then(r => r.json());

  return response.colors[0].tags[0].name;
}

async function send(data) {
  const response = await fetch('https://reqres.in/api/users', {
    method: 'post',
    mode: 'cors',
    body: JSON.stringify(data),
  })
    .then(r => r.json())
    .catch(console.log);

  console.log( 'Data sent successfully, response: ', response );
}

function initSending(data, Construct) {
  setInterval(async () => {
    send({ type: 'brick', color: await getRandomColorName() });
  }, 10000);
}

initSending();

Но если нужен класс, то как-то так:

С классом

class Block {
  constructor(data) {
    if (!data.type) {
      throw new Error('No type passed!');
    }

    this.color = data.color || '';
    this.type = data.type;
  }

  prepareForSending() {
    this.color = this.color.toUpperCase();

    return this;
  }
}

async function getRandomColorName() {
  const response = await fetch('http://www.colr.org/json/color/random').then(r => r.json());

  return response.colors[0].tags[0].name;
}

async function send(data) {
  const response = await fetch('https://reqres.in/api/users', {
    method: 'post',
    mode: 'cors',
    body: JSON.stringify(data),
  })
    .then(r => r.json())
    .catch(console.log);

  console.log( 'Data sent successfully, response: ', response );
}

function initSending(data, Construct) {
  setInterval(async () => {
    const block = new Block({ type: 'brick', color: await getRandomColorName() });
    send(block.prepareForSending());
  }, 10000);
}

initSending();

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

[Артур]

Никита Полевой, Да, пробую решить таск в ООП стиле. Хотелось аакумулировать всю логику в классе, чтобы с ним потом можно было работать и в других случаях.
Поэтому всеже буду создавать новый блок каждый раз в интервале, потому что все же это разные блоки отправляются, а не один и тот же с новым цветом)

спасибо за варианты решения