Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor some setBlocks code into the next API #7

Open
jonathanfine opened this issue Sep 25, 2014 · 4 comments
Open

Refactor some setBlocks code into the next API #7

jonathanfine opened this issue Sep 25, 2014 · 4 comments

Comments

@jonathanfine
Copy link
Contributor

Here's some code from castle.py in https://bitbucket.org/MattHawkinsUK/rpispy-misc/.

This task in this issue is to refactor this fragment (and agree on the outcome).

def CreateWalls(size,baseheight,height,material,battlements,walkway):
  # Create 4 walls with a specified width, height and material.
  # Battlements and walkways can also be added to the top edges.

  mc.setBlocks(-size,baseheight+1,-size,size,baseheight+height,-size,material) 
  mc.setBlocks(-size,baseheight+1,-size,-size,baseheight+height,size,material)
  mc.setBlocks(size,baseheight+1,size,-size,baseheight+height,size,material) 
  mc.setBlocks(size,baseheight+1,size,size,baseheight+height,-size,material) 

 [snip]
@jonathanfine
Copy link
Contributor Author

Some comments on the code

  1. There's a dependence on the mc global.
  2. At first glance it seems that swapping size and -size does not change the output.
  3. But it's hard to be sure.
  4. The docstring is comments, and does not say how to specify the width.
  5. It seems that the baseheight raises the whole castle.

Towards an alternative implementation

Think CSS (which is in any case a useful skill). We have a box / rectangular region, and we're padding it.

I think something like this might work (except for the troublesome global mc).

class Padding:

    def __init__(self, interior, vec1, vec2):
          # To follow.

pad = Padding(box, (-1, -1, 0), (1, 1, 0))
pad.block = material

To put up another set of walls

pad2 = pad + vec
pad2.block = material

@dbrgn
Copy link
Member

dbrgn commented Sep 25, 2014

Well, the primary question should be an architectural one. Do we want a collection of useful functions like create_wall, where you can pass in the mc reference?

Or do we want an OOP style where you can create Wall objects?

@jonathanfine
Copy link
Contributor Author

Sorry, @dbrgn, but it's not clear to me what you mean. Perhaps you could provide an example or two.

@dbrgn
Copy link
Member

dbrgn commented Sep 28, 2014

A functional approach would be

def create_walls(mc, x, y, z, width, depth, height):
    # do stuff with mc reference

An object oriented example would be:

class Walls:
    def __init__(self, mc, width, depth, height):
        # do stuff

    def place_at(self, x, y, z):
        # create walls on `mc` instance

The first approach is a clear command to the referenced minecraft instance: Create walls with the specified dimensions at the specified place.

The second approach requires to understand classes, instances and methods.

I think the functional approach is easier to understand than creating objects and manipulating the contained data via method calls. As I'd focus on children and beginners, I'd favor a "flat", non-OOP style (e.g. not using vector classes by deafult) even though as a software engineer I know that object orientation has a lot of benefits.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants