submitting Homework 1 #3

Merged
merged 1 commit into from Nov 30, 2018

Conversation

Projects
None yet
2 participants
@ackalil

ackalil commented Nov 28, 2018

No description provided.

@vodnik

vodnik approved these changes Nov 30, 2018

bottles = 'bottle';
console.log(i + ' ' + bottles + ' of beer on the wall, ' + i + ' ' + bottles + ' of beer. \n Take one down and pass it around, no more bottles of beer on the wall.');
} else if ((i === 0)) {
console.log('No more bottles of beer on the wall, no more bottles of beer. \n Go to the store and buy some more, 99 bottles of beer on the wall.' + String.fromCodePoint(0x1F37b));

This comment has been minimized.

@vodnik

vodnik Nov 30, 2018

Owner

The icon is a nice touch. I love that you went the extra mile to do that!



for (i = 99; i >= 0; i--) {
let bottles = 'bottles';

This comment has been minimized.

@vodnik

vodnik Nov 30, 2018

Owner

Great idea to abstract the singular/plural word here. It seems possible for there to be some confusion here between the variable name ('bottles') and the potential values (either 'bottle' or 'bottles'), so I'd suggest in the future choosing a variable name that's not the same as its value -- something like bottleWord here, for instance.

} else if ((i === 1)) {
bottles = 'bottle';
console.log(i + ' ' + bottles + ' of beer on the wall, ' + i + ' ' + bottles + ' of beer. \n Take one down and pass it around, no more bottles of beer on the wall.');
} else if ((i === 0)) {

This comment has been minimized.

@vodnik

vodnik Nov 30, 2018

Owner

Your if/else if logic is solid. Here I'd suggest leaving off the final if and condition, because if the loop reaches this code, there's no value possible except 0, right?

// Step 1: Array with dummy data
let numbers = [62, 494, 662, 780, 772, 512];

let streets = ['27th Avenue', 'Hilltop Terrace', 'Pine Ridge Road', 'Commonwealth Avenue', 'Bryant Street', 'Market Street'];

This comment has been minimized.

@vodnik

vodnik Nov 30, 2018

Owner

Comm Ave! 🎓


let states = ['CA', 'VA', 'MA', 'IL', 'MN', 'WV'];

let zips = [94121, 22301, 94107, '02468', '02446', '02115'];

This comment has been minimized.

@vodnik

vodnik Nov 30, 2018

Owner

Note that you included a combination of numbers and strings here in the zips array. Given that you're concatenating values into a string, I'd suggest using all strings here.

@vodnik vodnik merged commit 786f839 into vodnik:master Nov 30, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment