OCR-D /
core
| Conditions | 14 |
| Total Lines | 58 |
| Code Lines | 47 |
| Lines | 0 |
| Ratio | 0 % |
| Changes | 0 | ||
Small methods make your code easier to understand, in particular if combined with a good name. Besides, if your method is small, finding a good name is usually much easier.
For example, if you find yourself adding comments to a method's body, this is usually a good sign to extract the commented part to a new method, and use the comment as a starting point when coming up with a good name for this new method.
Commonly applied refactorings include:
If many parameters/temporary variables are present:
Complex classes like ocrd.cli.resmgr.download() often do a lot of different things. To break such a class down, we need to identify a cohesive component within that class. A common approach to find such a component is to look for fields/methods that share the same prefixes, or suffixes.
Once you have determined the fields that belong together, you can apply the Extract Class refactoring. If the component makes sense as a sub-class, Extract Subclass is also a candidate, and is often faster.
| 1 | import sys |
||
| 52 | @resmgr_cli.command('download') |
||
| 53 | @click.option('-n', '--any-url', help='Allow downloading unregistered resources', is_flag=True) |
||
| 54 | @click.option('-o', '--overwrite', help='Overwrite existing resources', is_flag=True) |
||
| 55 | @click.option('-l', '--location', help='Where to store resources', type=click.Choice(['cache', 'config', 'data', 'cwd']), default='cache', show_default=True) |
||
| 56 | @click.argument('executable', required=True) |
||
| 57 | @click.argument('url_or_name', required=True) |
||
| 58 | def download(any_url, overwrite, location, executable, url_or_name): |
||
| 59 | """ |
||
| 60 | Download resource URL_OR_NAME for processor EXECUTABLE. |
||
| 61 | |||
| 62 | URL_OR_NAME can either be the ``name`` or ``url`` of a registered resource. |
||
| 63 | |||
| 64 | If ``--any-url`` is given, also accepts URL of non-registered resources for ``URL_OR_NAME``. |
||
| 65 | """ |
||
| 66 | log = getLogger('ocrd.cli.resmgr') |
||
| 67 | resmgr = OcrdResourceManager() |
||
| 68 | basedir = XDG_CACHE_HOME if location == 'cache' else \ |
||
| 69 | XDG_DATA_HOME if location == 'data' else \ |
||
| 70 | XDG_CONFIG_HOME if location == 'config' else \ |
||
| 71 | getcwd() |
||
| 72 | is_url = url_or_name.startswith('https://') or url_or_name.startswith('http://') |
||
| 73 | find_kwargs = {'executable': executable} |
||
| 74 | find_kwargs['url' if is_url else 'name'] = url_or_name |
||
| 75 | reslist = resmgr.find_resources(**find_kwargs) |
||
| 76 | if not reslist: |
||
| 77 | log.info("No resources found in registry") |
||
| 78 | if is_url and any_url: |
||
| 79 | log.info("Downloading unregistered resource %s" % url_or_name) |
||
| 80 | with requests.head(url_or_name) as r: |
||
|
Comprehensibility
Best Practice
introduced
by
|
|||
| 81 | content_length = int(r.headers.get('content-length')) |
||
| 82 | with click.progressbar(length=content_length) as bar: |
||
| 83 | fpath = resmgr.download( |
||
| 84 | executable, |
||
| 85 | url_or_name, |
||
| 86 | overwrite=overwrite, |
||
| 87 | basedir=basedir, |
||
| 88 | progress_cb=lambda delta: bar.update(delta) |
||
|
1 ignored issue
–
show
|
|||
| 89 | ) |
||
| 90 | log.info("Downloaded %s to %s" % (url_or_name, fpath)) |
||
| 91 | log.info("Use in parameters as '%s'" % fpath.name) |
||
| 92 | else: |
||
| 93 | sys.exit(1) |
||
| 94 | else: |
||
| 95 | for _, resdict in reslist: |
||
| 96 | log.info("Downloading resource %s" % resdict) |
||
| 97 | with click.progressbar(length=resdict['size']) as bar: |
||
| 98 | fpath = resmgr.download( |
||
| 99 | executable, |
||
| 100 | resdict['url'], |
||
| 101 | name=resdict['name'], |
||
| 102 | resource_type=resdict['type'], |
||
| 103 | path_in_archive=resdict.get('path_in_archive', '.'), |
||
| 104 | overwrite=overwrite, |
||
| 105 | basedir=basedir, |
||
| 106 | progress_cb=lambda delta: bar.update(delta) |
||
|
1 ignored issue
–
show
|
|||
| 107 | ) |
||
| 108 | log.info("Downloaded %s to %s" % (resdict['url'], fpath)) |
||
| 109 | log.info("Use in parameters as '%s'" % resmgr.parameter_usage(resdict['name'], usage=resdict['parameter_usage'])) |
||
| 110 | |||
| 111 |
Loading history...