code review

[Littlefield, Tyler]

Hello all:
I was curious if someone wouldn't mind poking at some code.
I have an idea for a game I want to write (and if this works I want to
use this as a framework for another project), but I'd like to make sure
I'm doing things correctly/there's not a better way to do things. My
concern is I'm going to get way far into this, then realize I totally
broke something. So, if someone wouldn't mind taking a peek I'd
appreciate it. My concerns are:
1) style/cleanlyness: does everything look ok?
2) Workability: is there a better way to do what is there?
3) Speed: am I doing something that could be improved? I don't want to
spend a ton of time looking for non-existent bottlenecks and trying to
improve on them, but if I'm doing something that's bad, I'd like to fix it.

The project page is at:
http://code.google.com/p/pymud
Any information is greatly appreciated.

--
Take care,
Ty
http://tds-solutions.net
The aspen project: a barebones light-weight mud engine:
http://code.google.com/p/aspenmud
He that will not reason is a bigot; he that cannot reason is a fool; he that dares not reason is a slave.

 

[alex23]

I was curious if someone wouldn't mind poking at some code.
The project page is at:http://code.google.com/p/pymud
Any information is greatly appreciated.

I couldn't find any actual code at that site, the git repository is
currently empty.

 

[Steven D'Aprano]

I couldn't find any actual code at that site, the git repository is
currently empty.

Given that all code contains bugs, that's the best sort of repository!

 

[Chris Angelico]

Given that all code contains bugs, that's the best sort of repository!

Only in the sense that a cheese shop can be lauded for its cleanliness...

But I am somewhat curious to see the OP's actual code. MUDs are my
personal specialty.

ChrisA

 

[Littlefield, Tyler]

OOPS, sorry. Apparently I'm not as good with git as I thought.
Everything's in the repo now.

 

[Alister]

OOPS, sorry. Apparently I'm not as good with git as I thought.
Everything's in the repo now.

I am no expert but from what have picked up so far

from x import

is frowned upon in most cases

also this section in main strikes me as a bit odd and convoluted

w = world()
serv = server(client)
w.server = serv
serv.world = w

I think you are cross referencing classes & would be better to
investigate inheritance.

 

[MRAB]

I am no expert but from what have picked up so far

from x import

is frowned upon in most cases

I think you mean:

from x import *

 

[Martin P. Hellwig]

I am no expert but from what have picked up so far

from x import

is frowned upon in most cases

from x import * is frowned upon, however, from x import y is fine IMHO.

also this section in main strikes me as a bit odd and convoluted

w = world()
serv = server(client)
w.server = serv
serv.world = w

I think you are cross referencing classes & would be better to
investigate inheritance.

Generally speaking, read PEP8 and apply it please, there are tools like pylint that can help you with that. It also seems you are doing things quite java like, but I guess that is just a thing of getting used to python.

If you are planning to let your code being used like a framework that is extended by others, try to avoid more advanced functions just because they seem handy, always ask yourself is it clearer?

Try to unit-test your code and try to gain some decent code coverage, that will increase maturity of your code rather quickly.

But for the rest it looks like you are good in organizing it all in sub-modules, which is a very nice thing to see.

Good luck!

 

[Alister]

from x import * is frowned upon, however, from x import y is fine IMHO.

well I said I was no expert & picking things up. re investigation I see
your reasoning and yes it was the from X import * I was thinking of.

Although a simple import X retaining the name-space ref does make it easy
to identify the origins of a function (at the expense of more typing)

 

[Littlefield, Tyler]

I am no expert but from what have picked up so far from x import is
frowned upon in most cases also this section in main strikes me as a bit
odd and convoluted w = world() serv = server(client) w.server = serv
serv.world = w I think you are cross referencing classes & would be
better to investigate inheritance.

From what I understand and how I've always employed it, inheritance is
ment when you wish to give a class characteristics of another class. All
I'm doing here is setting the world and server classes on each other, so
they can call one another. This probably isn't needed in case of
serv.server = w, but for sure the other way around.

--
Take care,
Ty
http://tds-solutions.net
The aspen project: a barebones light-weight mud engine:
http://code.google.com/p/aspenmud
He that will not reason is a bigot; he that cannot reason is a fool; he that dares not reason is a slave.

 

[Dennis Lee Bieber]

Although a simple import X retaining the name-space ref does make it easy
to identify the origins of a function (at the expense of more typing)

Well, there is the middle point...

import some_long_module_name as slmn

which retains the individual namespace, but reduces the amount of
typing.

 

[Steven D'Aprano]

also this section in main strikes me as a bit odd and convoluted

w = world()
serv = server(client)
w.server = serv
serv.world = w

I think you are cross referencing classes & would be better to
investigate inheritance.

What you show above is composition, and is a perfectly valid technique,
and in fact should often be *preferred* to inheritance.

The only slightly dubious part, and I stress *slightly*, is that there is
a circular reference: w refers to serv, and serv refers back to w. While
legitimate, it is a very slight "code smell" that should be investigated.
If there is a way to get the same result without the circular reference,
that would be preferred.

For example, perhaps server methods that need to know the world could
take it as an explicit argument, rather than fetching it implicitly from
server.world.

Or, a moderately advanced technique, use a weak-ref instead.

Inheritance should only be used to model "is-a" relationships. For
example, Herbie the Love Bug is a Volkswagen Beetle, so inheritance is
appropriate.

http://en.wikipedia.org/wiki/Herbie


class Vehicle(object):
pass

class Car(Vehicle):
pass

class Beetle(Car):
pass

herbie = Beetle()

Composition should be used to model "has-a" relationships. For example, a
car has an engine, it is not a kind of engine, and so inheritance is
inappropriate and composition should be used. I would re-write the Car
class as follows:

class Engine(object):
pass

class Car(Vehicle):
def __init__(self):
self.engine = Engine()

So now we can talk about Herbie's engine:

herbie.engine # Herbie, being a car, has an engine, he is not an engine

 

[Terry Reedy]

I am no expert but from what have picked up so far from x import is
frowned upon in most cases

from x import *
# frowned on by many as reader will not necessarily know what that
imports, conflicts are possible, and if you import * twice, reader may
really not know what came from where

from x import y,x # fine, common, used in stdlib
import x as y # ditto
from x import y as z # ditto

 

[Terry Reedy]

well I said I was no expert & picking things up. re investigation I see
your reasoning and yes it was the from X import * I was thinking of.

Although a simple import X retaining the name-space ref does make it easy
to identify the origins of a function (at the expense of more typing)

import tkinter as tk
for example, to minimize extra typing while identifying source.
___
Terry Jan Reedy

 

[Alister]

What you show above is composition, and is a perfectly valid technique,
and in fact should often be *preferred* to inheritance.

The only slightly dubious part, and I stress *slightly*, is that there
is a circular reference: w refers to serv, and serv refers back to w.
While legitimate, it is a very slight "code smell" that should be
investigated.
If there is a way to get the same result without the circular reference,
that would be preferred.

For example, perhaps server methods that need to know the world could
take it as an explicit argument, rather than fetching it implicitly from
server.world.

Or, a moderately advanced technique, use a weak-ref instead.

Inheritance should only be used to model "is-a" relationships. For
example, Herbie the Love Bug is a Volkswagen Beetle, so inheritance is
appropriate.

http://en.wikipedia.org/wiki/Herbie


class Vehicle(object):
pass

class Car(Vehicle):
pass

class Beetle(Car):
pass

herbie = Beetle()

Composition should be used to model "has-a" relationships. For example,
a car has an engine, it is not a kind of engine, and so inheritance is
inappropriate and composition should be used. I would re-write the Car
class as follows:

class Engine(object):
pass

class Car(Vehicle):
def __init__(self):
self.engine = Engine()

So now we can talk about Herbie's engine:

herbie.engine # Herbie, being a car, has an engine, he is not an engine

I probably was not to clear (due to my own inexperience) it was the
circular references that grabbed my attention, it may be OK but suggests
to me there may be a better approach.

 

[Alister]

I am no expert but from what have picked up so far from x import is
frowned upon in most cases also this section in main strikes me as a bit
odd and convoluted w = world() serv = server(client) w.server = serv
serv.world = w I think you are cross referencing classes & would be
better to investigate inheritance.

From what I understand and how I've always employed it, inheritance is
ment when you wish to give a class characteristics of another class. All
I'm doing here is setting the world and server classes on each other, so
they can call one another. This probably isn't needed in case of
serv.server = w, but for sure the other way around.

I was not too sure of exactly why the code looked odd.
as mentioned in another post I should really have referred to the
circular references.

I am new to python (about 6 months of home hacking), I looked at the code
to see if it could improve my knowledge & my responses have been intended
to spark a 2 way discussion of the pro's & cons of the approach.
So far that seems to be working, I expect by the end of this I will have
learnt much about real world python apps.

 

[Alister]

I was not too sure of exactly why the code looked odd.
as mentioned in another post I should really have referred to the
circular references.

I am new to python (about 6 months of home hacking), I looked at the
code to see if it could improve my knowledge & my responses have been
intended to spark a 2 way discussion of the pro's & cons of the
approach.
So far that seems to be working, I expect by the end of this I will have
learnt much about real world python apps.

perhaps now is a good time for me to look at the rest of the modules to
see if i can work out exactly what these circular references do.
btw where or what is pants.py?

 

[Alister]

OOPS, sorry. Apparently I'm not as good with git as I thought.
Everything's in the repo now.

I think I may be on firmer grounds with the next few:

isValidPassword can be simplified to

def isValidPassword(password:
count=len(password)
return count>= mud.minpass and count<= mud.maxpass

( I used count to save finding the length of password twice although it
probably makes no difference in this scenario)

similar construct can be used for isValidUser

def isValidUser(name):
if name.isalpha():
count=len(name)
return count>=mud.minname and count >mud.maxname
return False

 

[Peter Otten]

I think I may be on firmer grounds with the next few:

isValidPassword can be simplified to

def isValidPassword(password:
count=len(password)
return count>= mud.minpass and count<= mud.maxpass

( I used count to save finding the length of password twice although it
probably makes no difference in this scenario)

If you spell it

def is_valid_password(password):
return mud.minpass <= len(password) <= mud.maxpass

it is even easier to see that you are performing an interval check.

 

[Thomas 'PointedEars' Lahn]

If you spell it

def is_valid_password(password):
return mud.minpass <= len(password) <= mud.maxpass

it is even easier to see that you are performing an interval check.

This is probably a tautology around here, but *what* *a* *great*
*programming* *language*.