May 22nd, 2020 Join this week code review organized by Corvid master, Comment your code below(as per instruction) we will analyze provide you suggestion or refactored code that will be easy to read, to modify, and plenty of good practice.
We also welcome experience corvid forum members to share the knowledge and expertise.
Instructions:
*1: FULL PAGE CODE is recommended
*2: Include database structure like Collection name & Field type (if you refer them in the code)
3: Is this a live site? If so, drop a link so, we can check the code.
4: Videos or Images for explanation of what the code does is useful!
Marked * are required
Don’ts
1: Do NOT include any sensitive code (like passwords, secrect key).
// try to rewrite it as xxx if it's used inside the code
const key = "XXX-XXX";
2: Asking any questions unrelated to code review or corvid
3: Screenshot the code and not pasting the code in text format
4: Code that was not working before and asking to fix (Please create a new post in the community, This Post is dedicated to improve your current working code)
The code works, just made it work actually. Wondering if there is a better way to do it. Thanks!
import wixData from 'wix-data';
export function text32_viewportEnter(event) {
//this is so that the counter only starts when the right box comes into view
//In the query below I have included everything under the query to ensure the variables are recognised. And I used the async in the results of the first query and then the await for the second query.
//2 Listings one for cats and another one for dogs. Searching on the status field to count records that are "Adopted"
wixData.query("CatListings")
.eq("status", "Adopted")
.count()
.then(async (results) => {
let x = results; // total adopted Cats
await wixData.query("DogListings")
.eq("status", "Adopted")
.count()
.then( (result) => {
let y = result; // total adopted Dogs
// the initial Dog and Cat end numbers are historic figures
let CatstartNum = 1;
let CatendNum = 5+x;
let DogstartNum = 1;
let DogendNum = 6+y;
let TotstartNum = 1;
let TotendNum = DogendNum + CatendNum;
const duration = 50; // 1000 milliseconds
$w.onReady(function () {
setInterval(()=> {
countUp();
}, duration);
});
function countUp(){
if (CatstartNum <= CatendNum ){
$w('#text67').text = CatstartNum.toLocaleString().toString();
CatstartNum++;
}
if (DogstartNum <= DogendNum){
$w('#text69').text = DogstartNum.toLocaleString().toString();
DogstartNum++;
}
if (TotstartNum <= TotendNum ){
$w('#text75').text = TotstartNum.toLocaleString().toString();
TotstartNum++;
}
}
})
})
}
You are in the right place. let me refactor the code for you
If selecting an element by id in your code always rename the id based on the element and the purpose
like #textCountCat #textCountDog
So, just reading code you will know what type of element it is and whatit should be doing
I will refactor in 2 mins
SetInterval and Promise can cause lot of performance issue if not used right
If you are using Promise and if the result of promise a does not depend to call the promise b, you can call both promise at the same time without blocking each other
In your case if the cat query takes 2 sec and dog query take 3 sec
then the promise have to wait for 5 sec
But using Promise all we can wait for the promise to resolve at the same time
so it will take 3 sec.
Alway clearInterval when using setInterval after it’s done, if not it will continuously run and slow down the site.
import wixData from 'wix-data';
// Creat an interval in global so, we can call it outside scope
// and stop from running after the count down end or viewport leave
let interval;
$w.onReady(function () {
$w('#text32').onViewportEnter(() => {
// if there is an existing interval, remove that first
if(interval) {
clearInterval(interval);
}
initCount();
});
});
//this is so that the counter only starts when the right box comes into view
//In the query below I have included everything under the query to ensure the variables are recognised. And I used the async in the results of the first query and then the await for the second query.
//2 Listings one for cats and another one for dogs. Searching on the status field to count records that are "Adopted"
async function initCount() {
// countCat"P" stands for Promise, just for Indication
const countCatP = wixData.query("CatListings").eq("status", "Adopted").count();
const countDogP = wixData.query("DogListings").eq("status", "Adopted").count();
// Using Promise .all so, we will wait for to resolve both the promise
// at same time, without blocking each other
const [ countCat, countDog ] = await Promise.all([ countCatP, countDogP ]);
console.log({countCat, countDog});
// the initial Dog and Cat end numbers are historic figures
let CatstartNum = 1;
let CatendNum = 5 + countCat;
let DogstartNum = 1;
let DogendNum = 6 + countDog;
let TotstartNum = 1;
let TotendNum = DogendNum + CatendNum;
const duration = 50; // 1000 milliseconds
interval = setInterval(countUp, duration);
function countUp() {
const hasEndedCat = CatstartNum > CatendNum; // return true or false
const hasEndedDog = DogstartNum > DogendNum; // return true or false
const hasEndedTotal = TotstartNum > TotendNum; // return true or false
if ( !hasEndedCat ) {
$w('#text67').text = CatstartNum.toLocaleString().toString();
CatstartNum++;
}
if ( !hasEndedDog ) {
$w('#text69').text = DogstartNum.toLocaleString().toString();
DogstartNum++;
}
if ( !hasEndedTotal ) {
$w('#text75').text = TotstartNum.toLocaleString().toString();
TotstartNum++;
}
// stop the interval from running when all the count is ended
if( hasEndedCat && hasEndedDog && hasEndedTotal ) {
if(interval) clearInterval(interval);
}
}
}
Closing the thread
hope this was useful,
We(Corvid master) will do Code review on every Friday ,
get ready for the next code review on 29th May 2020
Happy coding
