HiveBrain v1.2.0
Get Started
← Back to all entries
patternpythonMinor

Class modelling for a shogi notation reader

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
shoginotationforclassmodellingreader

Problem

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.list

Solution

A few pointers:

  • Python has a style guide, which you should strongly consider following (some docstrings would be nice, too);



  • 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;



  • 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



  • 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.

Context

StackExchange Code Review Q#96707, answer score: 2

Revisions (0)

No revisions yet.