Placeholder Image

Subtitles section Play video

  • Hello, world.

  • This is CS 50 Live.

  • My name is David Malin.

  • We're here to review your code today.

  • So see a safety zone.

  • Andrew and Colton are secretly sitting off camera here just in case I break anything since I'm on my own today, I'm told.

  • But thanks so much for all of your submissions of code by the Google form.

  • Glad to see so many people and so many familiar names and the twitch stream feel free to tune in by a discourse or other channels.

  • And we're ready to go today.

  • So the goal at hand is to provide some feedback on code, realized that reasonable people will disagree, including you with me.

  • So by all means don't take everything I say as gospel.

  • But at least give you my own perspective based on my own experience on what you could do differently or better in your particular code.

  • Perhaps when it comes to design or style, especially all right, so feel free to chat along in the chat window here, ask questions as we go.

  • But here we are.

  • So thanks to salt Eric, I think we called him some time ago.

  • We have this mission of an HTML based HTML based code here, a little something reminiscent of Zelda with a little bit of gypsum warm, if some style.

  • Ah, Latin.

  • So let's dive right in here.

  • Eric has said that he made this because he was bored.

  • That's a perfectly valid reason, I suppose, to write some code.

  • So let's see what he wrote.

  • Now here are five files Read me a true type file and a CSS file.

  • An HTML files on a job, a script file.

  • So, as before, weren't you begin?

  • Well, my eyes might gravitate initially to the read me, but we pluck that one off already.

  • So how about we take a look at index dot html instead?

  • Is really the entry point anything that's very Web based on girls tomorrow?

  • Three.

  • Yes, we do have a discord server, and the true kidneys has kindly pasted the Earl there, too, for us.

  • So here is index dot html.

  • Let's go ahead and take a look here at what Erik has done and see if we can't wrap our mind around what's going on here and ultimately, what we might do differently.

  • And hello, It's a Babbitt tonight, so of course.

  • Every Web page has a head section, as Eric does up here and then a body section, as he does appear.

  • At first glance, everything looks pretty neat and everything is nicely indented and it's very readable.

  • But I do see a few things that are perhaps worth commenting on.

  • So let's work our way from the top to the bottom.

  • So up here at the top, we have the doc type declaration, which indicates this is HTML five.

  • The latest version S O.

  • That is helps us orient as to what kind of features of hte e mail here we might use.

  • Now it's out.

  • Here, I think, is the first opportunity for some feedback online.

  • Six and seven.

  • What perhaps jumps out it folks here, Where might there be an opportunity to do things a little bit differently?

  • If you look at these, you'll see that Eric is using online 70 your L to a CD en So Cloudflare, very popular for hosting CSS files, HTML files, Java script files and the like, but he's included his own CSS above that, which is not bad in this case, because, as we'll see in a moment, there's not too much CSS going on in there.

  • But this could be a problem.

  • In general, you might want to adopt instead of habit of having any of your cdn assets at the very top and below those having your own custom CSS or your own custom Java script.

  • But why might that be?

  • Well, the whole point of CSS is cascading style sheets, the C in C.

  • S s.

  • And what cascade means is that any rules they come below others can potentially cascade or override those particular directives.

  • And so is my worry.

  • Here is that if there's some settings in the CSS files here, Eriks missed out on an opportunity to potentially override those settings by having his index that CSS file below.

  • So I might literally just swap line six and seven so that you have the ability to override those You could in fact, in rename as Sabina proposes Index Nazi SS to style dot CSS.

  • But that's completely I would say Ah, personal decision, because there are so few files in this particular example index dot html indexed out J s index dot CSS.

  • Honestly, this is a pretty good naming scheme because it makes clear to me.

  • The viewer.

  • What files?

  • Lineup?

  • Indeed, they're all somehow interrelated.

  • So let's take a look down below.

  • Here we see some classes in CSS, and if you've not heard of containers or had texts entered before and CSS classes, odds are that's coming from this Boma framework that Eric has used up here via the seedy end.

  • So we would have to read the documentation to know what those air doing, but thankfully, they're pretty self explanatory.

  • Odds are he's trying to center the text here and here.

  • He's trying to make this text size one.

  • As via this class here.

  • No, this is not a big deal in The browser's going to ignore it.

  • But there looks like a little sloppiness here, having an errant space before the word that shouldn't really be there.

  • And I kind of see this year in here, although here it's okay because you want him actually put these radio buttons a little to the side of these words.

  • Na'vi, Beetle and Link.

  • But there's definitely something incorrect.

  • Hear this?

  • I'm not loving.

  • This might just be a typo, but lying breaks should be an open tag or a start tag so open bracket B R and then either close bracket or keep the trailing slash in the tack.

  • But at the end, this here suggests that this is closed br stop e r.

  • And that's actually nonsensical because line breaks are either there or not there.

  • So I'm guessing this was a mistake Guessing that slash should be after the BR not before the br, but again in HTML five.

  • You actually don't need to close tags like that anyway, instead, so let's take a look down here.

  • We see another line break, probably a typographical error, so I'd move that slash to the end of the BR.

  • Then we have a button which looks good.

  • Then we have another container which looks empty.

  • I would probably get rid of this unless maybe it's gonna be used in the Java script file.

  • We'll see.

  • And indeed, here some mention of Java script.

  • So here's maybe some food for thought, like why put index dot Js at the bottom of the file?

  • Why not put it in the head of the section where we do sometimes in CS 50 where you might have done in some of your projects?

  • Is this good or bad he'll put a chime in with your thoughts.

  • But before we answer that, let's actually take a look it index dot Js and see what it's actually doing and see if it indeed does matter.

  • So I just hit back.

  • I'm gonna go to index dot Js.

  • And in here we see a whole bunch of JavaScript code at the top.

  • I seem tohave.

  • Ah, few different variables.

  • Beetle tech savvy text and link text.

  • The first of those are initialized.

  • Is JavaScript a raise here to just be a little careful?

  • This is more style than anything, but this probably shouldn't be there.

  • This maybe shooter shouldn't be there.

  • Depends on what might be a pending to this.

  • We shall see here we have a great random in function.

  • That's a nice abstraction.

  • Sort of giving us a name for functionality that we might like and then down here, what do we have going on?

  • Generate text.

  • A nice comment here queries all of the radio buttons and returns the selected values.

  • So here we have, indeed a loop that's gonna go ahead and figure out which of the radio buttons is checked.

  • I see that There's probably an opportunity to improve here.

  • It's very minor, and in the real world you're not going to notice the difference.

  • But the moment you find a radio button that's checked, you might as well break out of this four loop because you probably don't need to keep checking those same radio buttons because they're supposed to, by definition, be mutually exclusive.

  • All right, let's keep going.

  • I got a random into, presumably between one and 100 or zero in 100 here.

  • I'm checking for the value.

  • Good use of the identity operator, just good habit to get into.

  • And JavaScript Checking not just equals, equals but rather equals, equals equals.

  • It makes no functional difference here, but it checks to make sure that what you're comparing on the left not only equals the thing on the right, but is also identically tight.

  • So both of them, in this case, our strings that can matter in corner cases where you're comparing empty strings or zeroes or false values or false e values.

  • So keep that in mind.

  • Is your oko perhaps?

  • All right, let's see.

  • So there's definitely a little bit of messin this year, but this is perhaps objective, but maybe not.

  • So I'm seeing no space in between the if condition and this parenthesis.

  • No space between the same here.

  • But I am seeing spaces after the keywords like four.

  • So Oh, and indeed, here there is no space, so just a little bit of sloppiness doesn't really matter, Certainly to me.

  • Which one?

  • You D'oh!

  • But go ahead and at least be consistent.

  • Otherwise it looks like your code isn't quite ready, quite complete and no sense.

  • And colleagues of yours are friends of years having to get mired in some of the stylistic errors as opposed to really focusing on the codes correctness.

  • But otherwise I see some good structures here.

  • It looks like there's some pretty consistent code blocks here.

  • It looks like in each of these three conditions were doing a loop and checking the contents of some array.

  • The savvy text beetle text link text feels like there's probably an opportunity to factor this out, maybe to a function that does that, but takes as input the name of the array.

  • So I might keep that in mind.

  • And then down here, it's where we're putting our actual event listener.

  • So in short.

  • It seems like quite a bit is going on in this job script file.

  • So let's come back to that earlier question.

  • Why might we pull it index dot Js at the bottom of our html file and not in the head?

  • Well, I saw it mentioned here.

  • Let me scroll up in our chat.

  • Uh uh, Jeong gi proposes that it's good because the J S s or Java script is among the most heavy files and you want to load it last so the rest of the site loads first he asks another question.

  • Another feedback Here.

  • Um, no.

  • Everyone else is talking about twits right now, which is totally fine as well.

  • So that's pretty spot on Giovanna Egg.

  • Uh, this is a convention putting the JavaScript code at the bottom of your HTML file because if one the code is large, it might indeed take more than a few milliseconds to download, and that can block the rest of your Web page from actually rendering on the screen.

  • So it might.

  • You might see nothing.

  • Initially, your users might just see white empty boxes until the entire page and that code is loaded to putting at the very end means that the code is only gonna be loaded at the very end after everything else that's important to you has indeed loaded.

  • And then, too, if you were to put in the head supposed that that file we're not on my server index dot Js, but like on a CD en or 1/3 party server, which is especially the case for things like advertisements and add servers that could really slow your own site down.

  • And you might have no control over that.

  • So even the link that we had appear using the bullet CSS This one has to be there because it's CSS and CSS links.

  • Do you need to go in the head of a page by definition of html five but Java script, you can at least afford to put it later in the file and load it therefore, only at the very end.

  • So so great.

  • Great feedback there, and I think we're in good shape here.

  • So let's just take a look at one more file, perhaps the index that CSS file And here is where we have some reassurance that okay, probably doesn't matter.

  • The ordering of those two files and index dot html because there's not terribly much custom CSS here.

  • But indeed, we see that Eric has defined a default face using this true type file that is the fifth of the files in this repo and specified that his H one tag should have letter spacing of five pixels and font family of tri force.

  • But in general, your CSS should probably come after any libraries that you're using else.