| Conditions | 38 | 
| Total Lines | 147 | 
| Code Lines | 122 | 
| Lines | 0 | 
| Ratio | 0 % | 
| Changes | 0 | ||
Small methods make your code easier to understand, in particular if combined with a good name. Besides, if your method is small, finding a good name is usually much easier.
For example, if you find yourself adding comments to a method's body, this is usually a good sign to extract the commented part to a new method, and use the comment as a starting point when coming up with a good name for this new method.
Commonly applied refactorings include:
If many parameters/temporary variables are present:
Complex classes like osci.dialog.PlanetsOverviewDlg.PlanetsOverviewDlg.show() often do a lot of different things. To break such a class down, we need to identify a cohesive component within that class. A common approach to find such a component is to look for fields/methods that share the same prefixes, or suffixes.
Once you have determined the fields that belong together, you can apply the Extract Class refactoring. If the component makes sense as a sub-class, Extract Subclass is also a candidate, and is often faster.
| 1 | # | ||
| 56 | def show(self): | ||
| 57 | player = client.getPlayer() | ||
| 58 | # | ||
| 59 | items = [] | ||
| 60 | for planetID in client.db.keys(): | ||
| 61 | planet = client.get(planetID, noUpdate = 1) | ||
| 62 | # skip non-planets | ||
| 63 | if not hasattr(planet, "type") or planet.type != Const.T_PLANET: | ||
| 64 | continue | ||
| 65 | # shall be shown? | ||
| 66 | ok = 0 | ||
| 67 | if hasattr(planet, 'owner'): | ||
| 68 | if self.showMine and planet.owner == player.oid: | ||
| 69 | ok = 1 | ||
| 70 | if self.showOtherPlayers and planet.owner != Const.OID_NONE and \ | ||
| 71 | planet.owner != player.oid: | ||
| 72 | ok = 1 | ||
| 73 | if self.showColonizable and planet.owner == Const.OID_NONE and \ | ||
| 74 |                     planet.plType not in ('G', 'A'): | ||
| 75 | ok = 1 | ||
| 76 |                 if self.showUncolonizable and planet.plType in ('G', 'A'): | ||
| 77 | ok = 1 | ||
| 78 | elif hasattr(planet, 'plType'): | ||
| 79 |                 if self.showColonizable and planet.plType not in ('G', 'A'): | ||
| 80 | ok = 1 | ||
| 81 |                 if self.showUncolonizable and planet.plType in ('G', 'A'): | ||
| 82 | ok = 1 | ||
| 83 | if not ok: | ||
| 84 | continue | ||
| 85 | # fill in data | ||
| 86 | #rel = Const.REL_UNDEF | ||
| 87 | maxNA = 999999 | ||
| 88 | maxNone = 99999 | ||
| 89 | ownerID = Const.OID_NONE | ||
| 90 | if hasattr(planet, 'owner'): | ||
| 91 | ownerID = planet.owner | ||
| 92 | #if planet.owner != Const.OID_NONE: | ||
| 93 | # rel = client.getRelationTo(planet.owner) | ||
| 94 | if planet.owner == Const.OID_NONE: | ||
| 95 | #else: | ||
| 96 |                     owner = _('[Nobody]') | ||
| 97 | if hasattr(planet, 'owner') and planet.owner == player.oid: | ||
| 98 | if planet.prodQueue and planet.effProdProd > 0: | ||
| 99 | index = 0 | ||
| 100 | totalEtc = 0 | ||
| 101 | for task in planet.prodQueue: | ||
| 102 | if task.isShip: | ||
| 103 | tech = client.getPlayer().shipDesigns[task.techID] | ||
| 104 | else: | ||
| 105 | tech = client.getFullTechInfo(task.techID) | ||
| 106 | if index == 0: | ||
| 107 | constrInfo = tech.name | ||
| 108 | # etc | ||
| 109 | if task.targetID != planetID: | ||
| 110 | if index == 0: | ||
| 111 | etc = math.ceil(float(tech.buildProd * Rules.buildOnAnotherPlanetMod - task.currProd) / planet.effProdProd) | ||
| 112 | totalEtc += etc | ||
| 113 | totalEtc += math.ceil((task.quantity - 1) * float(tech.buildProd * Rules.buildOnAnotherPlanetMod) / planet.effProdProd) | ||
| 114 | else: | ||
| 115 | totalEtc += math.ceil(float(tech.buildProd * Rules.buildOnAnotherPlanetMod - task.currProd) / planet.effProdProd) | ||
| 116 | totalEtc += math.ceil((task.quantity - 1) * float(tech.buildProd * Rules.buildOnAnotherPlanetMod) / planet.effProdProd) | ||
| 117 | else: | ||
| 118 | if index == 0: | ||
| 119 | etc = math.ceil(float(tech.buildProd - task.currProd) / planet.effProdProd) | ||
| 120 | totalEtc += etc | ||
| 121 | totalEtc += math.ceil((task.quantity - 1)* float(tech.buildProd) / planet.effProdProd) | ||
| 122 | else: | ||
| 123 | totalEtc += math.ceil(task.quantity * float(tech.buildProd - task.currProd) / planet.effProdProd) | ||
| 124 | totalEtc += math.ceil((task.quantity - 1) * float(tech.buildProd) / planet.effProdProd) | ||
| 125 | index += 1 | ||
| 126 | etc_raw = etc | ||
|  | |||
| 127 | etc = res.formatTime(etc) | ||
| 128 | totalEtc_raw = totalEtc | ||
| 129 | totalEtc = res.formatTime(totalEtc) | ||
| 130 | elif planet.prodQueue: | ||
| 131 | task = planet.prodQueue[0] | ||
| 132 | if task.isShip: | ||
| 133 | tech = client.getPlayer().shipDesigns[task.techID] | ||
| 134 | else: | ||
| 135 | tech = client.getTechInfo(task.techID) | ||
| 136 | constrInfo = tech.name | ||
| 137 |                     etc = _('N/A') | ||
| 138 | etc_raw = maxNA | ||
| 139 |                     totalEtc = _("N/A") | ||
| 140 | totalEtc_raw = maxNA | ||
| 141 | elif planet.effProdProd > 0: | ||
| 142 |                     constrInfo = _("-") | ||
| 143 | etc = "-" | ||
| 144 | etc_raw = 0 | ||
| 145 |                     totalEtc = _("-") | ||
| 146 | totalEtc_raw = 0 | ||
| 147 | else: | ||
| 148 |                     constrInfo = _("-") | ||
| 149 | etc = "-" | ||
| 150 | etc_raw = maxNone | ||
| 151 |                     totalEtc = _("-") | ||
| 152 | totalEtc_raw = maxNone | ||
| 153 | else: | ||
| 154 | constrInfo = '?' | ||
| 155 | etc = '?' | ||
| 156 | etc_raw = maxNA | ||
| 157 | totalEtc = '?' | ||
| 158 | totalEtc_raw = maxNA | ||
| 159 | # used slots | ||
| 160 | if hasattr(planet, 'slots'): | ||
| 161 | freeSlots = planet.plSlots - len(planet.slots) | ||
| 162 | else: | ||
| 163 | freeSlots = '?' | ||
| 164 | # morale | ||
| 165 | if hasattr(planet, "morale"): | ||
| 166 | morale = int(planet.morale) | ||
| 167 | else: | ||
| 168 | morale = "?" | ||
| 169 | # | ||
| 170 | plType = gdata.planetTypes[getattr(planet, 'plType', None)] | ||
| 171 | # list item | ||
| 172 | item = ui.Item( | ||
| 173 | getattr(planet, 'name', res.getUnknownName()), | ||
| 174 | tPlType = _(plType), | ||
| 175 | tPlBio = getattr(planet, 'plBio', '?'), | ||
| 176 | tPlMin = getattr(planet, 'plMin', '?'), | ||
| 177 | tPlEn = getattr(planet, 'plEn', '?'), | ||
| 178 | tChangeBio = getattr(planet, 'changeBio', '?'), | ||
| 179 | tChangeEn = getattr(planet, 'changeEn', '?'), | ||
| 180 | tETC = etc, | ||
| 181 | tETC_raw = etc_raw, | ||
| 182 | tTotalETC = totalEtc, | ||
| 183 | tTotalETC_raw = totalEtc_raw, | ||
| 184 | tConstrInfo = constrInfo, | ||
| 185 | tFree = freeSlots, | ||
| 186 | tMorale = morale, | ||
| 187 | tSpace = getattr(planet, 'plSlots', '?'), | ||
| 188 | tDiam = getattr(planet, 'plDiameter',0)/1000, | ||
| 189 | tProd = getattr(planet, 'effProdProd', '?'), | ||
| 190 | tSci = getattr(planet, 'effProdSci', '?'), | ||
| 191 | tPlanetID = planetID, | ||
| 192 | #foreground = res.getFFColorCode(rel), | ||
| 193 | foreground = res.getPlayerColor(ownerID), | ||
| 194 | ) | ||
| 195 | items.append(item) | ||
| 196 | self.win.vPlanets.items = items | ||
| 197 | self.win.vPlanets.itemsChanged() | ||
| 198 | # buttons | ||
| 199 | self.win.vMine.pressed = self.showMine | ||
| 200 | self.win.vOtherPlayers = self.showOtherPlayers | ||
| 201 | self.win.vColonizable = self.showColonizable | ||
| 202 | self.win.vUncolonizable = self.showUncolonizable | ||
| 203 | |||
| 280 |