6
\$\begingroup\$

I've created a simple static website generator called pjen. The GitHub repository can be found here.

import os
class pjen:
 def __init__(self, path=None):
 """
 By default, the path will be set to the current working directory, 
 or the path can be specified using the 'path' keyword parameter.
 """
 if path is None:
 self.path = os.getcwd() 
 else:
 self.path = path
 self.path += "/website"
 def create_project(self):
 """
 Creates an initial file structure for a project at the specified path. 
 An exception is raised if a project already exists in the desired location.
 Creates the following file structure:
 website
 |-> templates
 |-> group1
 |-> images
 |-> scss
 |-> css
 |-> scripts
 """
 if not os.path.exists(self.path):
 #make the root directory
 os.makedirs(self.path)
 os.makedirs(self.path+"/templates")
 os.makedirs(self.path+"/templates/group1") 
 os.makedirs(self.path+"/images")
 os.makedirs(self.path+"/scss")
 os.makedirs(self.path+"/css")
 os.makedirs(self.path+"/scripts")
 print("Created project in {}".format(self.path))
 else:
 raise IOError("A directory already exists at: {}".format(self.path))
 def _count_indent(self, str, tab_equiv=4):
 """
 Returns the number of leading spaces. A tab is counted as 'tab_equiv' spaces.
 """
 i = 0
 count = 0
 while(str[i] == " " or str[i] == "\t"):
 if str[i] == " ":
 count += 1
 if str[i] == "\t":
 count += 4
 i += 1
 return count
 def _sanatise_file_list(self, l, fname=None):
 """
 Removes blacklisted files from the input list 'l'. In addition, a file with 
 the name 'fname' can also be removed.
 """
 blacklist = [".DS_Store"]
 if fname:
 blacklist.append(fname)
 for item in blacklist:
 try:
 l.pop(l.index(item))
 except ValueError:
 pass
 def generate(self):
 """
 Iterates through all the directories in the 'templates' directory, inserting all the 
 inputs into each template. 
 """
 #get a list of template groups
 static_groups = os.listdir(self.path+"/templates")
 self._sanatise_file_list(static_groups)
 print("Found {} group(s)".format(len(static_groups)))
 #iterate through each of the template groups
 for group in static_groups:
 #get each of the files in the group
 files = os.listdir(self.path+"/templates/"+group)
 #remove the template and hidden file
 self._sanatise_file_list(files, "template.html")
 #open the template 
 with open(self.path+"/templates/" + group + "/template.html", "r") as template:
 #iterate though the files that need to be generated
 for f in files:
 #create a new static page
 with open(self.path + "/" + f, "w") as page:
 print("Generating file: {}".format(f))
 #open up the input
 with open(self.path + "/templates/" + group + "/" + f, "r") as my_input:
 #iterate through the input and extract the various sections
 css = ""
 html = ""
 scripts = ""
 #flag to determine the section of the input file
 in_section = None
 #iterate through the input file and set the appropriate flag
 for line in my_input.readlines():
 if line.lstrip().startswith("{{ css }}"):
 in_section = "css"
 elif line.lstrip().startswith("{{ html }}"):
 in_section = "html"
 elif line.lstrip().startswith("{{ scripts }}"):
 in_section = "scripts"
 else:
 if in_section == "css":
 css += line
 if in_section == "html":
 html += line
 if in_section == "scripts":
 scripts += line
 #reset the file pointer as the template can be read many times
 template.seek(0)
 #iterate through the template file
 for line in template.readlines():
 if line.lstrip().startswith("{{ css }}"):
 indent = self._count_indent(line)
 #if there is css in the input file then insert it
 if css != "":
 for insert_line in [x + "\n" for x in css.split("\n")]:
 page.write(" "*indent + insert_line)
 #if there is html in the input file then insert it
 elif line.lstrip().startswith("{{ html }}"):
 indent = self._count_indent(line)
 if html != "":
 for insert_line in [x + "\n" for x in html.split("\n")]:
 page.write(" "*indent + insert_line)
 #if there are script links in the input file then insert them
 elif line.lstrip().startswith("{{ scripts }}"):
 indent = self._count_indent(line)
 if scripts != "":
 for insert_line in [x + "\n" for x in scripts.split("\n")]:
 page.write(" "*indent + insert_line)
 #otherwise copy the template text
 else:
 page.write(line)
if __name__ == "__main__":
 p = pjen()
 p.generate()
SuperBiasedMan
13.5k5 gold badges37 silver badges62 bronze badges
asked Dec 15, 2015 at 23:00
\$\endgroup\$

2 Answers 2

2
\$\begingroup\$

create_project doesn't have the best name. It should be create_project_folders if you want to be exact. Even if you don't want to be verbose, you're print call should be clearer about what exactly it does. I would flip your test, and use if os.path.exists(self.path) to raise IOError so then the rest of the function doesn't need to nested. Note that it also shows the flow better. In pseudocode:

create_project function:
 if project folder exists:
 raise error
 make folders

Speaking of making the folders, you repeatedly call the same function with new strings each time. This is perfect place to use a for loop. In addition, the strings should really be constants. Probably a set of constants at the class level:

class pjen:
 PROJECT_FOLDERS = (
 "/templates",
 "/templates/group1",
 "/images",
 "/scss",
 "/css",
 "/scripts",
 )

And now you can iterate over these folders to make them in create_project:

def create_project(self):
 if os.path.exists(self.path):
 raise IOError("A directory already exists at: {}".format(self.path))
 for folder in pjen.PROJECT_FOLDERS:
 os.makedirs(self.path + folder)
 print("Created project in {}".format(self.path))

Note that I haven't included os.makedirs(self.path). That's because Python makes all the directories necessary, not just the last one in the list (this is, in fact, why it's makedirs and not makedir). In the same way, there's no need to call os.makedirs(self.path + "/templates") and `os.makedirs(self.path + "/templates/group1"), as the latter would create both folders. However I kept it as a constant so that you can see the full list of folders created as that can be more readable, and you may want/need to iterate over all project folders at some point.

Don't use str as a variable name, as it will shadow the built in str function. Using string is usually better. I also don't like _count_indent starting with an underscore. That's a convention to signal that the function is private and shouldn't be used externally. I don't see why that should be the case here, apart from that you don't expect people to need it. But I'd personally leave the underscore off.

l is a bad name for an input list, either input_list or file_list would be much better. If fname is None then it'll be fine as it just wont have any effect on a list of strings. Also instead of appending a single fname to blacklist you could make a tuple:

 blacklist = (".DS_Store", fname)

In fact I'd iterate directly over that rather than assigning it to blacklist.

Also you're using pop to remove items, but you can just use remove. pop returns the value after removing that, if you don't need that then you don't need pop.

def _sanatise_file_list(self, l, fname=None):
 """
 Removes blacklisted files from the input list 'l'. In addition, a file with 
 the name 'fname' can also be removed.
 """
 for item in (".DS_Store", fname):
 try:
 l.pop(l.index(item))
 except ValueError:
 pass

generate is far too long as a function. You should break it up into smaller functions. Ideally one job per function. This helps readability, clarity and testing.

answered Dec 16, 2015 at 15:05
\$\endgroup\$
5
\$\begingroup\$

is is broken:

>>> 1000 is 10**3
False
>>> 1000 == 10**3
True

You can read more about this here, but in short it's better to use ==.


Ternary statements:

I really like ternaries, so I may be a little biased, but you can use ternary statements to simplify logic:

 if path is None:
 self.path = os.getcwd() 
 else:
 self.path = path

into:

 self.path = path if path != None else os.getcwd()

while loop:

Instead of using a while loop, you can simply iterate using a for loop:

(also, Python does not require explicit brackets around the loop)

(also, str[i] cannot be both " " and "\t", so you can use a elif for the second one.)

 while(str[i] == " " or str[i] == "\t"):
 if str[i] == " ":
 count += 1
 if str[i] == "\t":
 count += 4
 i += 1
 return count

into:

 for item in str:
 if item == " ":
 count += 1
 elif item == "\t":
 count += 4
 return count
answered Dec 15, 2015 at 23:17
\$\endgroup\$
2
  • \$\begingroup\$ Your for loop isn't equivalent because you're not breaking when you encounter a non-whitespace character. \$\endgroup\$ Commented Dec 16, 2015 at 14:50
  • \$\begingroup\$ Thanks for the suggestions - I really like the ternary statements and plan to add these in. As @SuperBiasedMan alluded to, I've used the while loop as I thought it was cleaner. \$\endgroup\$ Commented Dec 16, 2015 at 21:03

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.