Reformulating + check for if correct
Soo I wanted to know if the code I put in image is correct (I am not yet in the position where I can test my code, I am still busy writing the file it's pulling the data from)
it should basically check an array pulled from a JS-file with data and compare it to another array (Being the choices made by the user) and see if atleast one of them match, if atleast 1matches it passes the test and it'll go to the next test, this is to ensure that the bingo won't have duplicates and won't have impossible options)
However, for some reason I have a feeling Imay have written it wrongly or that there is a way to write this so much more compact, it doesn't feel easy to read anymore
Can anyone tell me if the code indeed does as I intend it to do and if there is a way to write it differently so that it is more readable and accesable
55 Replies
I also just realise I should have a way to skip all other tests if one is false, how should I best handle this?
I am thinking of putting every test except from the first one in an IF-statement
Being but I feel this would be too repetitive and there must be a simpler way
I'm not sure why you're passing in
checksPassed
into the function if that's what the function return
s.
But here's how I would check if one array contains a number from a second array:
This function only does one thing: checks if the two arrays contain an identical number. You can use the result of it to either return from a parent function or to continue checking other things.
Another option would be a more clever one, but less readable:
This one makes a new Set
(that can only contain unique values) and compares the size (number of items) of the unique set vs the length of the combined two arrays. If they're different then there has to be at least one repeated number. For a longer array this one could potentially be less CPU work as the JS engine won't have to iterate through (potentially) the entire dozens- to hundreds-long array. But the performance gain on a smaller array of items.personally I think readability outdoes cleverness, you see
I want to eventually be a programmer but I know how difficult code can be sometimes to understand and read if you use clever solutions or wrongly named vars that's why I rather have readable code over complex code that needs a lot of comment line for others seeing the code to understand
oh the checkpasses being send down is bc in this iterations I made seperate function for each check but they'd still use the same value in the end so I thought I would have to send it down in every function
at the end of everything there will be a for-loop , if even 1 check is false then the for loop will repeat without creating an element
(bc that means its either a copy or it doesn't fit the criteriathe user has asked for)
in my previous iteration I had put everything in one function, it works but readability is impossible, I have to Reread my own code several times to understand again
so now that im going to expand I've decided to rewrite the entire code from scratch and have it more readable without putting a billion of actions in one massive function
but so you're saying I dont have to pass it down in order to use it in the functions?
also you spoke of cpu usage, one array would potentially be 1008 objects
and the other at max 100 and at minimum 9 (I will allow the user to decide size within these margins)
however I personally feel 1008 might be a bit much so im thinking if I can narrow it down in some way I have ro do some thinking on that part
No, you don't need to pass it down since that's what you're returning. You call each function and pass in the needed data (the two arrays in this case) and have it return a boolean of if it passed or not. If the return value is false,
return
from the enclosing function, otherwise move on to the next test.
Since I don't know what all you need to check I can't do much more than show a generic function with generic info, but something like this:
So you have one parent function that calls all the check functions. You then run the checks one at a time (each one returning a boolean) and if any of them fail you return false
to finish the function early and not run the rest of the test functions.Oh, okay
So do I need to place my checksPassed in then or can I keep it in my parent function?
It's kind of the first time that I'm using nested functions with variables being passed down, I used to always keep my variables in global scope if multiple functions needed them
There are3 things to check but they all just involve comparing arrays
Here's basically how it works, generally speaking
This function isn't finished yet, I also still have to add the other checks, I think there will be a total of 3-4 checks
where the fucntion "isUnique" is called is where all the other checks will be called underneath
But I need to basically skip all the other checks if the check abovereturns a false, should I just put them in an statement or is there a better way
checksPassed
shouldn’t be a variable at all is what I’m saying. The function returns true
if the check is passed, otherwise it returns false
. There’s no need to keep that value in any scope.Wait, I'm a little bit lost
So what has to happen is, the
For
loop runs, within the loop, firstly a random number between 1 - 1008 is generated
That number will be used for the checks
That number will be used to see if an Object fits the criteria
This is one of the objects, so say the random number returns a 1 it will compare itself to the [0] in the array of data
ARRAY OF DATA:
So in this example it would compare itself to this, first it will check if its already created in the arraylist that will be used to fill the bingo, (so the first one will never be a duplicate as the array is still empty and with every object that passes all the tests it will increase by one until the desired length is reached)
After that it will have to check if its available in the chosen games (Only the games where they are available I have put into the array)
When that is done there may be one other check, altho I am not at that point yet
If they have passed all the checks, a bingotile will be created and that number will be added to the bingotilesarray
So say this is the first one, then this will be the first object in the bingo tiles array
HOWEVER if one of the checks fails, it shouldn't create a tile and the for-loop also shouldn't progress
(This is why I didn't put a 1++ in the but rather at the end where it checks if checksPassed is true
So if there's a false it would re-iterate the forloop without adding +1 to "start"
how can I make this work if th checksPassed does not exist? Would it just start the FOR-loop from teh beginning again if it meets with a return false
?
I haven't used returns quite often yet as ive always put everything in one big code before
`
This part is what currently uses the checksPassed as the final part of the forloop, as you can see it will progress the for-loop with 1 and build a bingotile
How can I write this differently then so that the checksPassed is not required? as I don't want to nest multiple IF's together
If I were to do this without this, Ithink it would be like
i feel like that solution would lose a lot of its readability, I personally don't see any other solution without a dedicated variable to keep track of itWhat I'm saying is that you use the early return principal instead of keeping the
checksPassed
variable. If a check doesn't pass then you stop the function so it never gets to the end where you build the bingo tile. If all tests pass then you didn't return early and the buildBingoTile()
function is run as normal.
However, I think you need to rethink how you're doing this. Since you have the data and you want to do some randomization on some of the data you should instead look into array methods. For example, you can use Array.filter()
to create a new array with only the chosen elements in it, so that could be done on your Pokélist to only get the pokémon that are in the game you desire. Then you can shuffle the array and Array.pop()
to take an element one at a time until the bingo card is made.OH, so you are saying
return false
makes everything below in the current iteration ignored and it starts from the top?Also, for your entry object you can change the
availableInGames
value to be an array that only includes the games instead of game: true
.o putting them as string then you mean, that indeed sounds like a useful alternative
Yes, once a function hits a
return
it returns that value and stops. That's what return
does, it signals the end of the fuction.
Yes, so you can start out with filteredDexEntry = pokelist.filter( mon => mon.availableInGames.contains("letsgopikachu")
will be an array of pokémon that are available in Let's Go, Pikachu!
Array.filter MDN link: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/filter
A lot of for loops can be replaced with modern array methods. The three most important are filter()
(which returns a new array with equal or fewer items from the first), map()
(which returns a new array with the same number of items but changed somehow) and reduce()
(which returns any data type that can do almost anything to the array). forEach()
is the basic "do this function to every item in the array but don't return anything"Oh, okay so basically what I should do now is remove the
var checksPassed
Have every of the check functions hold a return, make it true if they have passed so that they go on to the next part of the for-loop-iteration
And false if the for loop has to start over?
Then at the bottom of the forloop I can just do for-loop-start-number + 1 and the bingo tile creation?
OH I see, thank you, that filtering would make it a whole lot easierAnd one of the cool things about array methods (besides
forEach()
) is that they return a new array, so you can chain them onto each other.I have to build an arrayfrom scratch tho, so I don't think I can replace my
for()
as its used to build teh array
unless..Okay I think it is possible after all
if I think about the things you just said, I can basically let a full array (as I would already know the .length
by the desired settings of the user) be created by using random numbers and have it run checks with the pokelist array and swap those out that don't fit the criteria
and then when that whole process is done it would build the entire bingo
instead of tile by tile
Am I on the right track or is what I am saying utterly dumb and impossible
or less efficientSo you could do what you're trying to do with just one giant array chain!
It's one function that works on one array and gives you a new one!
No
if
checks, no re-running in case a check fails, just one function that gives you what you want.that's hurting my head :((
Okay let me try to break it down just for myself, bc if I don't understand the code I'm writing then it's not a good idea to write it in my opinion;
So, randomList basically requires a game (would it also work with multiple games being chosen n it only having to fit in one?) and the length of the array , in other words the size of the bingotable
Then it would search through PokeList for every entry that includes the chosen game
After that it would fill it with a random number within the array? Im a bit lost on the first
.map
what does it do exactly? as the random number would be the dexID's of possible mon's, so mons that fit the criteria
yeh no, I'm lost from that point onThe
game
property is a single string of one game (if you want to pass in an array the code would need to change). The numOfEntries
is the final array size you want (so yes, it's the size of the bingo table).
The first .filter()
removes any pokémon that isn't available in the chosen game.
The first .map()
is taking the pokémon entry and making a new object that includes not only the original entry, but a random number (this is just a random number, not related to anything at all)
Then the .toSorted()
(had to change this as .sort()
changes the original array and doesn't return a new one) method sorts the entries based on that random number. Again, the random number is just Math.random()
, so a random number from 0 to not-1.
The last .map()
is taking the new object and removing the random number, so it's just the pokémon entry.
Then the .filter()
is making sure the final array is only as long as you want it to be.
Gotta go to work now, but I'd suggest reading up on the array methods on MDN to see if you can learn more of how they work.OH, I think I get it now
the random number is to decide the order
but the randomList is already using only available pokemon, I think this is the part I got lost on :p
From that view it indeed all makes sense
I will read up on it, thank you
One last thing: this above code is best viewed on desktop as I put a lot of white-space to align things to look better. On mobile it looks weird :p
Ohdon't worry, I am on laptop xd
If it were with multiple games instead of just one, how would that look?
should I then just iteratie this code for the amount of games and add the arrays together before adding a random number?
As in, iterate the first step, of filtering
and add the results into one big array
then continue with that big array for the other steps
No, you’ll want to modify the initial filter function. I’ll whip something up when I have a few moments
Alright, I thought it would work :((
Replace the first bit of the function with the below changes:
This filter function will include any Pokémon that are in at least one game passes into the function. The inner
for…of
loop loops through all the games that you want to include and checks if the Pokémon can bee found in that game. If it is, it’s included in the array that the rest of the chained methods use.So is it like this then?
FUNCTION:
(p.s. I get that at the first bit I have to fill in the array it is using, but what am I to fill in on "pokemon entries" ?)
DATAARRAY:
const RandomList
is a function, so you don't need to wrap it in another function. It's using arrow function syntax, but it's identical to
I use arrow syntax b/c I like the implicit returns and the fact that it's bound to a variable name and I can't accidentally overwrite my own functions when I run out of function name ideas :pahh yeh I haven't used arrow syntax yet xd
but so hold on as I feel ive gotten some bits wrong
so the first bit:
const pokeList = [pokelist of pokémon entries];
is simply the array of pokémon
All 151 (or however many there are now, I'm old and remember playing Pokémon Red on my GameBoy lol)and at
does this mean that I have to make the array within
pokelist
with all the games they appear in be named "game"
or how am I to read or implement this
theres 1008 now lol
but it gives an error at "pokemon entries"For the
RandomList
function to work properly, you need to pass in two argument:
1. gamesToInclude
is an array of games you want to include in the bingo card. For example, ["Red", "Let's Go Pikachu", "Sun", "Moon"]
. Only pokémon that are in one of those games will be in the final list
1. numOfEntries
is how many entries you want to have in the endyeh those I understand
the gamesToInclude I still have to make a var of, as I haven't finished the form yet that itll be pulled from
same with the numOfEntries, will be pullen from that form
does this mean that I have to make the array within pokelist with all the games they appear in be named "game"No, the
for (const game of gamesToInclude)
is using the JS for...of
syntax, which is a better way to iterate over an array than the more traditional for(;;)
loop. What it does it for each item in an array, it does what's in the curly brackets {}
and the current item is named (in this case) game
. That's what the const game
bit is, it's declaring a scoped variable for this one loop.
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/for...ofbut its about what you want me to place at your "pokemon entries" placeholder
is that supposed to be the same as numOfEntries? or
o wait.. I think I get it
was I supposed to do
ill play around a bit with it and keep this as open for now, ill let you know if I need more assistance or of i got it to work, is that okay with you?
Sure thing!
I work shitty hours so it might take some time for me to reply, but when I can I will!
thats fine
thanks already!
👍
Heyy, so, I've written the array a bit so that I can test the code and get it to work asap, tho it has run into an error
I am not used to this kind code, so could you see where the issue lies?
JS code:
screenshot of log: (I noticed I didn't show the actual result of getChosenGames(), what it returns is
[blue]
in other words an array with 1 objectoh wait, availableGames is defined nowhere, I notice
Unless this is how your code is meant to be xd
but if not, hold on let me try something
Well I've solved that issue, but now mon isn't working 😂
Didn't you define that in the line above?
This is what the tooltip gives me, I assumed I would have to put
mon : { in front of every of the objects, but that just caused an error
OH, the issue is at
.map
Its the first map that holds the issues, the entry there also doesn't seem to be the same "entry" as the one in the second .mapThe first bit seems to be doing it correctly
But then the first
.map
somehow is disconnected from it?I assume I'll just have to define mon and entry somewhere, but the question is where and what
I've tried a few things but none yieldedresult
they fixed the error but they didn't do anything
I’ll be available in 4 or 5 hours, long days at work. I’ll see what I can fix for you
I mis-read and used
availableGames
instead of availableInGames
.
Yeah, you're not defining mon
anywhere in the arrow function. You have .map(entry => ({mon, rando: Math.random()}))
. It should be .map(mon => ({mon, rando: Math.random()}))
.
Keep in mind that each method is a self-contained function. So all the code inside the parentheses of .map()
are scoped to only those parenthesis.
I do think that I might have been to quick to use a bunch of array methods that you're not familiar with, though. I'm currently re-writing it to use a group of individual functions to get the same output. Give me a few minutesGist
A collection of functions to filter and randomize an array of Pokémon
A collection of functions to filter and randomize an array of Pokémon - GeneratePokéBingoCard.js
I have fixed this issue by changing these values, this one I could figure out and fix myself
changing entry to mon is one of the things I tried to fix it, however that wasn't the solution
speaking of this, was I correct in doing
const pokeList = pokelist
?
ill look at it when I finish work
I may play around with it in several days so I will keep this as unsolved til then atleast, I think I'll first want to finish up the other parts and figure this out at the end
If it all fails and I am unable to get it to work with the arrays, I can always fall back on the for-loop
I already figured out how to do it with the for-loop in a simpler manner, I overcomplicated the for-loop
I can just put all the checks in one if; IF(chck1() && check2() && check ...){doCode}
If it doesn't pass one, then the IF returns false and the code won't be executed
BUT I first want to do everything I can to make sense of the Arrays code, it looks more professional although it lacks readability
In my head I am thinking : good code is code you can read without needing commentlines1
Hey, so I have good news and bad news!
Good news: I understand it more now and gotten it to work, well sort of
Bad news, it is not going into a random order, I thought the rando would make the order randomized
maybe I did make a mistake along the way
code:
its generating the array as intended, with only the games requested and the size requested
but its not a randomized orderNo matter how often I ran it, it always returned like this
oh wait i accidently changed the tosorted, let me change that n see if that sovles it
it did not :(
I have seen the mistake... I made the value in
.map
something else and forgot to also change in .toSorted()
🤦♂️
I'm stupid, it even took me several looks to realise that's the issue
I tried everything but not looking at my code!
Thanks!
This has been a huge help, I will need to do a lil' bit more looking into the array functions, but I'm now able to understand them somewhat,
I also now understand the arrow syntax, I do have one question
When should I use arrow syntax and when should I use regular function(), do they have any difference and should one be avoiding in certain situations?There are two major differences between using arrow syntax and
function(){}
. The first is that function myFunc()
is "hoisted" to the top of the file, so they can be called before they're declared. Arrow functions need to be bound to a variable name so can't be hoisted: they have to be declared before use.
The second is the this
problem. In a function(){}
expression this
is bound to the function whereas arrow functions don't bind this
to the function, so this
is actually the parent of the arrow function.
Arrow functions are most commonly used in method calls like the Array.map()
and Array.sort()
as they're easier to write and, if you don't include {}
in the arrow function, it has what's called an "implicit return". That means that what comes after the =>
is returned without needing to say return value;
. Very handy for short, concise functions inside a method!ahhh I see, thanks