Skip to content

Basic auction optimization of code and null checks #226

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
vishwasrvalke wants to merge 3 commits into
base: develop
Choose a base branch
from
Open

Basic auction optimization of code and null checks #226

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
50a796c
Basic auction optimization of code and null checks
vishwasrvalke Jul 24, 2021
217258b
Updated the url
vishwasrvalke Jul 24, 2021
6cd8789
Merge branch 'develop' into feature/optimising-auctions
vishwasrvalke Sep 20, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
191 changes: 97 additions & 94 deletions components/Auction/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,43 +11,25 @@ const WALLET_URL = `${BASE_API_URL}/wallet`;

const HandleAuctions = () => {
const [auctionsData, setAuctionsData] = useState([]);
const [userBid, setUserBid] = useState();
const [userBid, setUserBid] = useState(0);
const [isUserLoggedIn, setIsUserLoggedIn] = useState(false);
const [isLoading, setIsLoading] = useState(true);
const [userMoney, setUserMoney] = useState();

useEffect(() => {
fetchAndSetAuctions();
}, []);

useEffect(() => {
getUserWallet();
}, []);

useEffect(() => {
fetchSelfDetails()
.then((res) => {
if (res.status === 200) {
setIsUserLoggedIn(true);
}
})
.catch((err) => {
console.log('User is not logged in', err);
});
}, []);
const [userMoney, setUserMoney] = useState(0);

const fetchAndSetAuctions = async () => {
const response = await fetchData(AUCTIONS_URL);
const json = await response.json();
setAuctionsData(json.auctions);
if (!response) return null;
setAuctionsData(
JSON.parse(response && response.auctions ? response.auctions : 0) || 0
Copy link

@correaswebert correaswebert Jul 26, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could use the new syntax here!
JSON.parse(response.auctions ?? 0)
It mostly won't need the fallback || 0 as well 😄

);
setIsLoading(false);
};

const getUserWallet = async () => {
const response = await fetchData(WALLET_URL, 'GET', {
credentials: 'include',
});
const { status } = await response;
const { status } = response;
if (status === 200) {
const { wallet } = await response.json();
if (Object.keys(wallet).length === 0) return setUserMoney(0);
Expand All @@ -59,9 +41,26 @@ const HandleAuctions = () => {
}
} else {
setUserMoney(0);
return null;
}
};

useEffect(() => {
(async () => {
Copy link

@correaswebert correaswebert Jul 26, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please create named functions!
Reading and understanding stack traces would be far easier and better 😄

async function funcname = () => {}
funcname()

Copy link
Author

@vishwasrvalke vishwasrvalke Aug 7, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's an IIFE ,please recheck the implementation,thanks

await fetchAndSetAuctions();
await getUserWallet();
await fetchSelfDetails()
.then((res) => {
Copy link

@correaswebert correaswebert Jul 26, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please don't mix the .then().catch() syntax with async-await if not needed.
You can wrap the block in a try-catch as any of the above requests could also fail 😓

if (res.status === 200) {
setIsUserLoggedIn(true);
}
})
.catch((err) => {
console.error('User is not logged in', err);
});
})();
}, []);

const handleNewBid = async (e, auctionId) => {
e.preventDefault();
if (!isUserLoggedIn) {
Expand All @@ -86,7 +85,7 @@ const HandleAuctions = () => {
);
const { status } = await response;
if (status === 201) {
fetchAndSetAuctions();
await fetchAndSetAuctions();
setIsLoading(false);
}
}
Expand All @@ -108,78 +107,82 @@ const HandleAuctions = () => {
e.target.src = '/assets/default_avatar.jpg';
};

const auctionHandler = auctionsData.map((auction) => {
const { id, seller, quantity, highest_bid, bidders } = auction;
return (
<div className={`${styles.auctionContainer} ${id}`} key={id}>
<div className={styles.auctionSeller}>
<h2>Seller:</h2>
<img
className={styles.profilePhoto}
src={`${BASE_IMAGE_URL}/${seller}/img.png`}
onError={brokenImageHandler}
/>
</div>
<div className={styles.auctionStats}>
<div className={styles.itemInfo}>
<h1>
{quantity} x {' '}
<Image
className={styles.gemImage}
layout="fixed"
src="/assets/gem.png"
width={25}
height={25}
/>
</h1>
</div>
<div className={styles.currentStatus}>
<h2>
Current Bid:
<div className={styles.currentBidPrice}>{highest_bid}</div>
</h2>
</div>
</div>
<div>
<form
className={styles.bidOptions}
onSubmit={(e) => handleNewBid(e, id)}
>
<input
className={styles.inputBid}
type="number"
min={parseInt(highest_bid) + 1}
required={true}
onBlur={({ target: { value } }) =>
validateBid(value, highest_bid)
}
/>
<button type="submit" className={styles.bidBtn}>
Bid
</button>
</form>
</div>
<div className={styles.bidders}>
{bidders.map((bidder) => {
return (
<div
className={styles.biddersImg}
key={bidder}
data-columns={getColumns(bidders.length)}
title={bidder}
>
const auctionHandler =
auctionsData && auctionsData.length
Copy link

@correaswebert correaswebert Jul 26, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use the auctionsData?.length syntax, it's simpler 😄

? auctionsData.map(({ id, seller, quantity, highest_bid, bidders }) => {
Copy link

@correaswebert correaswebert Jul 26, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please create a sub-component which you can then use in the ternary operator...
Easier to read and understand the fallback, which in this case we have to scroll a lot to find 😅

return (
<div className={`${styles.auctionContainer} ${id}`} key={id}>
<div className={styles.auctionSeller}>
<h2>Seller:</h2>
<img
className={styles.profilePhoto}
src={`${BASE_IMAGE_URL}/${bidder}/img.png`}
onError={brokenImageHandler}
src={`${BASE_IMAGE_URL}/${seller}/img.png`}
onError={(e) => brokenImageHandler(e)}
/>
</div>
);
})}
</div>
</div>
);
});
<div className={styles.auctionStats}>
<div className={styles.itemInfo}>
<h1>
{quantity} x {' '}
<Image
className={styles.gemImage}
layout="fixed"
src="/assets/gem.png"
width={25}
height={25}
/>
</h1>
</div>
<div className={styles.currentStatus}>
<h2>
Current Bid:
<div className={styles.currentBidPrice}>{highest_bid}</div>
</h2>
</div>
</div>
<div>
<form
className={styles.bidOptions}
onSubmit={(e) => handleNewBid(e, id)}
>
<input
className={styles.inputBid}
type="number"
min={parseInt(highest_bid) + 1}
required={true}
onBlur={({ target: { value } }) =>
validateBid(value, highest_bid)
}
/>
<button type="submit" className={styles.bidBtn}>
Bid
</button>
</form>
</div>
<div className={styles.bidders}>
{bidders.length
? bidders.map((bidder) => {
return (
<div
className={styles.biddersImg}
key={bidder}
data-columns={getColumns(bidders.length)}
title={bidder}
>
<img
className={styles.profilePhoto}
src={`${BASE_IMAGE_URL}/${bidder}/img.png`}
onError={brokenImageHandler}
/>
</div>
);
})
: null}
</div>
</div>
);
})
: null;
Copy link

@correaswebert correaswebert Jul 26, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use the inline && operator to avoid the null condition 😄


return (
<div className={styles.mainContainer}>
Expand Down