\$\begingroup\$
\$\endgroup\$
5
I am working on my first open source project, and I have read a lot to get the desire coding style and I think I got it. But before go further I want to get reviews about my code. This script is not finished but I want to know if I'm on the right path.
# -*- coding: utf-8 -*-
"""config.py script, this file is part of BSR(Bolivian street racer) game."""
#################################################################
#
# Copyright (C) 2017 Pablo Cesar Chavez Suarez
#
# Autor : Pablo Cesar Chavez Suarez
#
# Email : [email protected]
#
# License : GLPv3
#
# year : 2017
#
# version : Blender 2.78
#
# If you use this code, please include this information header.
#
# This file is part of Bolivian street racer from now on called BSR.
#
# BSR is free software: you can redistribute it and/or modify
# it under the terms of the GNU General Public License as published by
# the Free Software Foundation, either version 3 of the License, or
# (at your option) any later version.
#
# BSR is distributed in the hope that it will be useful,
# but WITHOUT ANY WARRANTY; without even the implied warranty of
# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
# GNU General Public License for more details.
#
# You should have received a copy of the GNU General Public License
# along with BSR. If not, see <http://www.gnu.org/licenses/>.
#
#################################################################
from configparser import ConfigParser
from os.path import dirname, abspath, isfile
class Config():
"""This class read and write config.ini file."""
def __init__(self):
"""While initializing checks whether the configuration file exists."""
install_path = self.readInstallPathFromFile
pass
@property
def installPath(self):
"""Returns the full path to the root directory of the game."""
return '{}\\'.format(dirname(dirname(abspath(__file__))))
@property
def configFilePath(self):
"""Retursn the full path of the configuration file."""
return '{}'.format(self.installPath + 'config.ini')
def createEmptyConfigFile(self):
"""Creates the empty configuration file."""
with open(self.configFilePath,'w', encoding="utf-8") as file:
#file.closed
return
def addFileSection(self, section, path = ''):
"""Adds sections to the configuration file"""
conf = ConfigParser()
conf.readfp(open(self.configFilePath, encoding="utf-8"))
conf.add_section(section)
conf.set(section, 'path', path)
with open(self.configFilePath,'w', encoding="utf-8") as file:
conf.write(file)
@property
def readInstallPathFromFile(self):
"""Reads and returns the full path of the root directory
from the configuration file if it exists,
otherwise creates and writes the path to then return it."""
conf = ConfigParser()
if isfile(self.configFilePath):
conf.readfp(open(self.configFilePath, encoding="utf-8"))
if conf.has_section('install'):
return '{}'.format(conf.get('install','path'))
else:
self.addFileSection('install')
return '{}'.format(conf.get('install','path'))
else:
self.createEmptyConfigFile()
self.addFileSection('install', self.configFilePath)
conf.readfp(open(self.configFilePath, encoding="utf-8"))
return '{}'.format(conf.get('install','path'))
@property
def scenesPath(self):
"""Returns the scenes full Path"""
conf = ConfigParser()
conf.readfp(open(self.configFilePath, encoding="utf-8"))
if conf.has_section('scenes'):
return '{}'.format(conf.get('scenes','path'))
else:
self.addFileSection('scenes', self.installPath + 'scenes\\')
conf.readfp(open(self.configFilePath, encoding="utf-8"))
return '{}'.format(conf.get('scenes','path'))
@property
def vehiclesPath(self):
"""Returns the vehicles full Path"""
conf = ConfigParser()
conf.readfp(open(self.configFilePath, encoding="utf-8"))
if conf.has_section('vehicles'):
return '{}'.format(conf.get('vehicles','path'))
else:
self.addFileSection('vehicles', self.installPath + 'vehicles\\')
conf.readfp(open(self.configFilePath, encoding="utf-8"))
return '{}'.format(conf.get('vehicles','path'))
@property
def tracksPath(self):
"""Returns the tracks full Path"""
conf = ConfigParser()
conf.readfp(open(self.configFilePath, encoding="utf-8"))
if conf.has_section('tracks'):
return '{}'.format(conf.get('tracks','path'))
else:
self.addFileSection('tracks', self.installPath + 'tracks\\')
conf.readfp(open(self.configFilePath, encoding="utf-8"))
return '{}'.format(conf.get('tracks','path'))
pass
#For purposes of testing
config = Config()
print(config.installPath)
print(config.scenesPath)
print(config.vehiclesPath)
print(config.tracksPath)
#output
#c:\game\bsr\
#c:\game\bsr\scenes\
Jamal
35.2k13 gold badges134 silver badges238 bronze badges
1 Answer 1
\$\begingroup\$
\$\endgroup\$
1
In no particular order:
- Your code is unnecessarily indented 4 spaces; start everything top-level at the beginning of the lines.
- One newline between methods within a class.
- You need no
pass
at the end of__init__
ortracksPath
and possibly elsewhere. - "snake_case", rather than "camelCase", is the norm in Python. See PEP8.
file
is a built-in and should not be used as a local variable, like in yourwith
statements.- My personal feeling is that properties should not do a lot of "work", leaving that for regular methods. If you feel compelled to put a verb like "read" in the name, it's probably doing too much "work" for a property. (I admit that I am sometimes the worst offender of this rule.)
- I'm not crazy about function/method names being overly-complete sentences, like
readInstallPathFromFile
; I would probably go with one ofinstall_path
,read_install_path
orinstall_path_from_file
, depending on any distinctions I was trying to make (are there more places to get the install path than a file?), my mood and phase of the moon. - Use
os.path.join
to join path components, like you've done inconfigFilePath
. - Don't use
str.format
needlessly, like you've done inconfigFilePath
-- you string-concatenate the base path with the file name and then format them as themselves. And in a lot of other places--any place with'{}'.format(...)
. If you're doing that to ensure you've got a string instead of an integer or something, wrap it withstr()
. - You probably don't need the extra formatting in
installPath
if you useos.path.join
; you're adding the trailing backslash so your string-concatenation inconfigFilePath
works. - I don't like the repetitious file opening & parsing. Read your config and keep it in an instance variable. (Unless this is a special case like a huge file or something that could be rewritten without you knowing it.)
- I also don't like all the schema construction upon read -- add all the sections when you create the empty file or when you first open the file. Do it by iterating over a list of sections, so you have the section list in one canonical location for reference. (NB: There might be things about ConfigParser that make this not feasible; I've used it and remember it was fiddly in some respects but it's been a few years.)
- Run flake8 on your code and fix what it does not like.
-
\$\begingroup\$ I did not notice the indentation, but it was generated when I copy the code to the question and I will try to improve the code with your suggestions, thanks. \$\endgroup\$Strapicarus– Strapicarus2017年03月10日 20:33:11 +00:00Commented Mar 10, 2017 at 20:33
Explore related questions
See similar questions with these tags.
lang-py
file.closed
do? \$\endgroup\$