2010-04-07 5 views
5

ऐसा लगता है कि मुझे यहां बहुत सारे कोड को रेखांकित करना था। मैं सोच रहा हूँ अगर यह इस तरह एक हेडर फाइल में पूरी तरह से छोड़ने के लिए बुरा डिजाइन अभ्यास है:क्या हेडर केवल लाइब्रेरी के लिए यह बहुत अधिक कोड है?

#include <list> 
#include <string> 
#include <boost/noncopyable.hpp> 
#include <boost/make_shared.hpp> 
#include <boost/iterator/iterator_facade.hpp> 
#include <Windows.h> 
#include "../Exception.hpp" 

namespace WindowsAPI { namespace FileSystem { 

class NonRecursiveEnumeration; 
class RecursiveEnumeration; 
struct AllResults; 
struct FilesOnly; 

template <typename Filter_T = AllResults, typename Recurse_T = NonRecursiveEnumeration> 
class DirectoryIterator; 

template <typename Recurse_T> 
struct FileData; 

class NonRecursiveEnumeration : public boost::noncopyable 
{ 
    WIN32_FIND_DATAW currentData; 
    HANDLE hFind; 
    std::wstring root; 
public: 
    NonRecursiveEnumeration() : hFind(INVALID_HANDLE_VALUE) { 
    }; 
    NonRecursiveEnumeration(const std::wstring& pathSpec) { 
     std::wstring::const_iterator lastSlash = 
      std::find(pathSpec.rbegin(), pathSpec.rend(), L'\\').base(); 
     if (lastSlash != pathSpec.end()) 
      root.assign(pathSpec.begin(), lastSlash); 
     hFind = FindFirstFileW(pathSpec.c_str(), &currentData); 
     if (hFind == INVALID_HANDLE_VALUE) 
      WindowsApiException::ThrowFromLastError(); 
     while (!wcscmp(currentData.cFileName, L".") || !wcscmp(currentData.cFileName, L"..")) { 
      increment(); 
     } 
    }; 
    void increment() { 
     BOOL success = 
      FindNextFile(hFind, &currentData); 
     if (success) 
      return; 
     DWORD error = GetLastError(); 
     if (error == ERROR_NO_MORE_FILES) { 
      FindClose(hFind); 
      hFind = INVALID_HANDLE_VALUE; 
     } else { 
      WindowsApiException::Throw(error); 
     } 
    }; 
    ~NonRecursiveEnumeration() { 
     if (hFind != INVALID_HANDLE_VALUE) 
      FindClose(hFind); 
    }; 
    bool equal(const NonRecursiveEnumeration& other) const { 
     if (this == &other) 
      return true; 
     return hFind == other.hFind; 
    }; 
    const std::wstring& GetPathRoot() const { 
     return root; 
    }; 
    const WIN32_FIND_DATAW& GetCurrentFindData() const { 
     return currentData; 
    }; 
}; 

//Not implemented yet 
class RecursiveEnumeration : public boost::noncopyable 
{ 
}; 

template <typename Recurse_T> 
struct FileData //Serves as a proxy to the WIN32_FIND_DATA struture inside the iterator. 
{ 
    const Recurse_T* impl; 
    template <typename Filter_T, typename Recurse_T> 
    FileData(const DirectoryIterator<Filter_T, Recurse_T>* parent) : impl(parent->impl.get()) {}; 
    DWORD GetAttributes() const { 
     return impl->GetCurrentFindData().dwFileAttributes; 
    }; 
    bool IsDirectory() const { 
     return (GetAttributes() & FILE_ATTRIBUTE_DIRECTORY) != 0; 
    }; 
    bool IsFile() const { 
     return !IsDirectory(); 
    }; 
    bool IsArchive() const { 
     return (GetAttributes() & FILE_ATTRIBUTE_ARCHIVE) != 0; 
    }; 
    bool IsReadOnly() const { 
     return (GetAttributes() & FILE_ATTRIBUTE_READONLY) != 0; 
    }; 
    unsigned __int64 GetSize() const { 
     ULARGE_INTEGER intValue; 
     intValue.LowPart = impl.GetCurrentFindData().nFileSizeLow; 
     intValue.HighPart = impl.GetCurrentFindData().nFileSizeHigh; 
     return intValue.QuadPart; 
    }; 
    std::wstring GetFolderPath() const { 
     return impl->GetPathRoot(); 
    }; 
    std::wstring GetFileName() const { 
     return impl->GetCurrentFindData().cFileName; 
    }; 
    std::wstring GetFullFileName() const { 
     return GetFolderPath() + GetFileName(); 
    }; 
    std::wstring GetShortFileName() const { 
     return impl->GetCurrentFindData().cAlternateFileName; 
    }; 
    FILETIME GetCreationTime() const { 
     return impl->GetCurrentFindData().ftCreationTime; 
    }; 
    FILETIME GetLastAccessTime() const { 
     return impl->GetCurrentFindData().ftLastAccessTime; 
    }; 
    FILETIME GetLastWriteTime() const { 
     return impl->GetCurrentFindData().ftLastWriteTime; 
    }; 
}; 

struct AllResults 
{ 
    template <typename Recurse_T> 
    bool operator()(const FileData<Recurse_T>&) { 
     return true; 
    }; 
}; 

struct FilesOnly 
{ 
    template <typename Recurse_T> 
    bool operator()(const FileData<Recurse_T>& arg) { 
     return arg.IsFile(); 
    }; 
}; 

#pragma warning(push) 
#pragma warning(disable: 4355) 
template <typename Filter_T, typename Recurse_T> 
class DirectoryIterator : public boost::iterator_facade<DirectoryIterator<Filter_T>, const FileData<Recurse_T>, std::input_iterator_tag> 
{ 
    friend class boost::iterator_core_access; 
    boost::shared_ptr<Recurse_T> impl; 
    FileData<Recurse_T> derefData; 
    Filter_T filter; 
    void increment() { 
     do { 
      impl->increment(); 
     } while (! filter(derefData)); 
    }; 
    bool equal(const DirectoryIterator& other) const { 
     return impl->equal(*other.impl); 
    }; 
    const FileData<Recurse_T>& dereference() const { 
     return derefData; 
    }; 
public: 
    typedef FileData<Recurse_T> DataType; 
    friend struct DataType; 
    DirectoryIterator(Filter_T functor = Filter_T()) : 
     impl(boost::make_shared<Recurse_T>()), 
     derefData(this), 
     filter(functor) { 
    }; 
    explicit DirectoryIterator(const std::wstring& pathSpec, Filter_T functor = Filter_T()) : 
     impl(boost::make_shared<Recurse_T>(pathSpec)), 
     derefData(this), 
     filter(functor) { 
    }; 
}; 
#pragma warning(pop) 

}} 
+4

यदि यह केवल शीर्षलेख है, तो मुझे लगता है कि आपकी सबसे अच्छी शर्त इसे कई फ़ाइलों में विभाजित करना है। "विस्तार/DirectoryIterator.hpp", आदि की तरह, कम से कम यह एक फ़ाइल में निहित नहीं है। बूस्ट करता है यही है। वैसे, बूस्ट में एक फाइल सिस्टम लाइब्रेरी है, बस अगर आपको पता नहीं था। – GManNickG

+0

@GMan: +1। हां, बूस्ट में एक फाइल सिस्टम लाइब्रेरी है, लेकिन इसे क्रॉस प्लेटफार्म संगतता के लिए डिज़ाइन किया गया है, जिसके लिए मुझे पसंद नहीं होने वाले वर्म्स का पूरा खुलना आवश्यक है (उदाहरण के लिए इसका अपना पथ प्रोसेसर है)। इसके साथ मुझे लगता है कि मैं क्लाइंट कोड स्पष्ट कर सकता हूं जो क्रॉस प्लेटफार्म पोर्टेबिलिटी की परवाह नहीं करता है। –

+0

यह देखने में अच्छा लगा कि आप का अच्छा उपयोग हो रहा है: http://stackoverflow.com/questions/2531874/how-might-i-wrap-the-findxfile-style-apis-to-the-stl-style-iterator -पटर-इन-सी –

उत्तर

9

मैं, मेरा में से कुछ में और अधिक कोड है, तो है कि किसी भी सांत्वना का है। और इसलिए सभी सी ++ मानक लाइब्रेरी कार्यान्वयन, बूस्ट और माइक्रोसॉफ्ट (उदाहरण के लिए, एटीएल) करें।

1

जहां तक ​​हेडर की लंबाई बढ़ जाती है, तब तक आप अपने हेडर फ़ाइलों में जितना चाहें उतना कोड प्राप्त कर सकते हैं। व्यापार बंद कोड की मात्रा है जिसे हर बार आपके प्रोग्राम के निर्माण के बाद पुन: संकलित किया जाना चाहिए; आपके सीपीपी फाइलों में रखा गया कोड ऑब्जेक्ट फाइलों में संकलित किया जा सकता है और प्रत्येक आगामी निर्माण पर लिंक किया जा सकता है।

मैं सुझाव दूंगा कि DirectoryIteratorImpl के लिए प्रत्येक विधि परिभाषा .cpp फ़ाइल में ले जाया जाना चाहिए। यदि आप कक्षा परिभाषा के अंदर एक विधि इनलाइन परिभाषित नहीं कर रहे हैं, तो हेडर फ़ाइल में इसे शामिल करने का कोई कारण नहीं है।

एक असंबंधित एक तरफ: inline DirectoryIteratorImpl(); लिखने से बचें - वास्तव में अपने इनलाइन फ़ंक्शंस इनलाइन लिखें, या इनलाइन को चिह्नित न करें। C++ FAQ Lite से:

यह आमतौर पर आवश्यक है कि समारोह की परिभाषा एक हेडर फाइल में रखा जाना (के बीच {...} भाग)। यदि आप इनलाइन फ़ंक्शन की परिभाषा को .cpp फ़ाइल में डालते हैं, और यदि इसे किसी अन्य .cpp फ़ाइल से कहा जाता है, तो आपको लिंकर से "अनसुलझा बाहरी" त्रुटि मिल जाएगी।

यदि आपके फ़ंक्शन हेडर फ़ाइल में लिखने के लिए "बहुत बड़ा" हैं, तो वे इनलाइन के लिए बहुत बड़े हैं और संकलक संभवतः आपके इनलाइन सुझाव को अनदेखा कर देगा।

+0

# 1। आपने मेरे प्रश्न का उत्तर नहीं दिया। मुझे पता है कि इस कोड का उपयोग करने वाली प्रत्येक फ़ाइल में कोड डाला जाता है - मैं सोच रहा था कि इससे बहुत अधिक कोड ब्लोट हो जाएगा। # 2। मैं कार्यों को वर्ग का एक शाब्दिक हिस्सा नहीं बना सकता क्योंकि उनमें से आधे 'कक्षा फ़ाइलडेटा' की पूर्ण परिभाषा पर निर्भर हैं। आपको क्यों लगता है कि मैंने सभी 'फाइलडाटा' सदस्यों को स्पष्ट रूप से इनलाइन घोषित कर दिया है? :) –

+0

@ बिली शायद प्रश्न के शरीर को निर्दिष्ट किया जाना चाहिए था। साथ ही, मेरा सुझाव खड़ा है - 'इनलाइन' परिभाषा को निक्स करें यदि आप फ़ंक्शन इनलाइन घोषित नहीं कर सकते हैं। – meagar

+0

@ मीगर: मुझे लगता है कि इस तरह का निहित है। फ़ंक्शन इनलाइन बनाने के प्रभाव मूल रूप से स्पष्ट होना चाहिए। मैं पूछ रहा था कि क्या यह एक अच्छा विचार है। एकमात्र कारण यह कभी भी एक बुरा विचार होगा यदि यह बहुत अधिक कोड ब्लोट का कारण बनता है। –

6

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

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

+0

दैनिक वोट सीमा तक पहुंच गया; 4 घंटों में वापस आओ। <- अरे! मैं 4 घंटों में ऊपर उठ जाऊंगा। –

+0

@ बिली: हा, आप निश्चित रूप से वोटों से बाहर निकलते हैं। आपके लिए +1 किया – GManNickG

+0

@GMan: आज सुबह 8 की तरह से बाहर चला गया। * विधेयक अधिक वोटों के लिए याचिका के लिए मेटा को चलाता है। (मजाक कर रहा हूं) –

1

ऐसा लगता है कि आप यहां विंडोज के लिए प्रोग्रामिंग कर रहे हैं, क्या हम मान लेंगे कि आप विजुअल स्टूडियो का उपयोग कर रहे हैं?

वैसे भी, मुझे नहीं लगता कि हेडर में बहुत अधिक कोड है।

यह व्यापार नापसंद ज्यादातर की बात है:

  • धीमी संकलन (लेकिन हम multicores और precompiled हेडर है)
  • अधिक लगातार रखता (फिर से, multicores)
  • शायद कोड ब्लोट ...

एकमात्र बिंदु जो परेशान है (मेरी राय में) नवीनतम है ... और मुझे यहां सहायता चाहिए: क्या हमें यकीन है कि कार्यों को रेखांकित किया जा रहा है, क्या यह संभव नहीं है कि संकलन आर और लिंकर तय करते हैं कि उन्हें इनलाइन न करें और उन्हें नियमित कॉल में बदल दें?

सचमुच, मैं इसके बारे में बहुत ज्यादा चिंता नहीं करता। Boost पुस्तकालयों की संख्या केवल उनके गैर-टेम्पलेट भागों के लिए ही है-क्योंकि यह एकीकरण को आसान बनाता है (कोई लिंकिंग आवश्यक नहीं है)।

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