זהו נושא דיון מלווה לערך המקורי שב־https://www.tocode.co.il/bundles/frontend/lessons/ajax-lab
מצרפת התרגיל הראשון:
1.) אשמח להערות על התרגיל.
2.) הטעינה של הדמויות אורכת אצלי הרבה זמן - האם יש דרך יותר יעילה?
3.) הכתובת הזו אינה תקינה: https://swapi.co/api/people/1/
צריך לשנות אותה ל : https://swapi.dev/api/people/1/
תודה רבה!!!
HTML
<!DOCTYPE html>
<html>
<head>
<meta charset="UTF8"/>
<title>Traning 1</title>
<link rel="stylesheet" href="https://maxcdn.bootstrapcdn.com/bootstrap/4.0.0/css/bootstrap.min.css" integrity="sha384-Gn5384xqQ1aoWXA+058RXPxPg6fy4IWvTNh0E263XmFcJlSAwiGgFAW/dAiS6JXm" crossorigin="anonymous">
<link rel="stylesheet" href="style.css"/>
</head>
<body>
<script src="https://code.jquery.com/jquery-3.2.1.slim.min.js" integrity="sha384-KJ3o2DKtIkvYIK3UENzmM7KCkRr/rE9/Qpg6aAZGJwFDMVNA/GpGFF93hXpG5KkN" crossorigin="anonymous"></script>
<script src="https://cdnjs.cloudflare.com/ajax/libs/popper.js/1.12.9/umd/popper.min.js" integrity="sha384-ApNbgh9B+Y1QKtv3Rn7W3mgPxhU9K/ScQsAP7hUibX39j7fakFPskvXusvfa0b4Q" crossorigin="anonymous"></script>
<script src="https://maxcdn.bootstrapcdn.com/bootstrap/4.0.0/js/bootstrap.min.js" integrity="sha384-JZR6Spejh4U02d8jOt6vLEHfe/JQGiRRSQQxSfFWpi1MquVdAyjUar5+76PVCmYl" crossorigin="anonymous"></script>
<script src="stars.js"></script>
</body>
</html>
CSS
html {
font-size: 20px;
}
body {
height: 100vh;
display: grid;
grid-template-areas: "left right";
}
h1 {
margin-top: 50px;
color: white;
text-align: center;
margin-bottom: 50px;
font-size: 2rem;
}
ul {
display: flex;
flex-direction: column;
align-items: center;
list-style: none;
max-height: calc(100vh - 148px);
overflow: auto;
}
.list-group-item {
border: none;
}
.peoples .list-group-item {
background-color: black;
}
.peoples .list-group-item span {
margin-right: 20px;
}
.peoples li * {
color: white;
background-color: black;
}
.films li * {
color: black;
background-color: white;
}
.left-side {
grid-area: left;
width: 40vw;
height: 100vh;
background-color: black;
}
.right-side {
margin-top: 50px;
grid-area: right;
width: 60vw;
height: calc(100vh-50px);
background-color: white;
}
button:hover {
color: black;
background-color: white;
cursor: pointer;
}
JAVASCRIPT
class StarWars {
constructor() {
this.setupUi();
this.renderPepolesList();
this.peopleFilmsUrlsObj = {};
}
async doGetFetch(url) {
const response = await fetch(url);
if(response.status === 200 && response.headers.get('content-type').startsWith('application/json')) {
return await response.json();
}
}
setupUi() {
document.body.innerHTML = `
<div class="left-side">
<h1>Star Wars</h1>
<ul class="list-group peoples">
</ul>
</div>
<div class="right-side">
<ul class="list-group films">
</ul>
</div>
`;
this.peopleUl = document.querySelector('ul.peoples');
this.filmsUl = document.querySelector('ul.films');
}
async renderPepolesList() {
let peoplesListHtml = ``;
let response = await this.doGetFetch("http://swapi.dev/api/people/");
do {
let peoplesList = response.results;
for(let people of peoplesList) {
const name = people.name;
this.peopleFilmsUrlsObj[name] = people.films;
peoplesListHtml+=`
<li class="list-group-item">
<span>${name}</span>
<button>list of my films</button>
</li>
`;
}
this.peopleUl.addEventListener('click',this.renderFilms.bind(this));
response = response.next? await this.doGetFetch(response.next) : null;
} while(response);
this.peopleUl.innerHTML = peoplesListHtml;
}
async renderFilms(evt) {
evt.stopImmediatePropagation();
if(evt.target.tagName!=='BUTTON') return;
let filmsListHtml = ``;
for(let filmUrl of this.peopleFilmsUrlsObj[evt.target.previousElementSibling.innerHTML]) {
const film = (await this.doGetFetch(filmUrl)).title;
filmsListHtml+=`
<li class="list-group-item">
<span>${film}</span>
</li>
`;
};
this.filmsUl.innerHTML = filmsListHtml;
}
}
new StarWars();
לייק 1
הי
המבנה נראה טוב. שתי הפונקציות האחרונות (renderFilms ו renderPeoplesList) מרגישות קצת מבולגנות. יש תחושה שהן עושות יותר מדי דברים - גם למשוך את המידע מהשרת וגם לכתוב את ה HTML כדי להציג אותו. אני חושב שהייתי מפריד את המשימות לפונקציות נפרדות.
דבר נוסף בשיעור הזה דיברתי על Document Fragments:
https://www.tocode.co.il/bundles/frontend/lessons/dom-navigation?tab=video
דרך אחת לסדר את הקוד לפונקציות קטנות יותר היא לבנות Fragment ואז לתת לפונקציות הקטנות לבנות את האלמנטים עצמם בתוך ה Fragment.
דבר אחרון שנראה מוזר זה הקריאה ל addEventListener בתוך הלולאה. בדרך כלל נהוג לרכז קריאות כאלה במקום אחד למשל בפונקציה setupUi אצלך
לייק 1
תודה!
- בקשר ל-Fragment, מכירה אותו אבל היה נראה לי ארוך יותר, גם ליצור את האלמנטים, גם להוסיף קלאסים איפה שצריך וגם אחר כך לסדר אותם בתוך ה-Fragment, האם ישנה עדיפות להשתמש בה בדוקא, למרות שאת ה-innerHTML אני עושה פעם אחת?
- בקשר ל-addEventListener באמת מוזר לא יודעת כיצד השתרבב לשם…
ושוב תודה על ההיערות, תמיד מחכימות!
מצרפת את התרגיל השלישי , אשמח אם תוכל לעבור עליו - תודה רבה!!!
class Starships {
constructor() {
this.starShipsObj = {};
this.allPilotsArray = [];
this.currentGame = {
currentStarShipIndex:0,
currentRealPilotName:'',
};
this.setupUi();
this.start();
}
async doGetFetch(url) {
try {
const response = await fetch(url);
if(response.status === 200 && response.headers.get('content-type').startsWith('application/json')) {
return await response.json();
}
} catch(err) {
console.log(err);
}
}
setupUi() {
document.body.innerHTML = `
<div class="left-side">
<h1>Starships</h1>
<div class="container">
<div class="starShip">
<label>StarShip name</label>
<select multiple class="custom-select" size="10">
</select>
</div>
<div class="pilots">
<label>Pilots</label>
<select multiple class="custom-select" size="10">
</select>
</div>
</div>
</div>
<div class="right-side">
<p>
<span> Choose pilot... </span>
<span class="win"></span>
</p>
<button> new starship </button>
</div>
`;
this.starShipNameSelectElement = document.querySelector('.starShip select');
this.pilotsSelectElement = document.querySelector('.pilots select');
this.winElement = document.querySelector('.right-side p .win');
document.querySelector('.right-side button').addEventListener('click', () => {
if(!this.starShipsObj.starShipsArray) return;
this.clearSelects();
this.displayNewStarShip();
});
this.pilotsSelectElement.addEventListener('change' , this.checkChosePilot.bind(this));
}
clearSelects() {
this.starShipNameSelectElement.innerHTML = '';
this.pilotsSelectElement.innerHTML = '';
this.winElement.innerHTML = '';
}
checkChosePilot(evt) {
if(evt.target.value === this.currentGame.currentRealPilotName) {
this.winElement.classList.add('green');
this.winElement.innerHTML = 'Very true - you won !!!';
} else {
this.winElement.classList.remove('green');
this.winElement.innerHTML = 'Oops mistake try again';
}
}
randomFromZero(countOfNumbers) {
return Math.floor(Math.random()*countOfNumbers);
}
existsInRealPilotsArray(pilotName) {
if(this.starShipsObj.starShipsArray[this.currentGame.currentStarShipIndex].pilots.findIndex(pilot=>pilot===pilotName) !==-1)
return true;
return false;
}
getRandomNames() {
const names = [...Array(4)];
let randomPilot = this.randomFromZero(this.allPilotsArray.length);
let randomIndexToPush = this.randomFromZero(4);
for(let i=0; i<=3; i++) {
while(names.findIndex(name => this.allPilotsArray[randomPilot]===name) !==-1 || this.existsInRealPilotsArray(this.allPilotsArray[randomPilot])) randomPilot = this.randomFromZero(this.allPilotsArray.length);
while(names[randomIndexToPush]) randomIndexToPush = this.randomFromZero(4);
names[randomIndexToPush] = i!==3 ? this.allPilotsArray[randomPilot] : this.currentGame.currentRealPilotName;
}
return names;
}
fillSelectElement(innerOption , currentSelect) {
const option = document.createElement('option');
option.innerHTML = innerOption;
option.value = innerOption;
currentSelect.appendChild(option);
}
randomOneRealPilotName() {
this.currentGame.currentRealPilotName = this.starShipsObj.starShipsArray[this.currentGame.currentStarShipIndex].pilots[this.randomFromZero(this.starShipsObj.starShipsArray[this.currentGame.currentStarShipIndex].pilots.length)];
}
fillPilotsElement() {
this.randomOneRealPilotName();
const names = this.getRandomNames();
names.forEach(name=>{
this.fillSelectElement(name , this.pilotsSelectElement);
});
}
fillStarShipElement() {
this.fillSelectElement(this.starShipsObj.starShipsArray[this.currentGame.currentStarShipIndex].name , this.starShipNameSelectElement);
}
lotteryStarShipThatHasPilots() {
this.currentGame.currentStarShipIndex = Math.floor(Math.random()*this.starShipsObj.starShipsCount);
while(!this.starShipsObj.starShipsArray[this.currentGame.currentStarShipIndex].pilots.length) this.currentGame.currentStarShipIndex = Math.floor(Math.random()*this.starShipsObj.starShipsCount);
}
displayNewStarShip() {
this.lotteryStarShipThatHasPilots();
this.fillStarShipElement();
this.fillPilotsElement();
}
addCurrentPilotToPilotsArray(name) {
if(this.allPilotsArray.findIndex(pilot=>pilot===name)===-1) {
this.allPilotsArray.push(name);
}
}
async getPilots(urlsPeople) {
const pilotsNames = [];
urlsPeople.forEach(async urlPilot => {
pilotsNames.push((await this.doGetFetch(urlPilot)).name);
this.addCurrentPilotToPilotsArray(pilotsNames[pilotsNames.length-1])
});
return pilotsNames;
}
async getStarShipsArray() {
let response = await this.doGetFetch('http://swapi.dev/api/starships/');
const starShipsArray = [];
do {
response.results.forEach(async starshipObj => {
starShipsArray.push({'name': starshipObj.name , 'pilots': await (this.getPilots(starshipObj.pilots)),});
});
response = response.next ? await this.doGetFetch(response.next) : null;
} while(response);
return starShipsArray;
}
async initializationStarshipsObj() {
this.starShipsObj = {
starShipsCount: (await this.doGetFetch("http://swapi.dev/api/starships/")).count,
starShipsArray: await this.getStarShipsArray(),
};
}
async start() {
await this.initializationStarshipsObj();
this.displayNewStarShip();
}
}
const starShip = new Starships();
הי,
אני מאחד תשובות שיהיה יותר קל - בנושא ה Fragments מה שהם בעיקר מאפשרים לנו זה לחלק את העבודה למספר פונקציות או למספר קלאסים ואז למשל אפשר לשבור את הפונקציה renderPeoplesList לכמה פונקציות קטנות יותר: אחת תמשוך את המידע, והשניה תבנה DocumentFragment עבור פריט בודד ותחזיר אותו, ואולי עוד אחת תיצור DocumentFragment עבור כל הרשימה וכך אנחנו בונים את הקוד מחלקים קטנים יותר שיותר קל לקרוא אותם. לכל חלק יש את התפקיד שלו. ה DocumentFragment זו פשוט דרך ליצור חלק מהעץ ולהעביר אותו בין פונקציות (שהיא גם יותר מהירה ומאובטחת מהעברת מחרוזת).
לגבי השני ההערה דומה - הקוד בכל פונקציה טוב אבל יש תחושה שאפשר היה לחלק את זה ליותר מחלקות או ליותר פונקציות. הנה דוגמה קטנה הפונקציה addCurrentPilotToPilotsArray. היא מדויקת, יש לה שם הגיוני וגם ממש לא ארוכה, ובכל זאת כשאני קורא את הקוד שלה:
addCurrentPilotToPilotsArray(name) {
if(this.allPilotsArray.findIndex(pilot=>pilot===name)===-1) {
this.allPilotsArray.push(name);
}
}
אז קשה להבין את המטרה והחשיבות שלה. אני יכול להבין איך הגעתי לכתוב אותה, אבל כשקוראים את זה היא לא אומרת כלום.
הייתי מתחיל ארגון מחדש של הקוד עם חלוקה אחרת למחלקות ואוביקטים, למשל אוביקט בשם pilotsRepository שהממשק שלו היה נראה משהו כזה:
const pilotsRepository = {
// get a list of all pilots in the system
initialize() { ... }
// returns the name of a random pilot from a spaceship
getPilot(spaceship_id) { ... }
// returns the name of a random pilot NOT flying a given spaceship
getPilotNotFrom(spaceship_id) { ... }
};
אם את ממלאת את המימושים של הפונקציות את מקבלת אוביקט שאפשר להשתמש בו בקוד שלך ושיש לו תפקיד מוגדר. כל פונקציה בו מספרת סיפור ומסבירה מאוד טובה מה היא עושה, בלי קשר לאיפה קראנו לה או לא קראנו לה בתוך הקוד.
נסי לסיים את כתיבת האוביקט ולשלב אותו בקוד שלך, ואחרי זה תראי אם את מוצאת עוד אוביקטים או מחלקות כאלה שאפשר לכתוב וכך לתת יותר משמעות לקוד.
לייק 1