2010-02-17 11 views
5

के ओवरराइडिंग या अधिभार को एमएस साक्षात्कार में मुझसे यह प्रश्न पूछा गया था। मैं कोड के इस टुकड़े में सटीक डिजाइन समस्या जानना चाहता हूं। कोड को पहले ही दिया गया था, डिजाइन समस्या को खोजने के लिए आवश्यक था।इस डिजाइन के साथ क्या गलत है? Java.util.HashMap

मेरे पास क्लास MyHashMap है जो जावा हैश मैप क्लास को बढ़ाता है। MyHashMap कक्षा में मुझे कर्मचारियों की कुछ जानकारी रखना है। इस मानचित्र में कुंजी पहले नाम + अंतिम नाम + पता होगा।

public MyHashMap extends HashMap<Object, Object> { 
    //some member variables 
    // 
    public void put(String firstName, String lastName, String Address, Object obj) { 
     String key = firstName + lastName+ Address; 
     put(key, obj); 
    } 

    public Object get(String firstName, String lastName, String Address) { 
     String key = firstName + lastName+ Address; 
     return get(key); 
    } 

    public void remove(Strig key) { 
     put(key, ""); 
    } 

    //some more methods 
} 

इस डिज़ाइन में क्या गलत है? क्या मुझे हैश मैप को उप-वर्ग करना चाहिए या क्या मुझे इस कक्षा के सदस्य चर के रूप में हैश मैप घोषित करना चाहिए? या क्या मैंने हैशकोड/बराबर विधियों को लागू किया है?

+0

मानचित्र इंटरफ़ेस की आवश्यकता है, या यह है कि आपका प्रश्न उठाए ? – extraneon

+0

क्या यह कहा गया था कि कुंजी firstName + lastName + पता का एक संयोजन होना चाहिए? मेरे लिए, इन गुणों को जोड़कर एक अतिरिक्त पीओजेओ और बराबर() और हैशकोड() लागू करना एक बेहतर समाधान लगता है। – ddso

+0

कोड पहले से ही दिया गया था मुझे डिजाइन के साथ मुद्दों का पता लगाना है, अगर कोई – Ashish

उत्तर

13

कुछ समस्याएं हैं, लेकिन सबसे बड़ी समस्या यह है कि मैं इसे देख सकता हूं कि आप एक कुंजी के रूप में एक संगत String का उपयोग कर रहे हैं। निम्नलिखित दो कॉल अलग हैं, लेकिन बराबर:

final MyHashMap map = new MyHashMap(); 

map.put("foo", "", "baz", new Object()); 
map.put("", "foo", "baz", new Object()); // Overwrites the previous call 

वहाँ भी एक मुद्दा आप घोषणा की कि कर रहे हैं कि कुंजी प्रकार एक Object के रूप में है, लेकिन हमेशा String का उपयोग कर और इसलिए प्रकार सुरक्षा की बात आती है कि का लाभ नहीं ले रहे है जेनेरिक के साथ। उदाहरण के लिए, यदि आप अपने Map के keySet के माध्यम से लूप करना चाहते हैं, तो आपको प्रत्येक Set प्रविष्टि को String पर डालना होगा, लेकिन आप यह सुनिश्चित नहीं कर पाएंगे कि कुंजी का उपयोग करके किसी ने आपको Map का दुरुपयोग नहीं किया है उदाहरण।

व्यक्तिगत रूप से, मैं विरासत पर अनुपालन की अनुग्रह करता हूं जब तक कि आपके पास कोई अच्छा कारण न हो। आपके मामले में, MyHashMapअधिक भारput, get और remove, लेकिन के मानक Map तरीकों उनमें से किसी को अधिभावी नहीं है। अपने व्यवहार को बदलने के लिए आपको कक्षा से उत्तराधिकारी होना चाहिए, लेकिन आपका कार्यान्वयन ऐसा नहीं करता है, इसलिए संरचना एक स्पष्ट पसंद है।

एक उदाहरण के रूप में कार्य करने के लिए, के बजाय अधिक भार का मतलब है कि यदि आप निम्नलिखित घोषणा कर अधिभावी:

Map<Object, Object> map = new MyHashMap(); 

कोई भी अपने घोषित तरीकों में से उपलब्ध हो जाएगा। जैसा कि कुछ अन्य उत्तरों द्वारा अनुशंसित किया गया है, यह आपके नाम कुंजी के रूप में कार्य करने के लिए प्रथम नाम, अंतिम नाम और पता से बना वस्तु का उपयोग करना बेहतर होगा, लेकिन आपको equals और hashCode को लागू करना याद रखना चाहिए, अन्यथा आपके मान पुनर्प्राप्त नहीं किए जाएंगे HashMap

4

मैं आपकी कक्षा के सदस्य चर के रूप में हैश मैप घोषित करने के लिए जाऊंगा।

मुझे याद नहीं है कि स्वच्छ कोड या प्रभावी जावा में एक महान स्पष्टीकरण दिया गया है, लेकिन मूल रूप से, यह करना आसान है, यदि एपीआई बदलता है तो कम काम की आवश्यकता होती है।

आम तौर पर, इस तरह के एक वर्ग का विस्तार करने का मतलब है कि आप इसका व्यवहार बदलना चाहते हैं।

+0

वही .. मैं – Inv3r53

5

उस डिजाइन के साथ क्या गलत है मुख्य रूप से HashMap<Oject, Object> होने का दावा है, लेकिन वास्तव में नहीं है। अधिभारित विधियों को Map विधियों को "प्रतिस्थापित करें", लेकिन वे अभी भी पहुंच योग्य हैं - अब आपको कक्षा का उपयोग इस तरह से करना चाहिए कि Map इंटरफ़ेस के साथ असंगत है और इसका उपयोग करने के लिए (अभी भी तकनीकी रूप से संभव) संगत तरीके से अनदेखा करें।

यह करने के लिए उन के आधार पर नाम और पता के खेतों और hashCode() और equals() तरीकों के साथ एक EmployeeData वर्ग बनाने के लिए सबसे अच्छा तरीका होगा (बेहतर अभी तक या, एक अद्वितीय ID क्षेत्र)। फिर आपको गैर-मानक मानचित्र सबक्लास की आवश्यकता नहीं है - आप बस HashMap<EmployeeData, Object> का उपयोग कर सकते हैं। दरअसल, मान प्रकार Object से भी अधिक विशिष्ट होना चाहिए।

+1

+1 पर sublcassing पर एकत्रीकरण पसंद करूंगा: Liskov प्रतिस्थापन सिद्धांत किसी भी ओओ भाषा में पालन किया जाना चाहिए। –

3

मेरी पहली ले होगा कि:

public MyHashMap extends HashMap<Oject, Object> 

गलत है। कोई प्रकार की सुरक्षा नहीं।

एक मानचित्र है तो आवश्यक कम से कम इसे

public MyHashMap extends HashMap<NameAddress, Employee> 

बनाने के लिए और डाल समायोजित करने और एक ही मिलता है। एक नाम एड्रेस बराबर और हैशकोड में प्रथम नाम, अंतिम नाम और पता का उपयोग करें।

मुझे व्यक्तिगत रूप से लगता है कि एक सेट इंटरफ़ेस संभवतः एक हैश मैप सदस्य के रूप में बेहतर होगा। तो फिर तुम इंटरफ़ेस निर्धारित कर सकते हैं जैसे कि यह होना चाहिए:

public EmployeeSet implements Set<Employee> { 
    private static class NameAddress { 
    public boolean equals(); 
    public int hashCode(); 
    } 

    private HashMap employees = new HashMap<NameAddress, Employee>(); 

    public add(Employee emp) { 
    NameAddress nad = new NameAddress(emp); 
    empoyees.add(nad, emp); 
    } 

    public remove(Employee emp) { 
    } 
} 

और नाम निकालने और कार्यान्वयन के भीतर जानकारी को संबोधित कुंजी बनाने के लिए।

EDIT डेविड ने इंगित किया कि नाम एड्रेस को तुलना करने की आवश्यकता नहीं है और मैंने इंटरफ़ेस के उस हिस्से को हटा दिया है। शुद्धता के लिए हैशकोड जोड़ा गया।

+0

नाम एड्रेस तुलनात्मक क्यों बनाते हैं? हैश मैप सॉर्टेड मैप को लागू नहीं करता है। –

+0

@ डेविड आप सही हैं, मैं अपडेट करूंगा। – extraneon

0

मुझे लगता है कि यह संदर्भ पर बहुत कुछ निर्भर करता है और उन्होंने वास्तव में क्या पूछा। उदाहरण के लिए, एक अत्यधिक समवर्ती संदर्भ है, आपको इसके बजाय हैशटेबल का उपयोग करना चाहिए था। इसके अलावा, आपको कुंजी पर कंपाइलर समर्थन प्राप्त करने के लिए (ऑब्जेक्ट, ऑब्जेक्ट) के बजाय प्रकार (स्ट्रिंग, ऑब्जेक्ट) निर्दिष्ट करना चाहिए।

मुझे लगता है कि नक्शा इंटरफ़ेस को लागू करने और ऑश के आंतरिक पैरामीटर के रूप में हैश मैप/हैशटेबल को रखने के लिए एक बेहतर डिज़ाइन होता।

3

नक्शा पहले नाम, अंतिम नाम और पता के आधार पर एक आंतरिक कुंजी का उपयोग करता है। तो remove विधि (1) को remove(String firstName, String lastName, String Address) के रूप में कार्यान्वित किया जाना चाहिए और (2) इसे केवल मूल परिवर्तन विधि द्वारा मूल निकालने विधि (जो वास्तव में प्रविष्टि को हटा दिया गया) के व्यवहार को नहीं बदला जाना चाहिए। निकालें का इससे बेहतर कार्यान्वयन होगा:

public void remove(String firstName, String lastName, String address) { 
    String key = firstName + lastName + address; 
    return remove(key); 
} 
+0

2) यह मूल निकालने विधि के व्यवहार को नहीं बदलता है, क्योंकि यह इसे ओवरराइड नहीं करता है। –

+1

निश्चित - लेकिन सवाल खराब डिजाइन के लिए कहा। और कुछ पूरी तरह से अलग करने के लिए 'निकालें' विधि नाम का उपयोग करना बहुत बुरा है (यह निकालना विधि कुछ भी नहीं हटाती है)। –

2

दो अन्य अच्छा अभ्यास के खिलाफ "घृणित अपराधों" हैं कि remove विधि:

  • भार के Map.remove(<K>) बल्कि यह अधिभावी से (ताकि आप एक टपकाया अमूर्त है), और

  • उस व्यवहार को लागू करता है जो उस विधि के व्यवहार के साथ असंगत है जो इसे ओवरराइड करता है!

एक खाली स्ट्रिंग में कुंजी के साथ जुड़े मान को सेट करना कुंजी के लिए प्रविष्टि को हटाने के समान नहीं है। (यहां तक ​​कि इसे null पर सेट करना भी काफी अलग है ...)

यह लिस्कोव प्रतिस्थापन सिद्धांत का उल्लंघन नहीं है, लेकिन यह इस प्रकार का उपयोग करने की कोशिश करने वाले किसी व्यक्ति के लिए वास्तव में भ्रमित हो सकता है। (इस पर निर्भर करता है कि आपने Map प्रकार या MyHashMap प्रकार के माध्यम से विधि को बुलाया है, तो आप अलग-अलग अर्थशास्त्र के साथ विभिन्न विधियों का उपयोग कर समाप्त कर सकते हैं ....)

संबंधित मुद्दे