Skip to main content
Code Review

Return to Answer

Commonmark migration
Source Link
Community Bot
Community Bot
  • 1

I don't see any performance bottlenecks in the traditional sense. Namely I don't see any tight loops that have to do a lot of work where you are likely to be waiting for the computer to finish. Instead what I see are a lot of small opportunities for you to make the code a little easier to read. Here I'll go over a few ways you might approach doing so:

Finding alternate ways to perform arithmetic###arithmetic

After getting strength and skill values from the person running the program, you compute the difference between the values entered. If it's negative, you invert it, and then divide by five. This sounds a lot like taking the absolute value of the difference, then dividing by five. The advantage of using an absolute value function to do this is that you hide the if/else code so you don't have to read through it later. A very similar approach is available for ensuring that the skills don't go below zero, using a maximum or minimum function.

Finding ways to avoid repetition

After you have the strengths, skills, differences, and "rolls" (num1 and num2), you have two branches of code which are largely the same. If num1 is bigger than num2 then the first character wins the encounter. If num2 is larger, the second character wins.Since both of the if statements execute the same operation, but on different variables, it would be nice if you could give the variables a new name, say winner and loser, then add points to the winner and subtract them from the loser. However, in order to do that, you would have to change the storage of your strength and skill.

Small things

It might be helpful to include which character's skill the program is requesting you enter at the beginning. Without that, it might look weird that the program asks you two questions twice.

It's typically less idiomatic to compare a value to True as the condition of a while loop. Instead prefer to use the value itself as the condition: while play1: .... Or, better still, it may make sense to incorporate your later conditions into the while loop, for example making its condition whether both of the character's strength are still above zero. If they are not, the while loop will end, and you can print the winner.

I don't see any performance bottlenecks in the traditional sense. Namely I don't see any tight loops that have to do a lot of work where you are likely to be waiting for the computer to finish. Instead what I see are a lot of small opportunities for you to make the code a little easier to read. Here I'll go over a few ways you might approach doing so:

Finding alternate ways to perform arithmetic###

After getting strength and skill values from the person running the program, you compute the difference between the values entered. If it's negative, you invert it, and then divide by five. This sounds a lot like taking the absolute value of the difference, then dividing by five. The advantage of using an absolute value function to do this is that you hide the if/else code so you don't have to read through it later. A very similar approach is available for ensuring that the skills don't go below zero, using a maximum or minimum function.

Finding ways to avoid repetition

After you have the strengths, skills, differences, and "rolls" (num1 and num2), you have two branches of code which are largely the same. If num1 is bigger than num2 then the first character wins the encounter. If num2 is larger, the second character wins.Since both of the if statements execute the same operation, but on different variables, it would be nice if you could give the variables a new name, say winner and loser, then add points to the winner and subtract them from the loser. However, in order to do that, you would have to change the storage of your strength and skill.

Small things

It might be helpful to include which character's skill the program is requesting you enter at the beginning. Without that, it might look weird that the program asks you two questions twice.

It's typically less idiomatic to compare a value to True as the condition of a while loop. Instead prefer to use the value itself as the condition: while play1: .... Or, better still, it may make sense to incorporate your later conditions into the while loop, for example making its condition whether both of the character's strength are still above zero. If they are not, the while loop will end, and you can print the winner.

I don't see any performance bottlenecks in the traditional sense. Namely I don't see any tight loops that have to do a lot of work where you are likely to be waiting for the computer to finish. Instead what I see are a lot of small opportunities for you to make the code a little easier to read. Here I'll go over a few ways you might approach doing so:

Finding alternate ways to perform arithmetic

After getting strength and skill values from the person running the program, you compute the difference between the values entered. If it's negative, you invert it, and then divide by five. This sounds a lot like taking the absolute value of the difference, then dividing by five. The advantage of using an absolute value function to do this is that you hide the if/else code so you don't have to read through it later. A very similar approach is available for ensuring that the skills don't go below zero, using a maximum or minimum function.

Finding ways to avoid repetition

After you have the strengths, skills, differences, and "rolls" (num1 and num2), you have two branches of code which are largely the same. If num1 is bigger than num2 then the first character wins the encounter. If num2 is larger, the second character wins.Since both of the if statements execute the same operation, but on different variables, it would be nice if you could give the variables a new name, say winner and loser, then add points to the winner and subtract them from the loser. However, in order to do that, you would have to change the storage of your strength and skill.

Small things

It might be helpful to include which character's skill the program is requesting you enter at the beginning. Without that, it might look weird that the program asks you two questions twice.

It's typically less idiomatic to compare a value to True as the condition of a while loop. Instead prefer to use the value itself as the condition: while play1: .... Or, better still, it may make sense to incorporate your later conditions into the while loop, for example making its condition whether both of the character's strength are still above zero. If they are not, the while loop will end, and you can print the winner.

Source Link
Michael Urman
Michael Urman
  • 3.9k
  • 1
  • 12
  • 23

I don't see any performance bottlenecks in the traditional sense. Namely I don't see any tight loops that have to do a lot of work where you are likely to be waiting for the computer to finish. Instead what I see are a lot of small opportunities for you to make the code a little easier to read. Here I'll go over a few ways you might approach doing so:

Finding alternate ways to perform arithmetic###

After getting strength and skill values from the person running the program, you compute the difference between the values entered. If it's negative, you invert it, and then divide by five. This sounds a lot like taking the absolute value of the difference, then dividing by five. The advantage of using an absolute value function to do this is that you hide the if/else code so you don't have to read through it later. A very similar approach is available for ensuring that the skills don't go below zero, using a maximum or minimum function.

Finding ways to avoid repetition

After you have the strengths, skills, differences, and "rolls" (num1 and num2), you have two branches of code which are largely the same. If num1 is bigger than num2 then the first character wins the encounter. If num2 is larger, the second character wins.Since both of the if statements execute the same operation, but on different variables, it would be nice if you could give the variables a new name, say winner and loser, then add points to the winner and subtract them from the loser. However, in order to do that, you would have to change the storage of your strength and skill.

Small things

It might be helpful to include which character's skill the program is requesting you enter at the beginning. Without that, it might look weird that the program asks you two questions twice.

It's typically less idiomatic to compare a value to True as the condition of a while loop. Instead prefer to use the value itself as the condition: while play1: .... Or, better still, it may make sense to incorporate your later conditions into the while loop, for example making its condition whether both of the character's strength are still above zero. If they are not, the while loop will end, and you can print the winner.