2016-12-28 68 views
4

我目前正在嘗試重構我的代碼後,我讀到的實現優先於擴展。目前,我正在嘗試創建一個將對象添加到場景的功能。爲了更好地確定每個對象是什麼,有多種解釋,如用於更新,渲染列表等Java「instanceof」添加多個列表

private List<Updatable> updatables; 
private List<Removable> removables; 
private List<Renderable> renderables; 
private List<Collidable> collidables; 

我想打一個函數到我的Scene類,像這樣:

public void add(Object o) { 
    if(o instanceof Updatable) 
     updatables.add((Updatable) o); 
    if(o instanceof Removable) 
     removables.add((Removable) o); 
    if(o instanceof Renderable) 
     renderables.add((Renderable) o); 
    if(o instanceof Collidable) 
     collidables.add((Collidable) o); 
} 

同樣,我會創建一個類似的刪除功能。我的問題是,如果這是不好的編碼習慣,因爲我聽說過instanceof的缺陷,其次,如果是這樣,是否有辦法繞過/重構我的代碼以使添加實例變得簡單?我更喜歡不必爲每個列表添加一個實例,並且每次都定義它屬於哪個實例。

+2

很多類似的問題。 [例如](https://www.google.com/webhp?sourceid=chrome-instant&ion=1&espv=2&ie=UTF-8#q=site:stackoverflow.com+java+instanceof+code+smell) –

回答

1

我會用Map

static Map<String, List> maps = new HashMap<>(); 
static { 
    maps.put(Updatable.class.getName(), new ArrayList<Updatable>()); 
    maps.put(Removable.class.getName(), new ArrayList<Removable>()); 
    maps.put(Renderable.class.getName(), new ArrayList<Renderable>()); 
    maps.put(Collidable.class.getName(), new ArrayList<Collidable>()); 
} 
public static void add(Object obj){ 
    maps.get(obj.getClass().getName()).add(obj); 
} 
+0

是可能使用class'Class'而不是class'String',然後執行:maps.put(Updatable.class,new ArrayList ());?我想我會採用這種方式。 –

+1

@MichaelChoi你不應該接受一個答案,甚至沒有嘗試過。這會拋出NPE,因爲你有這些接口的實現類,所以'maps.get()'將返回null。它也不能解決一個類實現多個接口的問題,根據你對我的答案的評論。 – EJP

+0

@EJP,OP沒有提及'Updateable'是類或接口,這個代碼將根據用例進行更新,並且在使用'maps.get'時需要在一些負面情況下做更多的事情。這只是一個想法,建議避免'if'' else'來檢查''的實例' – Jerry06

2

做工廠模式類似只需使用重載:add(Updateable u), add(Removable r),

+1

這似乎是更自然的Java方法。 – Sudicode

+0

啊,這很有趣。我不知道這會調用到每個接口。這意味着如果我有一個同時實現可更新和可移動的對象,那麼這兩個函數將被調用?如果這是首先被調用的情況。 –

+0

更新:這種方式不適用於我不幸的。當一個對象同時是Updatable和Removable時,然後嘗試添加該對象將導致「模糊的方法調用」。這可以通過投射解決,所以我會做scene.add((Updatable)obj); scene.add((Removable)obj);但這似乎不是一個理想的解決方案。 –

1

當您在評論中提到,你的使用情況有點特殊。 Updatable,Removable,RenderableCollidable都是接口。你有類實現一個或多個這些接口。最終,你想要你的add方法添加一個對象到它實現的接口的每個列表。 (糾正我,如果任何這是錯誤的。)

如果超載會過於冗長,您可以用反射做到這一點,就像這樣:

private List<Updatable> updatables; 
private List<Removable> removables; 
private List<Renderable> renderables; 
private List<Collidable> collidables; 

@SuppressWarnings("unchecked") 
public void add(Object o) { 
    try { 
     for (Class c : o.getClass().getInterfaces()) { 
      // Changes "Updatable" to "updatables", "Removable" to "removables", etc. 
      String listName = c.getSimpleName().toLowerCase() + "s"; 

      // Adds o to the list named by listName. 
      ((List) getClass().getDeclaredField(listName).get(this)).add(o); 
     } 
    } catch (IllegalAccessException e) { 
     // TODO Handle it 
    } catch (NoSuchFieldException e) { 
     // TODO Handle it 
    } 
} 

但是,如果你要使用這種方法,請注意以下注意事項:

  1. 反射本質上很容易出現運行時錯誤。
  2. 如果層次結構更復雜,則可能會遇到問題。例如,如果您有Foo implements Updatable, RemovableBar extends Foo,則在Bar上調用getClass().getInterfaces()將返回一個空數組。如果這聽起來像您的使用案例,您可能需要使用Apache Commons Lang中的ClassUtils.getAllInterfaces
+0

,這是因爲酒吧被認爲沒有實施任何東西,即使Foo是?還是隻是「.getInterfaces()」函數僅返回子類的接口? –

+1

這是因爲'getInterfaces()'只會選擇由implements直接引用的接口。它不會像遍歷Apache的'ClassUtils.getAllInterfaces(Class)'那樣遍歷類層次結構。然而,如果你冗餘地聲明Bar類爲'Bar extends Foo implements Updatable,Removable',那麼'getInterfaces()'會選擇這些。 – Sudicode

0

我會想到的是,你有一個單獨的add方法爲每個郵件列表,像這樣:

public void addUpdatable(Updatable updatable) { 
    updatables.add(updatable); 
} 

public void addRemovable(Removable removable) { 
    removables.add(removable); 
} 

// and so on 

這樣做的原因是,這是單獨的列表,並同時從關注實施方面的觀點是爲了使API更加優雅和光滑,這些努力會從用戶的角度來影響清晰度。

例如,添加Updatable(也就是Removable)時發生的誤解(在EJP的回答評論中)表明缺乏清晰度。儘管您對類型和狀態都使用了類似的術語,這使得它看起來多餘,但您仍應該清楚該項目添加到哪個列表(或州的一部分)。

無論如何,它看起來像你在更大範圍內的問題掙扎。我認爲,投入更多時間來理解更大的問題是個好主意。

+0

你能幫我理解這個問題嗎?那麼這是目前設計的問題嗎?我正在嘗試製作一個遊戲。對於每個遊戲循環,我只希望某些對象執行某些功能,即更新,渲染,刪除。這是一個不好的解決方案?如果是的話,你有另一種選擇。看起來你似乎認爲每個列表都應該是分開的,但是像Player這樣的類將如何可更新,可渲染和可移除? –

0

從@ Jerry06,爲我添加一個實例到多個不同類型列表的最好方法是使用一個魔術列表getter。

private Map<Class, List> maps = new HashMap<>(); 

'maps'用於查找您要查找的列表。

private <T> List<T> get(Class<T> c) { 
    return maps.get(c); 
} 

而不是使用maps.get,我只是使用get(SomeClass.class)。上面的函數會自動將其轉換爲具有正確元素類型的列表。然後,我能夠做的:的

get(Updatable.class).stream().filter(Updatable::isAwake).forEach(Updatable::update); 

代替:

updatables.stream().filter(Updatable::isAwake).forEach(Updatable::update); 

我主要是想這樣的結構,使所有的名單可以很好地包裹起來,其次所以我可以說地圖。添加(SomeNewClass.class,新的ArrayList()),然後隨時調用它。