hello world this is cs50 live my name is
David Malan and we're here to review
your code today so cs50 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 forum glad to see so
many people and so many familiar names
in the twitch stream feel free to tune
in by a discourse or our other channels
and we're ready to go today so the goal
at hand is to provide some feedback on
code realize that reasonable people will
disagree including you with me so by all
means don't take everything I say as
gospel but only 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 alright 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 if some lorem ipsum
style 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 readme a true type file and a
CSS file an HTML file and a JavaScript
file so as before where do you begin
well my eyes might gravitate initially
to the readme but we plucked that one
off already so how about we take a look
at index.html instead that's really the
entry point to anything that's very
web-based and Gerson Mauro 3 yes we do
have a discord server and the trucha
niece has kindly pasted the URL there
too for us so here is index dot HTML
let's go ahead and take a look here at
what eric 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 Babic night
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
doctype declaration which indicates this
is html5 the latest version so that is
helps us orient as to what kind of
features of HTML here we might use now
it's out here I think is the first
opportunity for some feedback on line
six and seven what perhaps jumps out at
folks here where might there be an
opportunity to do things a little bit
differently well if you look at these
you'll see that Eric is using on line
seven a URL to a CDN so CloudFlare very
popular for hosting CSS files HTML files
JavaScript 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 a habit of having any of
your CDN assets at the very top and
below those having your own custom CSS
or your own custom JavaScript but why
might that be well the whole point of
CSS is cascading style sheets the C in
CSS and what cascade means is that any
rules they come below others can
potentially cascade or override those
particular directives and so my worry
here is that if there's some settings in
these CSS files here Eric's 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 renamed as
Sofya of proposes index dot CSS to style
that's CSS but that's completely I would
say a personal decision because there
are so few files in this particular
example index dot HTML index J yes index
dot CSS honestly this is a pretty good
naming scheme because it makes clear to
me the viewer what files line up 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 text centered
before as CSS classes odds are that's
coming from this boma framework that
eric has used up here via the CDN so we
would have to read the documentation to
know what those are 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 now this is not a
big deal and the browser's gonna ignore
it but there looks like a little
sloppiness here having an arrant space
before the word that shouldn't really be
there and I kind of see this here and
here although here it's okay because you
want them actually put these radio
buttons a little to the side of these
words Navi beetle and link but there's
definitely something incorrect here this
I'm not loving this might just be a typo
but line breaks should be an open tag or
a start tag so open bracket BR and then
either close bracket or keep the
trailing slash in the tag but at the end
this here suggests that this is close BR
stop BR 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 html5
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 JavaScript file we'll see and indeed
here some mention of JavaScript so
here's maybe some food for thought like
why I put index J S at the bottom of the
file why not put it in the head of the
section where we do sometimes in cs50
where you might have done in some of
your projects is this good or bad feel
free to chime in with your thoughts but
before we answer that let's actually
take a look at index J s and see what
it's actually doing and see if it indeed
does matter so I've just hit back I'm
gonna go to index J s and in here we see
a whole bunch of JavaScript code at the
top I seem to have a few different
variables Beatle Tex Navi text and Link
text the first of those are initialized
as JavaScript arrays here to just be a
little careful this is more style than
anything but this probably shouldn't be
there
this maybe should or shouldn't be there
depends on what we might be appending to
this we shall see here we have a grit
random int 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
elected 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
gonna notice the difference but the
moment you find a radio button that's
checked you might as well break out of
this for 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
int presumably between one and a hundred
or zero and and a hundred here I'm
checking for the value good use of the
identity operator it's just good habit
to get into in 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 it's also identically typed so
both of them in this case are strings
that can matter in corner cases where
you're comparing empty strings or zeros
or false values or false II values so
keep that in mind is your own code
perhaps all right let's see so there's
definitely a little bit of messiness
here but this is perhaps subjective but
maybe not so I'm seeing no space in
between the if condition and this
parentheses 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 do 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 or friends of
yours 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
we're doing a loop and checking the
contents of some array the navvy 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 where we're putting our actual
event listener so in short it seems like
quite a bit is going on in this
javascript file so let's come back to
that earlier question why
like we put index J s 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 the Giovanna AG proposes
that it's good because the J SS or
JavaScript 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 no
everyone else is talking about twitch
right now which is totally fine as well
so that's pretty spot-on Giovanna egg
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 webpage 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 so 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 to if you were to put in
the head suppose that that file we're
not on my server index.jsp like on a CDN
or a third party server which is
especially the case for things like
advertisements and ad servers that could
really slow your own site down and you
might have no control over that so even
the link that we had up here using the
boma CSS this one has to be there
because it's CSS and CSS links do need
to go in the head of a page by
definition of html5 but javascript 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 dot CSS file
and here is where we have some
reassurance that ok 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
font face using this true type file that
is the fifth of the files in this repo
and specified that his h1 tag should
have letter spacing five pixels and a
font family of Triforce but in general
your CSS should probably come after any
libraries that you're using else you're
just giving up control over the
aesthetics
unnecessarily and indeed so this is a
code review for those of you have just
tuned in
we've solicited code from folks via a
cs50 ly slash code review we just took a
look at the first example from NaCl Eric
on github here we have a few more that
we're gonna take a look at and indeed
this is streaming on twitch on YouTube
and on Facebook so feel free to chat
with each other and chat with us by any
of those mechanisms if you're watching
live so let's go ahead here and let me
see if I can't draw your attention to
the code in question all of these are
public repos by definition so if you'd
like to go ahead here and perhaps Colton
off camera can paste the URL of this
current repo into the chat
absolutely the truká nice we can go
ahead and show that to you all right so
here we go oh and user man - I see that
you have heard that there is lazy
loading libraries for JavaScript so
that's indeed the case - you can use a
third-party library that will actually
go fetch as via Ajax your JavaScript
code and inject it into the document in
the appropriate place that two addresses
this issue and gives you more
flexibility if you don't have a complete
control over where the tags are actually
going in your web page so thank you to
Colton who's logged in here at cs50.tv
for the URL of that first repo and thank
you to Eric for contributing all right
so let's forge ahead and this one I
think you'll find is a bit of a
throwback years ago cs50 our
introductory course used to use PHP at
the tail end for our look at web
programming these days we use Python but
this brought back some memories for sure
because for quite a few years did we use
PHP which is still a very popular
language for web programming in answer
to your question the mr. distinguished
if you submit your code now will we get
to it probably not today just because we
have quite a bit of interest from other
folks that have submitted already but do
still submit and we'll weave it into a
future stream as well we hope so for
those unfamiliar or less familiar with
PHP here we have a repository that has
pretty representative structure that you
can do this in different ways a bin
directory where your binaries go which
is sometimes a misnomer because through
the files in that directory aren't
necessarily themselves compiled these
days as binary would imply zeros and
ones but that tends to being where your
executables are any scripts or such
source SRC of course is where people
tend to keep their source files their
source code things that are actually
core to the application in winde
tests of course is where you might put
some test scripts and then there's some
other files in here get ignore if you're
unfamiliar that's a file you can
maintain in a git repository to tell git
what files not to push to some service
like github comm things like dot files
or config files or personal files that
you don't want to accidentally get
caught up in version control composer
JSON is like standard within the PHP
community whereby you can specify what
packages you want this application to
depend on and using the composer command
can you pull those dependencies down
just like excuse me with pip or with gem
can you do that in python and ruby
respectively PHP unit XML is a common
file for configuring your unit test so
to speak your actual test code and to do
duck text sounds like Michael Paul's to
do this to himself here though it looks
like he's concluded this particular
project 11 some years ago in fact so
where to begin you know the most
interesting file at the top level of
this repo looks like it would be robo
file PHP because there's no readme so
unfortunately we're going into this kind
of blind so let's see what robo file dot
PHP looks like here so here we have
projects console commands configuration
for the Robo task Runner and we'll see
some syntax that you might not be
familiar with you might know about
classes from Python PHP has classes as
well C does not C++ does you'll see that
this extends a package really or name
space starting with Robo and the class
name therein is called tasks kind of a
weird syntax introduced a few years ago
to PHP but it's their equivalent really
to Python packages where you use that
dot notation or Java packages that gives
you the the dot notation as well let me
scroll down here and provide some
feedback now in the code if you're
familiar with C you might recall this
arrow operator is like the dereference
operator in c in PHP it's not
technically that but similar in spirit
it means go inside of some object and
call some function or method or go
inside some object and get out some data
it looks here that Michael is using this
to actually call a number of methods
inside of this Robo file
class or the task class that he's
extending so if stop on fail is one
method task phpunit tokenizer test
parser test compiler test this is
perhaps a matter of style and preference
I personally find it a bit strange to
put your chain method call on a separate
line
it looks like run is simply being called
on whatever the return value is of this
method task phpunit so this isn't wrong
it will still run no pun intended but it
just feels like this line of code is so
short already I personally would
probably pull it up onto the same line
so that I can just read things left to
right top to bottom but again to each
his or her own here let's scroll down
here interesting that we had seemed to
have the comments are switching between
what I gather is probably Spanish in
English that's okay I would probably be
consistent using one or the other it
doesn't really matter which depending on
who your user base is same comments here
about run sort of unnecessarily being on
a line of its own but it looks like we
just have a whole bunch of functions
here ultimately that are testing the
code that we're about to dive into so
we'll see that soon as well in here this
is fine too this is in PHP using the dot
operator which allows you to concatenate
one string with another lines like this
are pretty common in PHP I personally
find a little harder to read and might
prefer to use variable interpolation
whereby you actually use double quotes
around your string and you actually have
your dollar sign variable inside of that
double quoted string and it will just
get interpolated so to speak that is
replaced with the actual value of course
Michael seems to be using a function
call here and you can't do that trick in
quite the same way but that's fine as
well it's just a different approach just
kind of hard to read the format I think
when it's broken out a little bit like
this so this makes me a little nervous
and we'd have to look at the code more
closely to see if this is indeed a
problem but it looks like this code is
exacting so maybe that's actually
executing a process called tokenizer PHP
a PHP script in the bin directory and
then another shell script it seems
called Tex comparer sh it's a little
weird it seems to use one PHP file to
execute another PHP file as opposed to
calling functions defined in it
but it's a little too soon to have
judgement until we've actually seen
these files but generally speaking
exacting or executing other files you
want to be careful of because you have
less control over of what codes getting
executed if again you're saying execute
this whole file as well all right so
let's take a look a little further it
looks like more test related code
specific to the framework so let's go
back and dive into that bin directory in
the bin directory we'll see a few
different things here's that tokenizer
so let's take a look at this a tokenizer
is typically a program that chops up a
stream of text into what are called
tokens which might be individual letters
or words or sort of meaningful
grammatical units you might tokenize an
english sentence into your individual
words for instance but in programming we
of course have so much more so many more
symbols so tokenized means to return to
you one at a time all of the interesting
chunks that are inside of your code that
might be one or more characters all
right so let's say what we've got going
on here we see some very common PHP
conventions this is used by composer to
autoload library files that you might
want to use nice use of class here
presumably a jack tokenizer which we'd
have to look at in another file namely
in this namespace here here we seem to
have some pretty clean code except for
the fact I'm not seeing a single comment
and frankly this file is long enough
that I think this would probably benefit
from a bit of explanation but otherwise
it looks clean at first glance let's go
back and maybe look at Jack compiler
which sounds like it's the the guts of
the program Jack compiler here let's see
this looks like it's a command-line
program we're seeing the usage
information here which is good
PHP end of line this is nice because
it's making it a little more resilient
against platform so instead of just hard
coding backslash N or backslash R or
backslash R backslash n which are
different conventions on Linux and on
Mac OS and on Windows you're asking PHP
use the PHP end of line character so you
don't even have to to think about that
and now scrolling down to again very
nice code and it's not terribly long but
honestly at least for someone coming
into this cold like I am so
comments I think would certainly help us
explain what's going on so you know we
don't just have to waste time actually
figuring everything out let's take
another quick look maybe in the source
directory here and here is the essence
of the program though I'm a little
contree to see another JAC tokenizer dot
PHP I wonder if one of those really
should have been with the other I'm not
sure so let's take a look at main dot
PHP right because in the absence of not
knowing where to look look for some
familiar phrases index dot HTML main dot
PHP or anything suggesting that this is
an entry point so I'll go ahead and dive
into main dot PHP all right here we have
some unit tests question mark may be a
bit of a to-do there here we have a
class called main and you'll see now
especially if you're unfamiliar with PHP
just hints of some of the features of
PHP the ability to define some kind of
variable inside of the class that's
protected which has to do with its
visibility whether it's public or
private or somewhere in between
protected here you'll see this
convention of underscore underscore
construct which is PHP s approach to a
constructor function you might recall
the init method in Python which is very
similar in spirit or the constructors
that C++ and other languages have
scrolling down here would definitely
love some some comments but I think we
could infer from this what's going on oh
I see oh this might be Portuguese
instead perhaps it looks like our
exceptions are alternating between
English and that Portuguese so just as
we saw with the comments earlier so
let's see if we can't wrap our mind
around this so again I think it's just
the mattock like I'm finding the code to
be pretty clean overall that helps of
course that I know a bit of PHP but I
think definitely some commenting at this
point as well so what red didn't go
through every single file because I'm
not sure we're finding it particularly
troubling trends or anything like that
why don't we give this one a good
thumbs-up and Colton if you wouldn't
mind sharing it in the chat if we
haven't already
shall we take a look at something
completely different and again along the
way feel free to ask questions or ask me
to pause or slow down on any of these
files or code as well alright so next up
we have an example of a game written in
Python implementing a deck of cards in
fact let me take a look at our
description here
give me just one moment our deck of
cards this is a simple Python script
that builds shuffles and prints a deck
of cards such that it is ready for
expansion into a simple card game the
focus here is on clean clear style
Martin Jane welcome to the stream as
well
his main questions for us here today are
how is the formatting style can it be
wrapped into a class with some methods
for use as a module or they're neater
more concise ways to structure the deck
data or main naming conventions
acceptable let's see if we can pop up at
least a few of those as we take a look
our volunteer here tonight does give a
disclaimer note this was made just for
fun he was curious to see if he could
he's pretty new to this and that's fine
too if you're proposing and submitting
your code to me or to Colton or any of
the team here you don't need to disclaim
that you're new to this we expect as
much so we're happy to help however we
can and again as always take our own
thoughts with a grain of salt the true
kinase asks is it good practice to have
at least one comment at the beginning of
a file describing what it does no matter
the file program length I would say
probably I'm not gonna say yes to
absolutely everything especially if it's
a configuration file or something that's
rather self-explanatory but yes I think
at least certainly if you're making your
code open-source if you're sharing it
with a colleague or a friend if you're
submitting it to a course for a teacher
to review I mean anything that helps the
human focus on the interesting work that
you did without having to wrap their
mind around what was the problem you
were even solving just tell them so that
they can actually think harder about the
code and not about the overarching goal
of that code alright so let's take a
look at this let me go over to our repo
here for deck of cards dot Pi and here
we have what's called the gist on github
this is sort of a temporary file that
you can create and share independent of
repository but that has a bunch of other
features as well like commenting and
revisions as well so here we go deck of
cards PI I seen a couple of imports at
the top of the file that's great that's
where they typically belong giving me
access to the random library and the
time library or modules here I seem to
be getting the local time and storing it
in system time date and then iterating
over the system time date structure in
order to get at various fields so I
think a few comments here
it looks like we're setting ourselves up
to use what's called a pseudo random
number generator some function that's
going to give us random numbers again
and again and again and again using some
uniform distribution of values between
typically some lower bounds and some
upper bound I say it's pseudo-random
though because if you've taken cs50 or
some other class or read up on
pseudo-random numbers you'll know that
computers can't really be random right
they can be programmed to do what you
say but they can't be programmed to just
think of a number off the top of our
head of their head like we humans can so
you actually have to write a program to
generate random numbers and the way you
do that is by writing code that takes as
input at least one kind of seed initial
default value very often that seed is
like the current time not in terms of
hours or minutes but probably in terms
of seconds or milliseconds or
microseconds or something along those
lines because why time is always
changing so in this way if you're
running this deck of cards program or
file and now or in five minutes or
tomorrow that time value will be
different and so the pattern of numbers
your pseudo-random number generator will
generate will be hopefully different
based on when you run the program so
therefore it gives sort of the illusion
of randomness or at least the
approximation of a distribution of
values that would otherwise be perceived
as random but that invites the question
what seed do you choose if you choose
the same seed like 0 or 1 or 42 or 50 or
anything else and you generate random
numbers thereafter you're gonna get the
same random or pseudo-random numbers
again and again and again because they
all start with the same seed so you want
the seed to change in what I see our
volunteer here doing is kind of
implementing his or her own seed value
and long story short just not necessary
I'm guessing this code was inspired
maybe by a tutorial or a post online
because there's some interesting
mathematics going on here but this is
the kind of thing generally in computer
science and in programming but you don't
want to reinvent yourself unless you're
feeling very comfortable and you want to
solve some problem that someone else
hasn't much better much safer to use
libraries out there were lots of other
smart people and our open source
community ideally has all weight in
visually on the code and said yes this
code looks
rekt rather than reinventing these
heuristics ourselves so long story short
none of this is probably necessary
actually even this none of this is
actually necessary beyond getting the
local time if we need it because the
random library in Python the random
package actually provides you with the
ability to seed it for you so if I
google Python random and then let's type
in let's just do that see what results
we get random random random I want to
get to the version 3 of Python so here
we go generate pseudo random numbers and
you'll actually see if you read through
this that random does come with a seed
function that if you omit the value it's
gonna use the current system time and if
we really typed into the documentation
you can see that python has a secrets
module where if you really care about
certain cryptographic properties of
randomness there's other code that other
people have written so long story short
when it comes to implementing certain
primitives especially related to
randomness or cryptography generally
speaking I would rely on existing
libraries rather than reinventing that
wheel yourself alright well let's dive
into some of the actual code here and
see what it is we're looking at so this
is a program somehow related to cards
and if you're familiar with the deck of
cards they have spades diamond clubs and
hearts those are the different pictures
that are on each of the cards here we
have a nice utilization of Unicode these
characters do actually exist in Unicode
so even though you might not think you
can do this you can actually have
unicode characters in your code these
days assuming your interpreter or
compiler supports it which it should
these days you can even have emojis
but that's a whole can of worms that we
won't open today here we see another
list in Python declaring all of the
values that cards can have on a4 ace
then 2 3 4 all the way through 10 then
Jack then Queen then King so these
represent all of the numbers or the
values that a card might have and then
down here it looks like we're getting
ready to have some counters T sweets and
T values which are respectively the
number of suits and the number of values
I'm guessing this was used here so that
you don't have to ask the length
function again and again how long
something is it's a minor optimization
it's fine to store that value in a
variable but it's pretty efficient to
ask
distant Python what it's length is so I
wouldn't fret too much about that great
comments here throughout and I'll leave
it you to read through them if you'd
like Colton is kindly pasted as our
cs50.tv accounts the URL of this project
if you'd like to follow along here we
see some creation of some tuples it
looks like inside of a list some kind of
array or matrix via which we can
actually compare cards and their values
and then I see the placeholders for a
couple of more lists Dec print and Dec
combined and Dec shuffled the comments
are pretty detailed here it looks like
our volunteer is going to populate each
of these lists with things like the
values of the cards or the human
friendly versions of the cards the human
pronounceable versions so this seems to
be pre-emptive and indeed here we go we
have a bunch of loops now to build a
pretty deck and by that I take it to
mean that there's going to be strings
inside of these lists like such and such
a suit with this such-and-such value so
more of a human friendly English light
description of that card as opposed to
just a or two or three or so forth here
though is an opportunity I think
stylistically or even design wise to
factor this out it seems a little weird
that we're still in the confines of this
main program we've created a couple of
lists now we're have the code to fill
those lists it feels like this is a nice
opportunity for abstraction give me a
function that returns a list with all of
these values or do it in maybe a class
as you as our volunteer him or herself
asked in the submission of this actual
code just feels like we could clean this
up rather than just doing everything
from top to bottom because these are
pretty distinct operations all right so
let me scroll down a little further
here's where the deck of cards is
actually getting shuffled and here's one
helper function it looks like print deck
is gonna be a function that prints the
entire deck in its shuffled order and so
here we see some pretty fancy
utilization of iteration going ahead and
printing out the deck of cards itself
but that's that alone at least is good
good encapsulation although ironically
after going through all of this top
level code and then introducing one
method to your print deck we're just
calling it immediately which suggests we
didn't necessarily even need to define
this function
I think as our volunteer noted if this
file is meant to be used as a component
for some other card playing game then
you'll probably want print deck to exist
but you probably don't want to be
calling the print deck function in case
someone imports this function as with
the important keyword in Python because
the function might get called
accidentally good to see some familiar
names just to catch up on the stream
here Andre nice to see you and Bella as
well and AES sitaji nice to see everyone
as well and I see a question that just
scrolled off my screen let me scroll
back up here are the comments too
detailed or is that possible
ie should be worried about over
commenting asks the mr. distinguished
that's very valid concern I don't think
we've crossed that line with these
comments I think the fact that most of
these comments are one or maybe five or
so lines maximally is still pretty
readable if it became unreasonable well
frankly that's an opportunity again to
factor some of the code out and put it
in a function and then document the heck
out of that function right you really
can't go wrong documenting your
functions explaining what the inputs are
what their outputs are how you use them
any corner cases to be mindful of any
exceptions that might be thrown so if
you were to factor this out in two
separate functions to generate the
shuffle deck and these human-friendly
strings frankly you could comment even
more I would say I think the risk that
you're identifying is that when all of
your code is together like it is in this
one file at the moment those comments
just push and push and push the code
down and down and down
so you nup having more comments than
code and it's just a little harder to
wrap your mind around it it's hard to
distinguish the code among the comments
so there's some inflection point there
so yes worry about over commenting if
it's in line with everything else at
that point you should probably be
factoring things out into functions and
documenting things a little more modular
lis might be some helpful advice there
all right
so Gerson Morrow three nice to meet you
in the stream here glad you got to hear
about cs50 through nice daily new series
actually a former student of mine in
cs50 and another course here at Harvard
called CS 160 for software engineering
course and he's certainly with his whole
team has gone off to do some amazing
things including visiting campus that
one day and reminiscing with us about
his time here
on campus sodium SMTP high from India
one am here in cambridge massachusetts
USA it's about 3 p.m. in the afternoon
all right I think that leaves us with
one more program to dive into here let's
take a look at what we've got in the
form of this Redick wallpaper bots will
go ahead and paste the the link to this
and the chat if you'd like to follow
along some of you might like me spend a
little too much time on reddit reading
or writing in this case here we have a
submission that's gonna do the following
let me go ahead and read its formal
description and pull this up here on my
screen so in this case here we have a
reddit scraper that sets the background
image based on what's trending on reddit
so this sounds like a really cool use of
an API dynamic dynamic in that it's
gonna change over time based on what's
indeed trending on to on Twitter on
reddit so let's go ahead and take a look
at the actual repository that we have
here so this is from can do for for one
one thank you for this submission as
well so here we have a readme which is
wonderfully detailed this is a great
example of helping your users get on
board which is really quite cool reddit
paper wall bot says what it is or bought
for reddit that gets top hot sorted
picture from a given subreddit and sets
it as the background this will work
looks like on Windows Mac OS and Linux
prerequisites you need Python 3 6
installed which is actually helpful
those of you familiar with Python in
recent years know that it's been
evolving newer versions have newer
features and if you write code then
assume those features your Python code
won't work on older versions so helping
people to know what version you want if
not the latest is quite helpful to
ensure that people can actually engage
with your code and use it as well all
right let's take a look here here's some
installation instructions so if you'd
like to play along at home you're
welcome to follow these instructions
here it looks like you'll need a reddit
API key essentially a client ID and
client secret so that this code can
masquerade as you let me caution that
you should not just go using any old
code on the Internet including code that
we're taking a look at here because
these are live repositories they can
change over time and not to impugn any
of our kind volunteers today but you
should oh
waise beach we discuss in cs50 a little
bit skeptical of any code someone's just
put on the internet or encouraging you
to run you probably want to at least
skim through it and make sure it seems
to do what it says and doesn't have any
scary removals or email spams or
something like that
so again just always beware whether it's
with our examples or anything you find
on the internet that you shouldn't just
trust something just because it's free
indeed it can be doing any number of
things so let's take a look here I'm not
going to go ahead and install this on
this particular machine because we're
focus of course in the code review on
the code itself but I'm gonna go ahead
and scroll back up to the repository and
we'll see a different structure from
before but where should my mind wander
so git ignores get specific and keeps
files in or out of the repository
license file is a convention on github
that allows you to communicate in a
standard way what open source library if
any you're providing your code with pip
file here pertains presumably to pip and
any requirements that we might have for
this particular project the readme of
course is a github thing here and then
we have bata PI main dot P and
requirements text the last of which is
also particular to pip and any
dependencies you might want to have so I
like I'm a little torn I kind of want to
look at the bots but I kind of want to
look at main so let's look at main first
because that's probably my entry point
here so here we go this seems to be a
command line program that's going to
actually kickstart this bot as best I
can tell I like this some very clean
imports at the top here an additional
import that is probably included in our
requirements dot text separated from the
ones that presumably come with Python
bot meanwhile is separated by a white
space to make a little more clear that
it's not like the others it's indeed in
the same directory a module called bot
pi so let's keep scrolling down here
looks like there's some kind of
configuration going on here no comments
but I can perhaps glean from these
function names love the ASCII art down
here it looks like when this main
function is called we're actually gonna
see wallpaper bots on the screen which
is nice and fun and then down here it
looks like there's some kind of
configuration being opened I'm actually
being prompted it seems for the
subreddit
or the category or the bulletin board on
reddit if unfamiliar that you want to
access it looks like here now we're
using this bot library to actually get a
subreddit which one well the one that's
being passed in along with its name here
it looks like there's some kind of API
call here that's gonna get the picture
post based on hotness sorting by hot
like what are the most people looking at
uploading clicking on and so forth
this looks to be maybe just for the
humans benefit to print out the title
and the author of what it is that was
discovered a little progress message
here we're downloading really good use
of exceptions here we don't spend time
really in cs50 these days on exceptions
but it's another approach to handling
problems in code in C you might recall
that we tend to return value special
values maybe you've returned a negative
value in cases of error or AOF or null
or something like that depending on the
function well the tricky part there is
that if bunches of things can go wrong
it's hard to know what went wrong if you
can only return a value and it's a
little annoying to have to look it up in
a chart like with numeric codes so
Python in PHP and other languages Ruby
and C++ and Java have exceptions
exceptions are a way of just giving up
in the middle of a chunk of code but
with useful information saying this type
of error happened so for instance
pythons requests library which does HTTP
requests over the Internet
getting more information from some
website allows us in this case to try to
get the picture URL in question here but
if something goes wrong it can raise an
exception for the status if it's not a
200 status code maybe it's a 400
something or a 500 something this will
throw an exception and that exception
could be a request exception specific to
this library and if that exception
occurs notice how our volunteer here has
handled that exception well and frankly
Python doesn't make this super easy it
tends not to be wonderfully clearly
documented as to what exceptions can be
thrown by functions unfortunately you
kind of know sometimes only after the
exception has happened unless you really
dig into the documentation and
references online but this is a great
approach to handling exceptions and
being proactive about it welcome to
bitter spec bitter
bits per sec as well as our other recent
subscribers glad to have you here on the
stream Oh
so yes Pat McKnight notes that David has
a PhD in something like BOTS this is
going to be interesting this is
different type of BOTS David studied
botnets which were clusters of malicious
software
triggered by viruses or worms on PCs
that's collectively take over people's
computers otherwise known as botnets
this bot seems to be a bot specific to
Reddit it's more of a robot or a program
that has been written for good purposes
not evil but thank you though for the
shoutout
indeed M Cloppenburg no it's very
different type of bots so let's go see
this bot I'm very interested to see how
this is all working so in bot op I here
we seem to have a class called bot
exception which indeed per the comment
there describes what might go what might
be thrown when in a race when something
wrong happens get ready I seem to have a
function here that uses my login in the
form of my client ID and secret namely
those things you can sign up for per the
readme that we glanced at a moment ago
notice another good use of exceptions
things that can go wrong here it seems
that this underlying client peer or edit
excuse me the library that we're using
to actually connect to reddit's back-end
aachen through certain exceptions client
exceptions so we're catching those as
well so really good error handling
otherwise the user might actually see
errors if this weren't caught in this
way get subreddit looks like this is a
way of using the module or the library
it's actually go get information about a
specific reddit given its name just as
we see right here unfortunately things
can go wrong here and apparently a type
error is thrown in the event you pass an
invalid name how do we know that I'm
just inferring from the code here but
this library is probably documented
somewhere so we would know that as well
and let's scroll down here get picture
post I really like the organization of
this code because it's very hierarchical
or chronological we're getting a reddit
we're getting we're getting vir edits
logging in with my credentials I'm
getting a specific subreddit and now I'm
getting the picture posted that
represents what's hot in that
subreddit so here too we're flipping
over or iterating over it looks like the
subreddit having sorted earlier by
what's hot
I believe we're looking for the actual
image in question and you hear my voice
slowing down because now this is the
first time in this whole codebase where
I'm really starting to try to figure
this out I would really love one or more
comments here especially to help me
understand when maybe each of these
errors might happen although I do see a
comment or so here oh we were just going
doing so well up until that point I do
think I certainly would benefit from a
little bit of explanation as to what's
going on here just because even though I
could figure it out it's gonna take me
so much time to just read through every
individual line when at this point in
the story I really am just trying to get
a sense of things but code wise
everything is very nicely cleaned notice
that this is a very clean approach to
writing an if condition that actually
has multiple sentiments this I could go
either way on I notice that you've put
the and keywords at the beginnings of
the sentence it might be a little
cleaner to move those to the end of the
previous line this way each of the three
boolean expressions you're asking all
line up on the left but that's a very
nitpicky point I could be convinced
certainly otherwise and if we keep
scrolling down it looks like we have
another function here also nicely
chronological that's presumably going to
save the image at a particular path on
like the hard drive or the local folder
that I'm running in great use of error
checking here file exists error OS error
so it looks like either maybe all of
these errors were encountered at some
point and these lines of code were added
afterward to catch those errors or very
defensive programming and both of those
are good outcomes as well indeed colton
has kindly pasted the URL of a youtube
link to episode zero of a podcast an
audio only discussion he and I recently
had will be starting that off looking at
trends and current events and helping
folks to not only understand what's
going on but also to consider the
implications of various things so in our
first podcast for instance you'll find
that we looked at plaintext passwords
that should not have been as well as
robocalls which are unfortunately a very
abusive approach to software because
it's all too easy these days by api's to
make phone calls
feel free to check that out as well
you're of course tuned in relied right
now by a twitch and Facebook and YouTube
to our code review stream which of
course has a video component to it and
that's so that we can actually show you
code and actually provide some thoughts
on it with the whole community here so
another alternative user man to proposes
to the exceptions would be to have guard
or exit condition before you try to
actually do something with the file
system absolutely and I think my only
response there is that people in the
Python community tend instead not to
check for things first and then respond
to those error conditions it tends to be
a little more pythonic to literally try
to do something the operative word being
try and then catch any exceptions as
with the accept keyword if something
goes awry I'm not sure if one way is
necessarily better than the other it's
just sort of a different mindset
except arguably with Python you're at
least ensuring that state can't change
in between those error checks if you
were to have an if condition and then
air out if something is wrong it's
possible in certain environments
especially multi-threaded applications
or whatnot that the state could change
that you've checked in which case you
shouldn't blindly forge ahead so I could
be convinced either way but in the
Python world trying things and awaiting
exceptions tends to be a very common
approach but they're absolutely others
as well all right so let's take a look
finally at this code here creating an
image so some helpful comments
throughout I see setting background
image actually happens below it looks
like a nice use of UX so prompting the
user if he or she actually wants to set
the picture as the background rather
than just blindly doing it so again just
be aware if you're gonna be using this
code on your computer this could indeed
be affecting your own machine because
the whole point is to set a background
on Windows or Darwin aka Mac OS or Linux
so realize that running this code on
your machine even by following the
readme might have effects that you don't
intend so just do beware any time you're
using open source code like this here no
matter how well it's actually written
welcome to MIB AC one three eight who's
just joined us here as well and now we
see finally some implementation specific
details and this is a really good use of
abstraction I would say
in that we have this higher-level
function set image background but that
actually then uses one of these
following implementation specific
functions for Windows or Mac OS or Linux
notice that for Windows there's
apparently this crazy module buried in
this package inside of Python that
allows you to set the wallpaper using
what seems to be some kind of system
calls specific to the operating system
down here Mac OS seems to need some kind
of max script so this feels a little
hackish and that you have to run this OS
a script to tell the application finder
to set depth talk the desktop picture to
image path as POSIX file so it's a
wonderfully expressive scripting
language but we need to create that
script on the fly and then run it I've
not looked into this myself I'm guessing
that's probably the only way the best
way in Python to do it but that indeed
would be Mac specific and then down here
on linux and do you have another
approach that would actually use get dis
stripped a dist
script for distribution specific script
it would seem and then down below that
is a whole bunch of conditions that are
changing settings based on the version
of Linux you're using so this is a nice
abstraction to because you can support
different linux's as well so I see
andrey here is asked where would you
draw the line as far as throwing
exceptions go would you say it's good
practice to throw an exception for
things like erroneous input from the
user or would that be of a bit of an
abuse of the concept would it be better
practice to just make sure the input
conditions are met locally within the
calling function really good question I
think it partly depends on the audience
in cs50 for instance when we introduced
Python we deliberately don't use
exceptions because we only spend so much
time on Python and we want students to
be comfortable and experienced handling
errors but not necessarily through a
whole new paradigm and so we've tended
to rely on return values and proactive
error checking as someone noted a little
bit earlier ago user man - I believe but
it does tend to be and I think we're we
to spend more weeks on Python in cs50
and we're to be writing Python code
myself it does seem it does tend to be
pythonic to leverage exceptions
more heavily so yes if the user is
prompted for a floating-point value and
they provide or rather if the user is
prompted for an integer
and they provide a floating-point value
or a string throwing literally a type
error raising a type error would be
appropriate there and certainly
conventional but you could absolutely
just return false or some other such
value but it does tend to be an
exception centric language certainly
among the standard libraries so I don't
think you'd be abusing it at all in that
case it just changes the the approach to
programming but it does ultimately give
you a bit more expressiveness you just
end up perhaps writing a little more
code in order to catch different types
of exceptions but that's perhaps not a
bad thing so let's scroll back up in a
couple of comments here after Andres we
had here that user man two notes one
thing I want to point out is using print
for a prompt instead of the question in
input let me find that here might have
been in our other file input so using
input here indeed so I glossed over that
earlier and we were using print here
instead of the input function which we
could have been using here absolutely I
think we could have moved that text to
the input call especially if the human
doesn't cooperate and doesn't type Y or
n but maybe type something random
instead you could certainly imagine
wanting to reap romp them again and
again and again inside of this while
loop so I agree with user man too I
think there's probably an opportunity to
to clean that up a little bit or at
least take a different approach as well
vicious brain writes is it a good idea
to code some extra methods thinking that
one may use this in the future it
depends I think the question there not
unlike Andres is where you draw a line
in this case how where do you draw the
line between writing something that
works now versus something that's gonna
work better later you don't want to
necessarily get into the habit of
over-engineering your code because what
might be a very simple script with a few
dozen or a couple hundred lines of code
could mushroom into five hundred or
thousand lines because you're trying to
provide the kitchen sink so to speak to
do everything possible that you might
want in the future I think I at least in
recent years have been more of the
mindset of write design your code well
keep in mind features you might want to
have in the future but don't necessarily
implement all of those features now but
just be mindful of the implications of
having a different function so for
instance
if I were writing a reddit bot that was
only supposed to set the background on
Mac OS frankly I'm not probably gonna
bother writing the windows code or the
Linux code if I only care about my own
Mac however if I do want to open-source
the code and I could imagine maybe using
a PC down the road or a Linux box or
letting other people use this then maybe
just like our volunteer here can deal
for for one one I would engineer the
code in advance to have these
abstractions setting the background on a
generic operating system implementing
only the Mac OS version of that function
but knowing that eventually I or someone
else could contribute the windows or the
Linux versions instead so that might be
the line I wouldn't necessarily cut a
lot of corners just to get the code done
but design things well leave
placeholders for future features that
don't necessarily limit you in the near
term so hopefully that helps and just my
thoughts and hopefully a helpful food
for thought there all right so let's see
what else we've got in the chat let me
just flip through here some good chats
going on all right so I think we are now
caught up and that leads us really kind
of to the bottom of the file here we've
got upsetting above Mac background
windows background Linux backgrounds and
then lastly we've just got some error
handling in terms of exception handling
so I would say overall this was pretty
well written and in fact I really like
how it was again top bottom structured
chronologically this function calls this
one which calls this one really didn't
make me think I could focus entirely on
the code and the functionality some nice
comments throughout even though a few
more here and there probably would have
helped me at least and this abstraction
of those operating specific features
really well done as well and honestly I
think of the read means we've seen today
this is probably the most solid we've
got samples screenshots we've
highlighted and drawn attention to where
those values should be and read its UI
so I would encourage you absolutely to
take a look at this and all four of
today's repos so our thanks to to all of
our volunteers and actually notice this
this is one of the cool things about
open source contributions if you want to
not only get better at writing code and
getting feedback from the community you
can also get your acknowledgments here
so it looks like dec straws
actually contributed Mac support so I'm
guessing he or she wrote the Mac
specific function that was slotted in to
the bottle pile that we just went ahead
and took a look at so those are the four
repositories that we thought we'd take a
look at today I'm feel free to scroll up
in the chat if you'd like to see some of
those URLs again as always head to cs50
dot ly slash code review if you'd like
to submit your own code for review and
we're happy to take a look at it myself
Colton cs50 s team by all means as
before don't worry if you're feeling a
little less comfortable or if you're new
to programming that's the whole point
here but we would just ask that you make
the code public so that others can
certainly follow along on stream with us
and maybe even play with the code on
their own screens as well so puppet 4 4
1 1 I recognize the number you're very
welcome and really cool a really cool
program and very well documented and
designed so glad everyone's been able to
chime in so call denial linger here for
another minute or two if you have
questions but otherwise again Colton's
just pasted cs50.tv cs50 dot ly slash
code review go ahead at any point during
the week and submit some code that you
might like us to take a look at there it
can be most any language though ideally
one that he or I know as well as Brian
and other members of cs50 s team but
even then it's been a few years now
since I've written any PHP code but
hopefully I made my way through our
second example today mr. distinguished
you were quite welcome glad you tuned in
Gers morrow 3 glad you made two new
friends the trucha niece and the mr.
distinguished guys you are socializing
in addition to the twitch channel here
in facebook and youtube there's also
cs50 s discourse channel which has
gotten mentioned a few times as well
today and if you go to cs50 x dot slack
com is that the cs50 slack as well and
you can get generate an invite by going
to cs50 edx.org slash slack which will
automatically invite you to the slack
workspace if you'd like to join us there
as well Paul - today we don't have time
to consider today what we typically do
is take a look through everything in
advance just to make sure we know what
we're diving into but we'll hopefully
hold another one of these in the next
week or two so thank you for submitting
and we'll hope to get to that next time
we may alright so thank you so much to
Colton who's been sitting off-screen
here today and working his magic to make
sure everything went so smoothly thanks
to Babic knight and everyone for tuning
in Andrew as well who helped us get up
setup with the audio and video that's it
for today's code review this will be
available on demand later if you missed
the beginning or would like to go back
through some of the code and again all
of these repos were publicly available
you can read them off of the URL bars
and take a look at your own machine and
again lastly cs50 dot ly slash code
review if you'd like to submit your code
as well that's it and we'll see you soon