Skip to main content
Code Review

Return to Answer

edited body
Source Link

You are assigning c = i in both cases, which means you could pull this out and do this once unconditionally. This also will give you an idea of cs purpose and might give you ideas for how to name your variables in a better way. So trying to stay as close to your code as possible bitbut with the suggested changes, your code could look something along the lines:

inputString = 'azcbobobegghakl'
 
prevChar = ""
currentString = ""
longestString = ""
for char in inputString:
 if prevChar <= char:
 currentString += char
 if len(currentString) > len(longestString):
 longestString = currentString
 else:
 currentString = char
 prevChar = char
print('Longest substring in alphabetical order is: ' + longestString )

Also I suggest you think about edgecases and treat these cases or at very least you make it clear (at least in a comment) what happens in those cases. As an idea of what I mean: what happens if there are several longest-strings of the same length? Which one should be printed? All? The first? The last? Just any one? What happens if the inputString is empty - your message will look a bit funny. You could output a different message in such a case.

You might think I‘m a bit pedantic, but I seriously recommend to think about your code in this way.

Other than that, InmustI must admit I don‘t know python well enough to give you „the pythonic" answer - I‘m not even sure if they use camelCase for python - still I hope you get the idea of the type of improvements I‘m recommending. Other than that your code looks ok to me.

You are assigning c = i in both cases, which means you could pull this out and do this once unconditionally. This also will give you an idea of cs purpose and might give you ideas for how to name your variables in a better way. So trying to stay as close to your code as possible bit with the suggested changes, your code could look something along the lines:

inputString = 'azcbobobegghakl'
 
prevChar = ""
currentString = ""
longestString = ""
for char in inputString:
 if prevChar <= char:
 currentString += char
 if len(currentString) > len(longestString):
 longestString = currentString
 else:
 currentString = char
 prevChar = char
print('Longest substring in alphabetical order is: ' + longestString )

Also I suggest you think about edgecases and treat these cases or at very least you make it clear (at least in a comment) what happens in those cases. As an idea of what I mean: what happens if there are several longest-strings of the same length? Which one should be printed? All? The first? The last? Just any one? What happens if the inputString is empty - your message will look a bit funny. You could output a different message in such a case.

You might think I‘m a bit pedantic, but I seriously recommend to think about your code in this way.

Other than that, Inmust admit I don‘t know python well enough to give you „the pythonic" answer - I‘m not even sure if they use camelCase for python - still I hope you get the idea of the type of improvements I‘m recommending. Other than that your code looks ok to me.

You are assigning c = i in both cases, which means you could pull this out and do this once unconditionally. This also will give you an idea of cs purpose and might give you ideas for how to name your variables in a better way. So trying to stay as close to your code as possible but with the suggested changes, your code could look something along the lines:

inputString = 'azcbobobegghakl'
 
prevChar = ""
currentString = ""
longestString = ""
for char in inputString:
 if prevChar <= char:
 currentString += char
 if len(currentString) > len(longestString):
 longestString = currentString
 else:
 currentString = char
 prevChar = char
print('Longest substring in alphabetical order is: ' + longestString )

Also I suggest you think about edgecases and treat these cases or at very least you make it clear (at least in a comment) what happens in those cases. As an idea of what I mean: what happens if there are several longest-strings of the same length? Which one should be printed? All? The first? The last? Just any one? What happens if the inputString is empty - your message will look a bit funny. You could output a different message in such a case.

You might think I‘m a bit pedantic, but I seriously recommend to think about your code in this way.

Other than that, I must admit I don‘t know python well enough to give you „the pythonic" answer - I‘m not even sure if they use camelCase for python - still I hope you get the idea of the type of improvements I‘m recommending. Other than that your code looks ok to me.

Source Link

You are assigning c = i in both cases, which means you could pull this out and do this once unconditionally. This also will give you an idea of cs purpose and might give you ideas for how to name your variables in a better way. So trying to stay as close to your code as possible bit with the suggested changes, your code could look something along the lines:

inputString = 'azcbobobegghakl'
 
prevChar = ""
currentString = ""
longestString = ""
for char in inputString:
 if prevChar <= char:
 currentString += char
 if len(currentString) > len(longestString):
 longestString = currentString
 else:
 currentString = char
 prevChar = char
print('Longest substring in alphabetical order is: ' + longestString )

Also I suggest you think about edgecases and treat these cases or at very least you make it clear (at least in a comment) what happens in those cases. As an idea of what I mean: what happens if there are several longest-strings of the same length? Which one should be printed? All? The first? The last? Just any one? What happens if the inputString is empty - your message will look a bit funny. You could output a different message in such a case.

You might think I‘m a bit pedantic, but I seriously recommend to think about your code in this way.

Other than that, Inmust admit I don‘t know python well enough to give you „the pythonic" answer - I‘m not even sure if they use camelCase for python - still I hope you get the idea of the type of improvements I‘m recommending. Other than that your code looks ok to me.

lang-py

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