#javascript #refactoring
#javascript #Рефакторинг
Вопрос:
В настоящее время я настраиваю приложение с тремя отдельными кнопками, каждая из которых должна случайным образом выбирать элемент из массива, специфичного для этой кнопки. Я успешно закодировал ее с отдельными функциями для каждой кнопки, но мне было интересно, есть ли способ объединить ее в единую функцию, которая может применяться ко всем трем кнопкам.
Это мой текущий Javascript:
const greyButton = document.querySelector('#grey');
greyButton.addEventListener('click', () => {
let grey = ['Statblocks/Grey/badger.png', 'Statblocks/Grey/giantrat.png', 'Statblocks/Grey/badger.png', 'Statblocks/Grey/boar.png', 'Statblocks/Grey/panther.png', 'Statblocks/Grey/gitant badger.png', 'Statblocks/Grey/dire wolf.png', 'Statblocks/Grey/giant elk.png']
for (i=0;i<grey.length;i ){
let greyBalls = grey[Math.floor(Math.random() * grey.length)];
document.getElementById('greyBall').src = greyBalls;
}
});
const rustButton = document.querySelector('#rust');
rustButton.addEventListener('click', () => {
let rust = ['Statblocks/Rust/rat.png', 'Statblocks/Rust/owl.png', 'Statblocks/Rust/mastiff.png', 'Statblocks/Rust/goat.png', 'Statblocks/Rust/giant goat.png', 'Statblocks/Rust/giant boar.png', 'Statblocks/Rust/lion.png', 'Statblocks/Rust/brown bear.png']
for (i=0;i<rust.length;i ){
let rustBalls = rust[Math.floor(Math.random() * rust.length)];
document.getElementById('rustBall').src = rustBalls;
}
});
const tanButton = document.querySelector('#tan');
tanButton.addEventListener('click', () => {
let tan = ['Statblocks/Tan/jackal.png', 'Statblocks/Tan/ape.png', 'Statblocks/Tan/baboon.png', 'Statblocks/Tan/axe beak.png', 'Statblocks/Tan/black bear.png', 'Statblocks/Tan/giant weasel.png', 'Statblocks/Tan/giant hyena.png', 'Statblocks/Tan/tiger.png']
for (i=0;i<tan.length;i ){
let tanBalls = tan[Math.floor(Math.random() * tan.length)];
document.getElementById('tanBall').src = tanBalls;
}
});
И подключенный HTML:
<div class="row">
<div class="column">
<h1>Grey Bag of Tricks</h1>
<button class="button" id='grey'>Draw from the Bag</button>
<img src="" alt="" id="greyBall">
</div>
<div class="column">
<h1>Rust Bag of Tricks</h1>
<button class="button" id='rust'>Draw from the Bag</button>
<img src="" alt="" id="rustBall">
</div>
<div class="column">
<h1>Tan Bag of Tricks</h1>
<button class="button" id='tan'>Draw from the Bag</button>
<img src="" alt="" id="tanBall">
</div>
</div>
Комментарии:
1. Если вы сведете их в одну функцию, эта единственная функция должна будет иметь внутри условную логику для выполнения разной логики для каждого пути, поскольку каждый элемент имеет свой собственный набор URL-адресов. Вы могли бы это сделать, но я думаю, что это просто сделало бы его менее читаемым.
2. Отлично, спасибо! Я не был уверен, возможно ли это сделать, и если необходимая логика снизит читаемость, тогда я думаю, что оставлю это как есть.
3. Изменяя разметку, вы могли бы упростить ее сокращение. Позвольте мне посмотреть, что я могу придумать.
4. @Taplar Ему нужна одна функция, которую он может вызывать три раза с разными аргументами, что является полностью подходящим решением
Ответ №1:
var urlsByColor = {
grey: ['Statblocks/Grey/badger.png', 'Statblocks/Grey/giantrat.png', 'Statblocks/Grey/badger.png', 'Statblocks/Grey/boar.png', 'Statblocks/Grey/panther.png', 'Statblocks/Grey/gitant badger.png', 'Statblocks/Grey/dire wolf.png', 'Statblocks/Grey/giant elk.png'],
rust: ['Statblocks/Rust/rat.png', 'Statblocks/Rust/owl.png', 'Statblocks/Rust/mastiff.png', 'Statblocks/Rust/goat.png', 'Statblocks/Rust/giant goat.png', 'Statblocks/Rust/giant boar.png', 'Statblocks/Rust/lion.png', 'Statblocks/Rust/brown bear.png'],
tan: ['Statblocks/Tan/jackal.png', 'Statblocks/Tan/ape.png', 'Statblocks/Tan/baboon.png', 'Statblocks/Tan/axe beak.png', 'Statblocks/Tan/black bear.png', 'Statblocks/Tan/giant weasel.png', 'Statblocks/Tan/giant hyena.png', 'Statblocks/Tan/tiger.png']
};
function changeBall (e) {
const urls = urlsByColor[e.target.id];
const randomIndex = Math.floor(Math.random() * urls.length);
const randomUrl = urls[randomIndex];
const associatedBall = e.target.closest('.column').querySelector('.ball');
console.log(`change ${e.target.id} to ${randomUrl}`);
associatedBall.src = randomUrl;
console.log(associatedBall);
}
[...document.querySelectorAll('.color.button')].forEach(button =>
button.addEventListener('click', changeBall)
);
<div class="row">
<div class="column">
<h1>Grey Bag of Tricks</h1>
<button class="button color" id='grey'>Draw from the Bag</button>
<img src="" alt="" class="ball" id="greyBall">
</div>
<div class="column">
<h1>Rust Bag of Tricks</h1>
<button class="button color" id='rust'>Draw from the Bag</button>
<img src="" alt="" class="ball" id="rustBall">
</div>
<div class="column">
<h1>Tan Bag of Tricks</h1>
<button class="button color" id='tan'>Draw from the Bag</button>
<img src="" alt="" class="ball" id="tanBall">
</div>
</div>
Итак, несколько вещей изменилось.
- Я извлек списки URL-адресов на карту, которая может использовать идентификатор кнопки для поиска используемых URL-адресов
- Разметка для кнопок и шаров изменена, чтобы иметь общие классы для поиска и поиска
- Когда происходит щелчок, мы получаем URL-адреса по идентификатору кнопки
- Затем мы получаем случайный индекс, как это было сделано раньше
- Мы получаем случайный связанный URL-адрес для этого индекса
- Мы находим шар, который контекстуально связан с кнопкой классом родительского столбца
- И, наконец, мы меняем URL-адрес шара
Комментарии:
1. Что-то, что я замечаю, это то, что сейчас иногда возвращает пустое изображение. Не уверен, как это исправить.
Ответ №2:
В общем, вы помещаете аналогичную повторяющуюся часть в функцию, затем вы абстрагируете все части, которые отличаются в каждом использовании, заменяя их параметрами функции:
function addHandler(buttonSelector, ballImages, ballId) {
const button = document.querySelector(buttonSelector);
button.addEventListener('click', e => {
for (let i=0; i<ballImages.length; i ) {
let ball = ballImages[Math.floor(Math.random() * ballImages.length)];
document.getElementById(ballId).src = ball;
}
});
}
чтобы вы могли вызвать это как
addHandler('#grey', ['Statblocks/Grey/badger.png', 'Statblocks/Grey/giantrat.png', 'Statblocks/Grey/badger.png', 'Statblocks/Grey/boar.png', 'Statblocks/Grey/panther.png', 'Statblocks/Grey/gitant badger.png', 'Statblocks/Grey/dire wolf.png', 'Statblocks/Grey/giant elk.png'], 'greyBall');
addHandler('#rust', ['Statblocks/Rust/rat.png', 'Statblocks/Rust/owl.png', 'Statblocks/Rust/mastiff.png', 'Statblocks/Rust/goat.png', 'Statblocks/Rust/giant goat.png', 'Statblocks/Rust/giant boar.png', 'Statblocks/Rust/lion.png', 'Statblocks/Rust/brown bear.png'], 'rustBall');
addHandler('#tan', ['Statblocks/Tan/jackal.png', 'Statblocks/Tan/ape.png', 'Statblocks/Tan/baboon.png', 'Statblocks/Tan/axe beak.png', 'Statblocks/Tan/black bear.png', 'Statblocks/Tan/giant weasel.png', 'Statblocks/Tan/giant hyena.png', 'Statblocks/Tan/tiger.png'], 'tanBall');
Тем не менее, это все еще некоторое дублирование. Мы можем избежать этого, выполнив дополнительную обработку данных, чтобы мы могли передавать более простые значения:
function addHandler(id, imageNames) {
const buttonSelector = '#' id;
const ballImages = imageNames.map(name => `Statblocks/${id[0].toUpperCase() id.slice(1)}/${name}.png}`);
const ballId = id 'Ball';
… // rest as before
}
addHandler('grey', ['badger', 'giantrat', 'badger', 'boar', 'panther', 'gitant badger', 'dire wolf', 'giant elk']);
addHandler('rust', ['rat', 'owl', 'mastiff', 'goat', 'giant goat', 'giant boar', 'lion', 'brown bear']);
addHandler('tan', ['jackal', 'ape', 'baboon', 'axe beak', 'black bear', 'giant weasel', 'giant hyena', 'tiger']);
Наконец, я бы изменил querySelector
на getElementById
поиск и удалил ненужный цикл, и мы пришли к
function addHandler(id, imageNames) {
const ballImages = imageNames.map(name => `Statblocks/${id[0].toUpperCase() id.slice(1)}/${name}.png}`);
const button = document.getElementById(id);
button.addEventListener('click', e => {
let ball = ballImages[Math.floor(Math.random() * ballImages.length)];
document.getElementById(id 'Ball').src = ball;
});
}
addHandler('grey', ['badger', 'giantrat', 'badger', 'boar', 'panther', 'gitant badger', 'dire wolf', 'giant elk']);
addHandler('rust', ['rat', 'owl', 'mastiff', 'goat', 'giant goat', 'giant boar', 'lion', 'brown bear']);
addHandler('tan', ['jackal', 'ape', 'baboon', 'axe beak', 'black bear', 'giant weasel', 'giant hyena', 'tiger']);