我编写了代码,它可以工作,但它使用了许多if条件,而且看起来很难看。如果有人能帮助我,让我朝着更面向对象的方向发展,我会非常感激的。
package ar.edu.uca.ceis.objetos.imperio;
import ar.uba.fi.algo3.batallaespacial.CabinaDeControl;
import ar.uba.fi.algo3.batallaespacial.Civilizacion;
import ar.uba.fi.algo3.batallaespacial.Piloto;
import ar.uba.fi.algo3.batallaespacial.Reporte.Espectro;
import ar.uba.fi.algo3.batallaespacial.Sustancia;
import ar.uba.fi.algo3.batallaespacial.comandos.Comando;
import ar.uba.fi.algo3.batallaespacial.Direccion;
public class PilotoImperial implements Piloto {
private CabinaDeControl cabina;
private Imperio civilizacion;
public PilotoImperial(Imperio civilizacion) {
super();
this.civilizacion = civilizacion;
}
public void setCabinaDeControl(CabinaDeControl cabina) {
this.cabina = cabina;
}
public Comando proximoComando() {
Direccion[] values = Direccion.values();
// for (int i = 1; i < values.length; i++) {
int i;
i = (int)Math.round(Math.random() *values.length-1) ;
//if border of an unknow position, go to a random place
if (Espectro.DESCONOCIDO == this.cabina.getRadar().getReporte( values[i] ).getEspectro()){
// generaremos numeros aleatorios entre 1 y values.length
int c = (int)Math.round(Math.random() *values.length) ;
if (Espectro.VACIO == this.cabina.getRadar().getReporte( values[c] ).getEspectro()){
return this.cabina.getControl().avanzar( values[c] );
}
}
// i found and enemy base, attack
if (Espectro.BASE == this.cabina.getRadar().getReporte( values[i] ).getEspectro()) {
if (this.cabina.getRadar().getReporte( values[i] ).getCivilizacion()!=this.civilizacion){
return this.cabina.getControl().atacar(values[i]);
}
}
// if found and enemy, attack
if (Espectro.NAVE == this.cabina.getRadar().getReporte( values[i] ).getEspectro()) {
if (this.cabina.getRadar().getReporte( values[i] ).getCivilizacion()!=this.civilizacion){
return this.cabina.getControl().atacar(values[i]);
}
}
// attack asteroid if (Espectro.ASTEROIDE == this.cabina.getRadar().getReporte( values[i] ).getEspectro()) {
return this.cabina.getControl().atacar(values[i]);
}
// if found a container, upload materia 100
if (Espectro.CONTENEDOR == this.cabina.getRadar().getReporte( values[i] ).getEspectro()) {
while (this.cabina.getRadar().getReporte( values[i] ).getCantidadDeSustancia(Sustancia.ANTIMATERIA)>0){
return this.cabina.getControl().transferirCarga(values[i], Direccion.ORIGEN, Sustancia.ANTIMATERIA, 100);
}
}
// if found a ally base, download anti-materia
if (Espectro.BASE == this.cabina.getRadar().getReporte( values[i] ).getEspectro()) {
while (this.cabina.getMonitor().getCarga(Sustancia.ANTIMATERIA)>0){
return this.cabina.getControl().transferirCarga(Direccion.ORIGEN,values[i], Sustancia.ANTIMATERIA, 100);
}
}
// if there is nothing, move to a position
int ii = (int)Math.round(Math.random() *values.length-1) ;
if (Espectro.VACIO == this.cabina.getRadar().getReporte( values[ii] ).getEspectro()){
return this.cabina.getControl().avanzar( values[ii] );
}
//}
// found none wait
return this.cabina.getControl().esperar();//.avanzar();
}
public Civilizacion getCivilizacion() {
return this.civilizacion;
}
public String getNombre() {
return "Piloto Imperial";
}
}考虑到这是一个框架,我只向您展示我正在更改的类,其余的都可以正常工作。这是一个家庭作业,所以不需要一个超级花哨的代码。最后但并非最不重要的是,如果你能帮助我使用一个模式,那将是惊人的。
发布于 2013-06-11 16:20:21
您的教授分配给您的代码不符合一些面向对象编程的基本原则,包括:
实际上,您的教授正在教您如何使用结构(而不是对象)使用Java进行编程。
例如,这里有一个违反德米特定律的问题:
if (Espectro.VACIO == this.cabina.getRadar().getReporte( values[ii] ).getEspectro()){如果以下任何一个是null,代码将抛出一个NullPointerException:
this.cabinagetRadar()getReporte( ... )没有try/catch块,该方法也不声明任何异常,因此程序将崩溃。一个崩溃的程序可能会让用户非常沮丧。
这一行应包括以下任何一项:
if( getCabina().isEspectro( Espectro.VACIO ) ) {
if( isEspectro( Espectro.VACIO ) ) {在第二种情况下,方法isEspectro类似于:
public boolean isEspectro( Espectro e ) {
return getCabina().isEspectro( e );
}这就是授权。它避免了层叠的点表示法,因为它是这么多错误的来源。有关信息隐藏的详细信息,请参阅这个答案。
getCabina()方法将完全负责确保设置CabinaDeControl实例。否则,如何保证this.cabina不是null (不使用支持倒置控制的框架,如Spring )?例如:
private synchronized CabinaDeControl getCabina() {
if( this.cabina == null ) {
this.cabinia = createCabina();
}
return this.cabina;
}
/** Subclasses can vary the CabinaDeControl instances used by this class. */
protected CabinaDeControl createCabina() {
return new CabinaDeControl(); // ... or whatever is required to instantiate.
}这被称为延迟初始化。重要的是,这种模式遵循开放关闭原则,您可以通过在子类中重写createCabina()方法来改变类的行为。您不必改变原来的类来改变它的行为。这防止了引入范围广泛的bug(新的子类仍然可以引入bug,但波纹效应应该不会像更改原始类那样严重)。
使用this.cabina.method()违反了干原理,因为this.cabina被重复了好几次。编程意味着消除重复的代码,原因很多。
每次出现this.cabina.method()都可以抛出一个NullPointerException,因为没有检查以确保cabina不是null。这会让你有一种不安的感觉。将this.cabina替换为getCabina() (正如我前面所实现的),这种不安的感觉应该会消失。这并不意味着代码将是正确的,但至少如果cabina被设置为null,程序就不会崩溃。
发布于 2013-06-12 11:29:35
Dave给了您一些关于如何增加代码的风格和稳定性的提示之后,我想我可以给您一些关于您的设计的建议。
在您的方法proximoComando()中,您一遍又一遍地做同样的事情,实际上是为了重构而尖叫:
检查开始时选择的Direccion,如果检查成功,则返回一个操作。策略和责任链的设计模式--在我的脑海中立即想到的--清理这一切。
开始吧..。
首先,我们用战略模式抽象出对战舰周围环境的检查。
public interface SituationCheck {
// check the situation and return true if the concrete conditions are met
boolean isConditionMet(CabinaDeControl cabina, Direccion contactDireccion, Civilizacion ownCivilizacion);
}..。飞行员可以执行的动作也是如此..。
public interface PilotAction {
// return the command that corresponds to the given action
Comando issueCommand(Direccion[] direccions, Direccion contactDireccion, Control control);
}现在,我们可以使用这些接口来模拟战舰遵循责任链模式的行为。我将类命名为Orders,因为它代表了飞行员在特定情况下必须做的一组命令:
public class Orders {
private SituationCheck check = null;
private PilotAction action = null;
private Orders next = null;
public Orders(SituationCheck check, PilotAction action) {
this.check = check;
this.action = action;
this.next = null;
}
public Orders(PilotAction action) {
this(null, action);
}
/**
* Performs situation checks and issues the appropriate command
*
* @param contactDireccion
* the direction that is checked
* @param cabina
* the radar report of a given direccion
* @param ownCivilizacion
* the ship's own civilizacion
* @return the command the fits to the situation
*/
public Comando checkSituationAndIssueCommand(Direccion[] direccions, Direccion contactDireccion, CabinaDeControl cabina, Civilizacion ownCivilizacion) {
if (check == null || check.isConditionMet(cabina, contactDireccion, ownCivilizacion)) {
if (action != null) { // adding null actions should be avoided ;)
return action.issueCommand(direccions, contactDireccion, cabina.getControl());
}
} else if (next != null) {
return next.checkSituationAndIssueCommand(direccions, contactDireccion, cabina, ownCivilizacion);
}
// if we reach the end of the chain and no situation check succeeded
// or if there was no action assigned to the situation
// this may be replaced by a dummy action e.g. DoNothing
return null;
}
public Orders add(Orders orders) {
if (next == null) {
next = orders;
return next;
} else {
return next.add(orders);
}
}
public Orders add(SituationCheck newCheck, PilotAction newAction) {
return add(new Orders(newCheck, newAction));
}
public void add(PilotAction finalAction) {
add(new Orders(finalAction));
}
}我会一步一步地解释:
二是创建你的例子的行为我们需要一些具体的检查..。
public class CheckForEspectro implements SituationCheck {
private Espectro espectro;
public CheckForEspectro(Espectro espectro) {
this.espectro = espectro;
}
@Override
public boolean isConditionMet(CabinaDeControl cabina, Direccion contactDireccion, Civilizacion ownCivilizacion) {
return cabina.isEspectro(espectro, contactDireccion);
}
}
public class CheckForEnemy implements SituationCheck {
private Espectro enemyType; // e.g. BASE, NAVE
public CheckForEnemy(Espectro enemyType) {
this.enemyType = enemyType;
}
@Override
public boolean isConditionMet(CabinaDeControl cabina, Direccion contactDireccion, Civilizacion ownCivilizacion) {
return cabina.isEspectro(enemyType, contactDireccion) && cabina.contactIsHostileTo(contactDireccion, ownCivilizacion);
}
}
public class CheckForAlly implements SituationCheck {
private Espectro allyType; // e.g. BASE, NAVE
public CheckForAlly(Espectro allyType ) {
this.allyType = allyType ;
}
@Override
public boolean isConditionMet(CabinaDeControl cabina, Direccion contactDireccion, Civilizacion ownCivilizacion) {
return cabina.isEspectro(allyType, contactDireccion) && cabina.contactIsAlliedTo(contactDireccion, ownCivilizacion);
}
}
public class CheckForContainer implements SituationCheck {
private Sustancia sustancia;
public CheckForContainer(Sustancia sustancia) {
this.sustancia = sustancia;
}
@Override
public boolean isConditionMet(CabinaDeControl cabina, Direccion contactDireccion, Civilizacion ownCivilizacion) {
return cabina.isEspectro(Espectro.CONTAINER, contactDireccion) && cabina.contactContainsSubstance(contactDireccion, sustancia);
}
}..。和行动..。
public class Attack implements PilotAction {
@Override
public Comando issueCommand(Direccion[] direccions, Direccion contactDireccion, Control control) {
return control.atacar(contactDireccion);
}
}
public class MoveToRandomPosition implements PilotAction {
@Override
public Comando issueCommand(Direccion[] direccions, Direccion contactDireccion, Control control) {
int index = (int) Math.round(Math.random() * direccions.length);
return control.avanzar(direccions[index]);
}
}
public class Wait implements PilotAction {
@Override
public Comando issueCommand(Direccion[] direccions, Direccion contactDireccion, Control control) {
return control.esperar();
}
}
public class Transfer implements PilotAction {
public enum Direction {
UPLOAD, DOWNLOAD
}
private Direction direction;
private Sustancia sustancia;
private int amount;
public Transfer(Direction direction, Sustancia sustancia, int amount) {
this.direction = direction;
this.sustancia = sustancia;
this.amount = amount;
}
@Override
public Comando issueCommand(Direccion[] direccions, Direccion contactDireccion, Control control) {
switch (direction) {
case UPLOAD:
return control.transferirCarga(contactDireccion, Direccion.ORIGEN, sustancia, amount);
case DOWNLOAD:
return control.transferirCarga(Direccion.ORIGEN, contactDireccion, sustancia, amount);
default: // currently unreachable
return null;
}
}
}请注意,我假设您已经实现了Dave的一些想法,例如,我调用了一个方法
cabina.contactIsHostileTo(contactDireccion, ownCivilizacion)我假设返回您的检查结果
this.cabina.getRadar().getReporte( values[i] ).getCivilizacion()!=this.civilizacion其中contactDireccion协同响应values[i],ownCivilizacion响应this.civilizacion。
这样你就可以在一个地方有这样的逻辑,并且可以很容易地将简单的检查转换成更复杂的检查,例如通过评估一些外交矩阵。
现在,我们可以通过为订单添加一个字段来更改您的类。
private Orders orders;并在构造函数中设置它们:
public PiloPilotoImperial() {
super();
Orders orders = new Orders(new CheckForEspectro(Espectro.DESCONOCIDO), new MoveToRandomPosition());
orders.add(new CheckForEnemy(Espectro.NAVE), new Attack())
.add(new CheckForEnemy(Espectro.BASE), new Attack())
.add(new CheckForContainer(Sustancia.ANTIMATERIA), new Transfer(Direction.UPLOAD, Sustancia.ANTIMATERIA, 100))
.add(new CheckForAlly(Espectro.BASE), new Transfer(Direction.DOWNLOAD, Sustancia.ANTIMATERIA, 100))
.add(new CheckForEspectro(Espectro.VACACIO), new MoveToRandomPosition())
.add(new Wait());
}
public PilotoImperial(Imperio civilizacion) {
this();
this.civilizacion = civilizacion;
}最后,您的方法将更短:
public Comando proximoComando() {
Direccion[] direccions = Direccion.values();
if (direccions == null || direccions.length == 0) {
return null;
}
Direccion contactDireccion = direccions[Math.round(Math.random() * values.length-1)];
return orders.checkSituationAndIssueCommand(direccions, contactDireccion, getCabina(), getCivilizacion());
}我希望这个答案不会太长,但我想向您展示如何通过将代码分割成简单易懂的小部分来重写代码,这些代码可以组合在一起,在许多不同的变体中形成一个更复杂的结构。
例如,您可以使用不同的检查和行动组合,以创建一个货轮,以转移其他东西和逃离敌人。
您可以找到我的源代码这里 -我为您的类添加了一些虚拟实现以避免编译器错误。随意改变你想要的任何东西。
https://codereview.stackexchange.com/questions/27268
复制相似问题