# Class modelling for a shogi notation reader

I have made GPL software in GitHub whose purpose is reading shogi notations (shogi is Japanese chess). I have been told that my software modelling is underdeveloped in this question and advised to post a question about the classes I made to manage the board, so here I go:

The main concern is about the file managers.py, which contains the classes that I call "managers". Two of those managers are for handling the board's coords for pieces.

One of them is the class coords_manager - managers.py line 27:

class coords_manager:
def __init__(self):
# Pieces arrays
self.lista_pn = None
self.lista_spn = None
self.cnt_pn = None
self.rpn = None
self.lista_pb = None
self.lista_spb = None
self.cnt_pb = None
self.rpb = None
self.lista_ln = None
self.lista_sln = None
self.cnt_ln = None
self.rln = None
self.lista_lb = None
self.lista_slb = None
self.cnt_lb = None
self.rlb = None
self.lista_nn = None
self.lista_snn = None
self.cnt_nn = None
self.rnn = None
self.lista_nb = None
self.lista_snb = None
self.cnt_nb = None
self.rnb = None
self.lista_sn = None
self.lista_ssn = None
self.cnt_sn = None
self.rsn = None
self.lista_sb = None
self.lista_ssb = None
self.cnt_sb = None
self.rsb = None
self.lista_gn = None
self.cnt_gn = None
self.rgn = None
self.lista_gb = None
self.cnt_gb = None
self.rgb = None
self.lista_tn = None
self.lista_stn = None
self.cnt_tn = None
self.rtn = None
self.lista_tb = None
self.lista_stb = None
self.cnt_tb = None
self.rtb = None
self.lista_bn = None
self.lista_sbn = None
self.cnt_bn = None
self.rbn = None
self.lista_bb = None
self.lista_sbb = None
self.cnt_bb = None
self.rbb = None
self.rey_n = None
self.rey_b = None

self.reverted = 1 #1 OR -1
self.coords_ax = {
'1': 837,
'2': 766,
'3': 695,
'4': 624,
'5': 553,
'6': 482,
'7': 411,
'8': 340,
'9': 269
}
self.coords_ay = {
'i': 589,
'h': 518,
'g': 447,
'f': 376,
'e': 305,
'd': 234,
'c': 163,
'b': 92,
'a': 21
}
self.coords_bx = {
'1': 269,
'2': 340,
'3': 411,
'4': 482,
'5': 553,
'6': 624,
'7': 695,
'8': 766,
'9': 837
}
self.coords_by = {
'a': 589,
'b': 518,
'c': 447,
'd': 376,
'e': 305,
'f': 234,
'g': 163,
'h': 92,
'i': 21
}
self.coords_x = None
self.coords_y = None
self.update()
self.begin()

def begin(self):
self.lista_pn = {1:[self.coords_x['1'],self.coords_y['g']],2:[self.coords_x['2'],self.coords_y['g']],3:[self.coords_x['3'],self.coords_y['g']],4:[self.coords_x['4'],self.coords_y['g']],5:[self.coords_x['5'],self.coords_y['g']],6:[self.coords_x['6'],self.coords_y['g']],7:[self.coords_x['7'],self.coords_y['g']],8:[self.coords_x['8'],self.coords_y['g']],9:[self.coords_x['9'],self.coords_y['g']]}
self.lista_spn = {}
self.cnt_pn = 10
self.rpn = 0
self.lista_pb = {1:[self.coords_x['1'],self.coords_y['c']],2:[self.coords_x['2'],self.coords_y['c']],3:[self.coords_x['3'],self.coords_y['c']],4:[self.coords_x['4'],self.coords_y['c']],5:[self.coords_x['5'],self.coords_y['c']],6:[self.coords_x['6'],self.coords_y['c']],7:[self.coords_x['7'],self.coords_y['c']],8:[self.coords_x['8'],self.coords_y['c']],9:[self.coords_x['9'],self.coords_y['c']]}
self.lista_spb = {}
self.cnt_pb = 10
self.rpb = 0
self.lista_ln = {1:[self.coords_x['1'],self.coords_y['i']],2:[self.coords_x['9'],self.coords_y['i']]}
self.lista_sln = {}
self.cnt_ln = 3
self.rln = 0
self.lista_lb = {1:[self.coords_x['1'],self.coords_y['a']],2:[self.coords_x['9'],self.coords_y['a']]}
self.lista_slb = {}
self.cnt_lb = 3
self.rlb = 0
self.lista_nn = {1:[self.coords_x['2'],self.coords_y['i']],2:[self.coords_x['8'],self.coords_y['i']]}
self.lista_snn = {}
self.cnt_nn = 3
self.rnn = 0
self.lista_nb = {1:[self.coords_x['2'],self.coords_y['a']],2:[self.coords_x['8'],self.coords_y['a']]}
self.lista_snb = {}
self.cnt_nb = 3
self.rnb = 0
self.lista_sn = {1:[self.coords_x['3'],self.coords_y['i']],2:[self.coords_x['7'],self.coords_y['i']]}
self.lista_ssn = {}
self.cnt_sn = 3
self.rsn = 0
self.lista_sb = {1:[self.coords_x['3'],self.coords_y['a']],2:[self.coords_x['7'],self.coords_y['a']]}
self.lista_ssb = {}
self.cnt_sb = 3
self.rsb = 0
self.lista_gn = {1:[self.coords_x['4'],self.coords_y['i']],2:[self.coords_x['6'],self.coords_y['i']]}
self.cnt_gn = 3
self.rgn = 0
self.lista_gb = {1:[self.coords_x['4'],self.coords_y['a']],2:[self.coords_x['6'],self.coords_y['a']]}
self.cnt_gb = 3
self.rgb = 0
self.lista_tn = {1:[self.coords_x['2'],self.coords_y['h']]}
self.lista_stn = {}
self.cnt_tn = 2
self.rtn = 0
self.lista_tb = {1:[self.coords_x['8'],self.coords_y['b']]}
self.lista_stb = {}
self.cnt_tb = 2
self.rtb = 0
self.lista_bn = {1:[self.coords_x['8'],self.coords_y['h']]}
self.lista_sbn = {}
self.cnt_bn = 2
self.rbn = 0
self.lista_bb = {1:[self.coords_x['2'],self.coords_y['b']]}
self.lista_sbb = {}
self.cnt_bb = 2
self.rbb = 0
self.rey_n = [self.coords_x['5'],self.coords_y['i']]
self.rey_b = [self.coords_x['5'],self.coords_y['a']]

def update(self):
if self.reverted == 1:
self.coords_x = self.coords_ax
self.coords_y = self.coords_ay
else:
self.coords_x = self.coords_bx
self.coords_y = self.coords_by

def revert(self):
self.reverted *= -1

revert_x = {
269: 837,
340: 766,
411: 695,
482: 624,
553: 553,
624: 482,
695: 411,
766: 340,
837: 269
}
revert_y = {
21: 589,
92: 518,
163: 447,
234: 376,
305: 305,
376: 234,
447: 163,
518: 92,
589: 21
}
for k, e in self.lista_pn.items():
self.lista_pn[k] = [revert_x[e[0]],revert_y[e[1]]]
for k, e in self.lista_spn.items():
self.lista_spn[k] = [revert_x[e[0]],revert_y[e[1]]]
for k, e in self.lista_pb.items():
self.lista_pb[k] = [revert_x[e[0]],revert_y[e[1]]]
for k, e in self.lista_spb.items():
self.lista_spb[k] = [revert_x[e[0]],revert_y[e[1]]]
for k, e in self.lista_ln.items():
self.lista_ln[k] = [revert_x[e[0]],revert_y[e[1]]]
for k, e in self.lista_sln.items():
self.lista_sln[k] = [revert_x[e[0]],revert_y[e[1]]]
for k, e in self.lista_lb.items():
self.lista_lb[k] = [revert_x[e[0]],revert_y[e[1]]]
for k, e in self.lista_slb.items():
self.lista_slb[k] = [revert_x[e[0]],revert_y[e[1]]]
for k, e in self.lista_nn.items():
self.lista_nn[k] = [revert_x[e[0]],revert_y[e[1]]]
for k, e in self.lista_snn.items():
self.lista_snn[k] = [revert_x[e[0]],revert_y[e[1]]]
for k, e in self.lista_nb.items():
self.lista_nb[k] = [revert_x[e[0]],revert_y[e[1]]]
for k, e in self.lista_snb.items():
self.lista_snb[k] = [revert_x[e[0]],revert_y[e[1]]]
for k, e in self.lista_sn.items():
self.lista_sn[k] = [revert_x[e[0]],revert_y[e[1]]]
for k, e in self.lista_ssn.items():
self.lista_ssn[k] = [revert_x[e[0]],revert_y[e[1]]]
for k, e in self.lista_sb.items():
self.lista_sb[k] = [revert_x[e[0]],revert_y[e[1]]]
for k, e in self.lista_ssb.items():
self.lista_ssb[k] = [revert_x[e[0]],revert_y[e[1]]]
for k, e in self.lista_gn.items():
self.lista_gn[k] = [revert_x[e[0]],revert_y[e[1]]]
for k, e in self.lista_gb.items():
self.lista_gb[k] = [revert_x[e[0]],revert_y[e[1]]]
for k, e in self.lista_tn.items():
self.lista_tn[k] = [revert_x[e[0]],revert_y[e[1]]]
for k, e in self.lista_stn.items():
self.lista_stn[k] = [revert_x[e[0]],revert_y[e[1]]]
for k, e in self.lista_tb.items():
self.lista_tb[k] = [revert_x[e[0]],revert_y[e[1]]]
for k, e in self.lista_stb.items():
self.lista_stb[k] = [revert_x[e[0]],revert_y[e[1]]]
for k, e in self.lista_bn.items():
self.lista_bn[k] = [revert_x[e[0]],revert_y[e[1]]]
for k, e in self.lista_sbn.items():
self.lista_sbn[k] = [revert_x[e[0]],revert_y[e[1]]]
for k, e in self.lista_bb.items():
self.lista_bb[k] = [revert_x[e[0]],revert_y[e[1]]]
for k, e in self.lista_sbb.items():
self.lista_sbb[k] = [revert_x[e[0]],revert_y[e[1]]]
self.rey_n = [revert_x[self.rey_n[0]],revert_y[self.rey_n[1]]]
self.rey_b = [revert_x[self.rey_b[0]],revert_y[self.rey_b[1]]]

self.update()


The other one is matrix_manager - managers.py line 436:

class matrix_manager:
def __init__(self):
self.coords_hx = {
837:'1',
766:'2',
695:'3',
624:'4',
553:'5',
482:'6',
411:'7',
340:'8',
269:'9'
}
self.coords_hy = {
589:'i',
518:'h',
447:'g',
376:'f',
305:'e',
234:'d',
163:'c',
92:'b',
21:'a'
}
# X es el 1er índice
self.matrix = [
[1,1,1,1,1,1,1,1,1],
[0,1,0,0,0,0,0,1,0],
[1,1,1,1,1,1,1,1,1],
[0,0,0,0,0,0,0,0,0],
[0,0,0,0,0,0,0,0,0],
[0,0,0,0,0,0,0,0,0],
[1,1,1,1,1,1,1,1,1],
[0,1,0,0,0,0,0,1,0],
[1,1,1,1,1,1,1,1,1]
]
def empty(self, coords_h):

def fill(self, coords_h):

def get_hcoords(self, coords):
return str(self.coords_hx[coords[0]])+str(self.coords_hy[coords[1]])

def check_ln(self, h_begin, h_destiny):
cursory -= 1
if self.matrix[cursory][cursorx] == True:
return False
return True

def check_lb(self, h_begin, h_destiny):
cursory += 1
if self.matrix[cursory][cursorx] == True:
return False
return True

def check_t(self, h_begin, h_destiny):

if cursorx == destx:
mod = 1 if cursory < desty else -1
while cursory != desty - mod:
cursory += mod
if self.matrix[cursory][cursorx] == True:
return False
return True
else:
mod = 1 if cursorx < destx else -1
while cursorx != destx - mod:
cursorx += mod
if self.matrix[cursory][cursorx] == True:
return False
return True

def check_b(self, h_begin, h_destiny):

modx = 1 if cursorx < destx else -1
mody = 1 if cursory < desty else -1

while (cursorx != destx - modx) or (cursory != desty - mody):
cursorx += modx
cursory += mody
if self.matrix[cursory][cursorx] == True:
return False
return True


coords_manager handles dictionaries of coords of pieces that are present on the board. It also handles revertion of board. There is a list for each kind of piece (promoted pieces count as kind of piece for this) for each player. Doing it like this (instead of a global list for all pieces with a parameter to determine the kind of piece) allows me to make loops to iterate only over certain kind of piece. Another secondary function of coords_manager is to provide dictionaries to translate human-readable coords in numeric coords to blit the sprites. Every list of pieces gets the coords in numeric version.

Kinds of pieces are abbreviated this way:

s -> if present, it would mean promoted piece
p -> english abbreviature for the kind of piece - pawn in this case
n -> player's color (n if 'black' or 'sente', b if 'white' or 'gote')


When I noticed that I needed a way to prevent pieces jumping over other pieces, I created matrix_manager to resolve the problem. It basically handles a matrix of boolean values to keep in account which squares are filled by a piece.

The point I'm trying to make is that I want to make a Move class to clean up the code to prevent abuse of exec statements and regexp matching, as pointed in my previous question. But I'm convinced that, before doing such a thing, I should improve the managers first, as @200_success♦ suggested me in the previous question.

Which ways would you try to improve the board managers? Should I join those classes into a single one for a better modelling? Which deficiencies or redundancies am I not noticing in these managers?

I started this project at first just because I wanted a way to read my shogi notations. When the basic functionality worked, I decided to make the code public under GPL license for sharing, and with the hope that some entusiasts would join me to suggest improvements or even to work on the code if they felt willing to. So, you will find extensive general information in the README section in GitHub. Any little contribution is highly appreciated.

• Step 1: you see how you've got almost the same code over and over again? That's an immediate sanction from the Redundancy Department of Redundancy. You clearly know how to use lists and dictionaries, so use them to reduce duplication. DRY! Jul 12 '15 at 22:37
• Your comment made me laugh, but you are completely right. I've started inmediately to search about DRY principles on Google, it seems I don't know them well enough. Good point. Until you lectured me, I didn't know repetition could be avoided so far (assuming you are referencing the loops over dictionaries and similar patterns-> yeah, they are ridiculously redundant! :-\$) Maybe exec statements would be useful for that (or I would end up abusing on them again)... 'Use lists/dictionaries to avoid repetition'... I have to think deeply about that... Jul 12 '15 at 22:49
• Gotcha! The first bite to the elephant: It won't reduce the amount of lines, but it can help sparing CPU cycles; I can use lists or dictionaries for that, thus avoiding redundant calculations on coords_manager.begin(), maybe placing them in global constants (or better alternative, due to it's not good abusing on globals too). I could start from here... another day... gotta sleep... work tomorrow... ZzZz... Jul 12 '15 at 23:12

2. Rather than using an integer to determine when you can and can't revert, why not a boolean (True/False)? Then the test in update becomes if self.reverted, and in revert you can have self.reverted = not self.reverted;
3. The dictionary literals that never change (e.g. revert_x) don't need to be rebuilt every time you call a method - make them class attributes, and give them UPPERCASE_WITH_UNDERSCORES names to indicate that they're constants; and
4. Rather than the endless copies of the same thing over and over again, use data structures to abstract the issue (e.g. rather than self.lista_lb, why not use nested dictionaries to access self.list['a']['lb']?) - this will allow you to significantly reduce duplication by looping over the structures.
• @jonsharpe About point 2: The good reason I used an Integer instead of a Boolean for revertion, is because I use this revertion value (1 or -1) as a multiplier for coords_sum (so it influences the piece's movement depending on revertion). This is done in function move_forward() -> see here in line 96 github.com/SebasSBM/shogi_reader/blob/… Oct 25 '15 at 19:31