Skip to main content
Code Review

Return to Answer

replaced http://codereview.stackexchange.com/ with https://codereview.stackexchange.com/
Source Link

Variable Names

Apart from the odd (non PEP8 compliant) variable casing, you also have a temp variable. Names like temp are frowned upon as they convey nothing about what that variable represents.

Repeated Tests

You do seem to repeat the tests c==',' or c=='}': a couple of times (lines 26 and 42. I think this could be refactored into using a test string, similar to your NoOps variable.

For example, on line 42, instead of:

 if c in NoOps or c==',' or c=='}':

You could just do

 if c in NoOps or c in CloseOps:

Assuming you had a variable CloseOps defined in the preamble, like this:

 CloseOps = ",}"

The same thing would apply for the repeated use of this test condition on line 26.

Repeated Logic

The following code (lines 45 - 49 inclusive):

 if lastKey:
 result[lastKey]=lastValue
 lastValue=''
 lastKey=''
 state=READY

Appears to be an (almost) verbatim repeat of the logic contained upon lines 29 - 33 (inclusive):

 if lastKey:
 result[lastKey]=lastValue
 lastValue=''
 lastKey=''
 state=READY

Functional Decomposition would dictate that such repeated logic be refactored into it's own function.

Numbers aren't Strings!

There exists a flaw in your code (which was not present in the answer code to your last question the answer code to your last question), where-by JSON-encoded numbers are turned into strings in your resulting dict.

If you (using your parse function implementation from this question) do:

 >>> type(parse('{"foo":"1"}')['foo'])

This yields:

 <type 'str'>

(Which is fine, since here "1" should be a string. However:

 >>> type(parse('{"foo":1}')['foo'])

Also yields:

 <type 'str'>

Which indicates an error, as this should be <type 'int'> if the JSON were decoded properly.

Variable Names

Apart from the odd (non PEP8 compliant) variable casing, you also have a temp variable. Names like temp are frowned upon as they convey nothing about what that variable represents.

Repeated Tests

You do seem to repeat the tests c==',' or c=='}': a couple of times (lines 26 and 42. I think this could be refactored into using a test string, similar to your NoOps variable.

For example, on line 42, instead of:

 if c in NoOps or c==',' or c=='}':

You could just do

 if c in NoOps or c in CloseOps:

Assuming you had a variable CloseOps defined in the preamble, like this:

 CloseOps = ",}"

The same thing would apply for the repeated use of this test condition on line 26.

Repeated Logic

The following code (lines 45 - 49 inclusive):

 if lastKey:
 result[lastKey]=lastValue
 lastValue=''
 lastKey=''
 state=READY

Appears to be an (almost) verbatim repeat of the logic contained upon lines 29 - 33 (inclusive):

 if lastKey:
 result[lastKey]=lastValue
 lastValue=''
 lastKey=''
 state=READY

Functional Decomposition would dictate that such repeated logic be refactored into it's own function.

Numbers aren't Strings!

There exists a flaw in your code (which was not present in the answer code to your last question), where-by JSON-encoded numbers are turned into strings in your resulting dict.

If you (using your parse function implementation from this question) do:

 >>> type(parse('{"foo":"1"}')['foo'])

This yields:

 <type 'str'>

(Which is fine, since here "1" should be a string. However:

 >>> type(parse('{"foo":1}')['foo'])

Also yields:

 <type 'str'>

Which indicates an error, as this should be <type 'int'> if the JSON were decoded properly.

Variable Names

Apart from the odd (non PEP8 compliant) variable casing, you also have a temp variable. Names like temp are frowned upon as they convey nothing about what that variable represents.

Repeated Tests

You do seem to repeat the tests c==',' or c=='}': a couple of times (lines 26 and 42. I think this could be refactored into using a test string, similar to your NoOps variable.

For example, on line 42, instead of:

 if c in NoOps or c==',' or c=='}':

You could just do

 if c in NoOps or c in CloseOps:

Assuming you had a variable CloseOps defined in the preamble, like this:

 CloseOps = ",}"

The same thing would apply for the repeated use of this test condition on line 26.

Repeated Logic

The following code (lines 45 - 49 inclusive):

 if lastKey:
 result[lastKey]=lastValue
 lastValue=''
 lastKey=''
 state=READY

Appears to be an (almost) verbatim repeat of the logic contained upon lines 29 - 33 (inclusive):

 if lastKey:
 result[lastKey]=lastValue
 lastValue=''
 lastKey=''
 state=READY

Functional Decomposition would dictate that such repeated logic be refactored into it's own function.

Numbers aren't Strings!

There exists a flaw in your code (which was not present in the answer code to your last question), where-by JSON-encoded numbers are turned into strings in your resulting dict.

If you (using your parse function implementation from this question) do:

 >>> type(parse('{"foo":"1"}')['foo'])

This yields:

 <type 'str'>

(Which is fine, since here "1" should be a string. However:

 >>> type(parse('{"foo":1}')['foo'])

Also yields:

 <type 'str'>

Which indicates an error, as this should be <type 'int'> if the JSON were decoded properly.

Source Link
Tersosauros
  • 944
  • 6
  • 16

Variable Names

Apart from the odd (non PEP8 compliant) variable casing, you also have a temp variable. Names like temp are frowned upon as they convey nothing about what that variable represents.

Repeated Tests

You do seem to repeat the tests c==',' or c=='}': a couple of times (lines 26 and 42. I think this could be refactored into using a test string, similar to your NoOps variable.

For example, on line 42, instead of:

 if c in NoOps or c==',' or c=='}':

You could just do

 if c in NoOps or c in CloseOps:

Assuming you had a variable CloseOps defined in the preamble, like this:

 CloseOps = ",}"

The same thing would apply for the repeated use of this test condition on line 26.

Repeated Logic

The following code (lines 45 - 49 inclusive):

 if lastKey:
 result[lastKey]=lastValue
 lastValue=''
 lastKey=''
 state=READY

Appears to be an (almost) verbatim repeat of the logic contained upon lines 29 - 33 (inclusive):

 if lastKey:
 result[lastKey]=lastValue
 lastValue=''
 lastKey=''
 state=READY

Functional Decomposition would dictate that such repeated logic be refactored into it's own function.

Numbers aren't Strings!

There exists a flaw in your code (which was not present in the answer code to your last question), where-by JSON-encoded numbers are turned into strings in your resulting dict.

If you (using your parse function implementation from this question) do:

 >>> type(parse('{"foo":"1"}')['foo'])

This yields:

 <type 'str'>

(Which is fine, since here "1" should be a string. However:

 >>> type(parse('{"foo":1}')['foo'])

Also yields:

 <type 'str'>

Which indicates an error, as this should be <type 'int'> if the JSON were decoded properly.

lang-py

AltStyle によって変換されたページ (->オリジナル) /