• fairly robust and explanatory command line options (thanks to python's optparse module)
  
• embraced an extensible OOP approach, highlighting my subject matter expertise
  
• allowed a deep level user configuration via configuration files (thanks to python's ConfigParser module)
  

lineFindAllRegexFlag:re.DOTALL+re.IGNORECASE+re.MULTILINE

        for section in _sourceConfig.sections():
            _sectionDict = {}
            for option in _sourceConfig.options(section):
                if re.search("[i]flag$[/i]",option,re.IGNORECASE):
[b]                    _regexFlag=0
                    flags=_sourceConfig.get(section,option)
                    if re.search(r"re\.I",flags,re.I):
                        _regexFlag=_regexFlag+re.IGNORECASE
                    if re.search(r"re\.M",flags,re.I):
                        _regexFlag=_regexFlag+re.MULTILINE
                    if re.search(r"re\.D",flags,re.I):
                        _regexFlag=_regexFlag+re.DOTALL
                    _sectionDict[option]=_regexFlag[/b]

[b]                elif re.search("[i]swapterms$[/i]",option,re.IGNORECASE) and len(_sourceConfig.get(section,option)):
                    _swapDict={}
                    for _swapTerm in _sourceConfig.get(section,option).split(","):
                        _swapDict[_swapTerm.split("=")[0]]=_swapTerm.split("=")[1]
                    _sectionDict[option]=_swapDict
                else:
                    _sectionDict[option]=_sourceConfig.get(section,option)
                    
            _sourceDict[section.lower()]=_sectionDict
[/b]

JoeNotCharles fucked around with this message at 15:44 on Jul 10, 2008

JoeNotCharles fucked around with this message at 17:16 on Jul 10, 2008

• It was written in a barely-readable mixture of C and C++. The mixture by itself doesn't strike me as being that bad, but if half your program is already in C++, why not go the whole way and write the entire thing in C++? And if you're going to write in C, must your printf statements be totally opaque and your variables named things like "ma" and "ce"?
  
• For some reason, this program was seriously multithreaded. I can understand perhaps splitting the network functionality into a different thread, but he made the exposure process its own thread as well. The camera gets its own external trigger that is independent of the software, so all you actually have to do is to poll the state and read it out; no need for it to have its own thread. What's more, most of the threads don't actually need to run concurrently with the main thread. They were apparently implemented so that virtually any process could be killed externally by the main thread, a rationale I'm still struggling to understand.
  
• As a consequence of the above point, virtually every variable of any import is global. This would be understandable for semaphores, but we're talking about everything from the current status of the camera to the data buffers themselves. Also every function is declared

  code:

  void function()

  . That's right, about half the functions in this program don't take arguments or return values. Why bother when everything is global anyway?!
  
• The threading isn't even done properly. There are some semaphores that basically signal things like network events or camera exposures, but absolutely no concurrency checking is done; this means that it's possible (unlikely, but possible) for the camera parameters to change mid-exposure. There are no mutexes or anything that would make sense to have in a threaded program.
  
• There are several classes implemented in this software that are essentially reinventions of the wheel. For example, when analyzing an image, you want to know how many stars (blobs) there are in it, and you want to store them and their coordinates. Logic would dictate (to me; correct me if I'm wrong) that one would create a class called Blob and make a vector<Blob> variable. You could even get away with a struct, since blobs are just coordinate pairs that don't need methods. What logic would not dictate would be to reinvent the concept of the doubly-linked list. That's right, he just coded a linked-list class, complete with all the usual operations (insert, delete, etc.) so he could store blobs. But this isn't even the best thing.
  
• The best thing about this program is that it's linked against Qt. Now, you might ask yourselves, why would a program that is essentially autonomous and has no graphical interface to speak of need to be linked against Qt? What possible features could Qt provide for this project? Whatever they are, you would think they're worth it; after all, building Qt for Microsoft Visual Studio is a huge pain in the rear end and it takes up a ton of space. Well, I'm going to put in a few lines to let you ponder what function Qt serves in this project.
  
  
  
  
  
  Have you figured it out yet?
  
  
  
  
  The answer is lists!
  
  
  That's right, dude linked against Qt so he could use a generic structure which is already present in standard C++. I nearly poo poo a brick when I figured out what Qt was for; I mean, you don't have to be a C++ wizard to use the STL, you just have to be aware of its existence. Now his reinvention of doubly-linked lists makes perfect sense to me; he probably wasn't even aware such a function already existed. Nevertheless, he did take the time to figure out how to do graphics programming with the Windows API (to display the camera images on the screen), which is the one area where I would (and eventually did) bring in an outside library just for programming simplicity.
  
• To add insult to injury (and this is a highly specialized point, but one that is immediately obvious to people in astronomy), instead of saving all the images in FITS format (which is the most popular format for saving astronomical data) he wrote his own wrapper class for libtiff. Also, large chunks of the code and the structure of the program are highly specific to the hardware they were using at the time and could not be easily changed.