HiveBrain v1.2.0
Get Started
← Back to all entries
patternpythonMinor

Racetrack plotter

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
plotterracetrackstackoverflow

Problem

My Racetrack is just that. A Racetrack. You can't race it (yet) because I had trouble with collision detection, but I wanted to share it anyway.

It creates a base polygon by using numpy and shapely. matplotlib (together with descartes) does the plotting and cPickle is used to write states to file so the exact same track can be plotted later.

This is my first Python script using argparse instead of sys for argument handling. I'm especially interested in whether the way I implemented it is up to par.

The way it's structured is probably not the best either and as always I'm not too fond of my naming. I stuck to -r and -w for reading and writing because it seemed intuitive, but deeper in the code load_state and save_state are used. I'm not sure this is acceptable and which of both I should stick with (if any of those at all).

Under the argument parsing I have a couple of BOLD_SNAKE_CASE variables which are pseudo constants. There's probably a better way to do this. Some of those can be freely changed by the user, others shouldn't. I think it's self explanatory enough, but feel free to comment.

As said, it was supposed to be part of an actual Racetrack-game. So extendability is important. I like extra features, but those are a pain in the behind to implement if the code isn't modular.

```
import numpy as np
import matplotlib.pyplot as plt
import shapely.geometry as sg
from argparse import ArgumentParser
from descartes.patch import PolygonPatch
from cPickle import load, dump

# Argument handling
parser = ArgumentParser(description='Racetrack')
parser.add_argument(
'InnerAmplitude',
type=float,
help='For example 0.1'
)
parser.add_argument(
'OuterAmplitude',
type=float,
help='For example 0.2, should be higher than InnerAmplitude'
)
parser.add_argument(
"-v",
"--verbose",
action="store_true",
help="increase output verbosity"
)
mutex = parser.add_mutually_exclusive_group()
mutex.add_argument(

Solution

This is my first Python script using argparse instead of sys for argument handling. I'm especially interested in whether the way I implemented it is up to par.

You nailed it :-)


Under the argument parsing I have a couple of BOLD_SNAKE_CASE variables which are pseudo constants. There's probably a better way to do this.

That's the common practice in Python, and it's fine like that.
Except for these:

INNER_AMPLITUDE = args.InnerAmplitude
OUTER_AMPLITUDE = args.OuterAmplitude
DIAMETER = OUTER_AMPLITUDE - INNER_AMPLITUDE


As these are values derived from the command line arguments.
The parsing logic would be better in a main() function,
which will determine the value of diameter,
and pass it to a plot(diameter) function that will take care of the plotting logic.

The biggest issue I see is with the layout:

  • Imports



  • Argument parser



  • Constants



  • Some functions



  • Argument handling



  • Another function



  • The main plotting logic



I suggest to reorganize like this:

  • Imports



  • Constants



  • Helper functions



  • def plot(): The main plotting logic



  • def main():



  • Argument parser



  • Argument handling



  • if __name__ == '__main__': guard that simply calls main()



After this change, I think it will look a lot better and clearer.

Code Snippets

INNER_AMPLITUDE = args.InnerAmplitude
OUTER_AMPLITUDE = args.OuterAmplitude
DIAMETER = OUTER_AMPLITUDE - INNER_AMPLITUDE

Context

StackExchange Code Review Q#102080, answer score: 7

Revisions (0)

No revisions yet.