jusongchen / pythonLearning

0 stars 0 forks source link

Salesforce Core Python class - day 2 lab exercises code review #1

Open jusongchen opened 6 years ago

jusongchen commented 6 years ago

@davewadestein Hi Dave,

This is Jusong from Salesforce who attended Core Python class at Salesforce. I completed day 2 lab exercises and checked in my source code at: word count reverse lines

Is it possible for you to do quick code review and give advice on where to improve?

I greatly appreciate your time and help.

Regards,

Jusong Chen

davewadestein commented 6 years ago

Hi Jusong,

Was out with a friend last night so I didn’t get to this.

The word count program looks great to me. My only comments would be:

1) In the line

in_file = open(in_file_name, 'rt’)

you could leave out ‘rt’ since read mode and text files are the default.

2) I don’t think the close() call should be in the finally block. If an exception is throw, presumably it will be because the file couldn’t be opened. Therefore the close() will fail. So I’d put it in the ‘else’ block of the exception handler.

Nice use of translate() to remove punctuation. I usually teach the explode/join method that is used in this.py, since it’s something we already covered, but I generally mention the existence of translate().

For the reverse lines program, I wasn’t expecting the lines to be sorted, but rather, just output in reverse order.

Best, Dave

On Oct 5, 2017, at 7:29 PM, jusongchen notifications@github.com wrote:

@davewadestein https://github.com/davewadestein Hi Dave,

This is Jusong from Salesforce who attended Core Python class at Salesforce. I completed day 2 lab exercises and checked in my source code at: word count https://github.com/jusongchen/pythonLearning/blob/master/execises/fileIoWordCnt.py reverse lines https://github.com/jusongchen/pythonLearning/blob/master/execises/fileIoReversLine.py Is it possible for you to do quick code review and give advice on where to improve?

I greatly appreciate your time and help.

Regards,

Jusong Chen

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/jusongchen/pythonLearning/issues/1, or mute the thread https://github.com/notifications/unsubscribe-auth/AOq45j_VzMmudJArrf8-OdwDqpMJRu4Uks5spZCYgaJpZM4Pv7xc.

jusongchen commented 6 years ago

Hi Dave,

Great review and it's very valuable for me! Thank you so much! I fixed two issues pointed out by you in the word count program. For the line reserve exercise, I misread the requirement.

I complete today's function lab. here is the code with testing included:

https://github.com/jusongchen/pythonLearning/blob/master/execises/function_lab.py

Is it possible for you to do a quick review?

I also have questions w.r.t. validating arguments passed-in to a function:

1) Is it a good practice for a function always check types of the arguments passed in? Or, is it the function caller's responsibility to ensure arguments with right type is passed in when invoking a function?

e.g. given code below, are the very first two if statements necessary?

def sumdigits(num): ''' takes an integer as a parameter, and sums up its digits. If the resulting sum contains more than 1 digit, the function should sum the digits again, e.g., sumdigits(1235) should compute the sum of 1, 2, 3, and 5 (11), then compute the sum of 1 and 1, returning 2. ''' if not isinstance(num, int): raise TypeError('Expect an integer, but get {}'.format(num)) if num < 0: raise ValueError( 'Expecting a non negitive number, but get:{}'.format(num))

2) Here are two ways I do type validation,

if not isinstance(num, int): raise TypeError('Expect an integer, but get {}'.format(num))

for arg in numbers: assert isinstance(arg, int) or isinstance(arg, float), "expect all args to be int or float"

Which one do you consider to be a good practice?

Your time is greatly appreciated!

Best,

Jusong

On Fri, Oct 6, 2017 at 8:54 AM, Dave Wade-Stein notifications@github.com wrote:

Hi Jusong,

Was out with a friend last night so I didn’t get to this.

The word count program looks great to me. My only comments would be:

1) In the line

in_file = open(in_file_name, 'rt’)

you could leave out ‘rt’ since read mode and text files are the default.

2) I don’t think the close() call should be in the finally block. If an exception is throw, presumably it will be because the file couldn’t be opened. Therefore the close() will fail. So I’d put it in the ‘else’ block of the exception handler.

Nice use of translate() to remove punctuation. I usually teach the explode/join method that is used in this.py, since it’s something we already covered, but I generally mention the existence of translate().

For the reverse lines program, I wasn’t expecting the lines to be sorted, but rather, just output in reverse order.

Best, Dave

On Oct 5, 2017, at 7:29 PM, jusongchen notifications@github.com wrote:

@davewadestein https://github.com/davewadestein Hi Dave,

This is Jusong from Salesforce who attended Core Python class at Salesforce. I completed day 2 lab exercises and checked in my source code at: word count https://github.com/jusongchen/pythonLearning/ blob/master/execises/fileIoWordCnt.py reverse lines https://github.com/jusongchen/pythonLearning/ blob/master/execises/fileIoReversLine.py Is it possible for you to do quick code review and give advice on where to improve?

I greatly appreciate your time and help.

Regards,

Jusong Chen

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/ jusongchen/pythonLearning/issues/1, or mute the thread < https://github.com/notifications/unsubscribe-auth/AOq45j_VzMmudJArrf8- OdwDqpMJRu4Uks5spZCYgaJpZM4Pv7xc>.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/jusongchen/pythonLearning/issues/1#issuecomment-334795701, or mute the thread https://github.com/notifications/unsubscribe-auth/AFWG53tRgW7N7wNkyi8p8CNgn6bCdd7-ks5spk0ogaJpZM4Pv7xc .

jusongchen commented 6 years ago

I have completed another two labs:

https://github.com/jusongchen/pythonLearning/blob/master/execises/args_parse.py https://github.com/jusongchen/pythonLearning/blob/master/execises/PrulaizationLab.py

On Fri, Oct 6, 2017 at 10:41 AM, Jusong Chen jusong.chen@gmail.com wrote:

Hi Dave,

Great review and it's very valuable for me! Thank you so much! I fixed two issues pointed out by you in the word count program. For the line reserve exercise, I misread the requirement.

I complete today's function lab. here is the code with testing included:

https://github.com/jusongchen/pythonLearning/blob/master/ execises/function_lab.py

Is it possible for you to do a quick review?

I also have questions w.r.t. validating arguments passed-in to a function:

1) Is it a good practice for a function always check types of the arguments passed in? Or, is it the function caller's responsibility to ensure arguments with right type is passed in when invoking a function?

e.g. given code below, are the very first two if statements necessary?

def sumdigits(num): ''' takes an integer as a parameter, and sums up its digits. If the resulting sum contains more than 1 digit, the function should sum the digits again, e.g., sumdigits(1235) should compute the sum of 1, 2, 3, and 5 (11), then compute the sum of 1 and 1, returning 2. ''' if not isinstance(num, int): raise TypeError('Expect an integer, but get {}'.format(num)) if num < 0: raise ValueError( 'Expecting a non negitive number, but get:{}'.format(num))

2) Here are two ways I do type validation,

if not isinstance(num, int): raise TypeError('Expect an integer, but get {}'.format(num))

for arg in numbers: assert isinstance(arg, int) or isinstance(arg, float), "expect all args to be int or float"

Which one do you consider to be a good practice?

Your time is greatly appreciated!

Best,

Jusong

On Fri, Oct 6, 2017 at 8:54 AM, Dave Wade-Stein notifications@github.com wrote:

Hi Jusong,

Was out with a friend last night so I didn’t get to this.

The word count program looks great to me. My only comments would be:

1) In the line

in_file = open(in_file_name, 'rt’)

you could leave out ‘rt’ since read mode and text files are the default.

2) I don’t think the close() call should be in the finally block. If an exception is throw, presumably it will be because the file couldn’t be opened. Therefore the close() will fail. So I’d put it in the ‘else’ block of the exception handler.

Nice use of translate() to remove punctuation. I usually teach the explode/join method that is used in this.py, since it’s something we already covered, but I generally mention the existence of translate().

For the reverse lines program, I wasn’t expecting the lines to be sorted, but rather, just output in reverse order.

Best, Dave

On Oct 5, 2017, at 7:29 PM, jusongchen notifications@github.com wrote:

@davewadestein https://github.com/davewadestein Hi Dave,

This is Jusong from Salesforce who attended Core Python class at Salesforce. I completed day 2 lab exercises and checked in my source code at: word count https://github.com/jusongchen/pythonLearning/blob/master/ execises/fileIoWordCnt.py reverse lines https://github.com/jusongchen /pythonLearning/blob/master/execises/fileIoReversLine.py Is it possible for you to do quick code review and give advice on where to improve?

I greatly appreciate your time and help.

Regards,

Jusong Chen

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub < https://github.com/jusongchen/pythonLearning/issues/1>, or mute the thread https://github.com/notifications/unsubscribe-auth/AOq45j_ VzMmudJArrf8-OdwDqpMJRu4Uks5spZCYgaJpZM4Pv7xc.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/jusongchen/pythonLearning/issues/1#issuecomment-334795701, or mute the thread https://github.com/notifications/unsubscribe-auth/AFWG53tRgW7N7wNkyi8p8CNgn6bCdd7-ks5spk0ogaJpZM4Pv7xc .

jusongchen commented 6 years ago

@davewadestein Hi Dave, Great review and it's very valuable to me! Thank you so much! I fixed two issues pointed out by you in the word count program. For the line reserve exercise, I misread the requirement.

I have completed three of today's lab exercises. here is the code with testing included: https://github.com/jusongchen/pythonLearning/blob/master/execises/function_lab.py https://github.com/jusongchen/pythonLearning/blob/master/execises/args_parse.py https://github.com/jusongchen/pythonLearning/blob/master/execises/PrulaizationLab.py

Is it possible for you to do a quick review?

I also have questions w.r.t. validating arguments passed-in to a function: Is it a good practice for a function always check types of the arguments passed in? Or, is it the function caller's responsibility to ensure arguments with right type is passed in when invoking a function?

Your time is greatly appreciated! Best, Jusong

jusongchen commented 6 years ago

@davewadestein

I have completed the Calculator Class exercise. Any comment/feedback is highly appreciated. https://github.com/jusongchen/pythonLearning/blob/master/execises/CalculatorClass.py

davewadestein commented 6 years ago

Hello Jusong,

I can maybe look tonight. My mother is unexpectedly ill, and I ended up flying to Los Angeles last night instead of home to Colorado.

I’ll be in touch.

Best, Dave

On Oct 6, 2017, at 6:39 PM, jusongchen notifications@github.com wrote:

@davewadestein https://github.com/davewadestein I have completed the Calculator Class exercise. HAny comment/feedback is highly appreciated. https://github.com/jusongchen/pythonLearning/blob/master/execises/CalculatorClass.py https://github.com/jusongchen/pythonLearning/blob/master/execises/CalculatorClass.py — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/jusongchen/pythonLearning/issues/1#issuecomment-334902215, or mute the thread https://github.com/notifications/unsubscribe-auth/AOq45hY7bT4H6NpdRqJ3GkvpISKAfJPhks5sptZAgaJpZM4Pv7xc.