Take the 2-minute tour ×
Programmers Stack Exchange is a question and answer site for professional programmers interested in conceptual questions about software development. It's 100% free, no registration required.

I am struggling with the best way to structure a program that I am writing because the method I currently have feels very clunky and each part depends a lot on the others. This is what the program needs to do:

  1. Parse a word document to extract a list of contacts.
  2. Get a list of contacts in a specified group from my Google account.
  3. Compare the contacts from the word document to the Google contacts and either:
  4. Update the contacts on Google with the details from the word document or
  5. Create a new contact on Google if one doesn't exist or
  6. Delete any contacts on Google that are not in the word document.

I also want

  • to have a GUI for this program that is responsive while all of the above is going on
  • the show a preview of the changes that steps 4-6 will make before actually performing them and allow the user to reject the changes (all or nothing would be fine for now)

Edit: everything from here on down is what I've currently done. I added it so that specific refactoring advice could be given if that was what was deemed necessary. If you have a way that I can structure this program then you can mostly likely skip the rest of it.


I've already made a start so allow me to explain how I've written the code so far. The program is written in C#.

  1. The parsing of the word document to get a list of contacts is in a class called WordDocumentParser. It has a constructor which takes the path of the word document and a GetContacts method which performs all of the parsing and returns a list of contacts. For what it's worth, I'm using COM to handle the parsing.

  2. Steps 2-6 from above all start from a new class called GoogleContactsMaintainer with a constructor that takes the list of contacts from the word document, a couple of important groups on Google and a ContactsRequest which is used to retrieve/update/save/delete contacts on Google. The first thing this class does is to get the contacts from Google and match them to the contacts from the word document. The matched Google contact is saved in a property on the word document contact.

  3. Then I do a preview which is the next 3 steps.

  4. I start off with updating the contacts that have been matched from the word document to the Google contacts. I create a ContactUpdater which updates the matched Google contact but doesn't commit the changes. It also adds to a list of strings any changes that have been made and sets a boolean property on the contact to say that it has been updated.

  5. Next is creating new Google contacts for those word document contacts that haven't been matched to a Google contact. This is done in a ContactCreator class which creates a Google contact, sets it on the word document contact and sets a flag on that contact to indicate it is a new contact. The new contact is not saved.

  6. Last is to delete any Google contacts that haven't been matched to a contact from the word document. To do this, I just find those contacts and add them to a list with a description saying that they will be deleted.

  7. At the minute, I display these changes to the user and then immediately update them (I'll explain why the user can't stop this in a minute). This is done in the following three stages.

  8. Update: find any contacts from the word document that have the HasBeenUpdated property set, construct a new ContactUpdater and commit the changes.

  9. Create: find any contacts from the word document that have the NewContact property set, construct a new ContactCreator and insert the new Google contact.

  10. Delete: find any Google contacts that haven't been matched to a contact from the word document and delete them from Google.

In an attempt to keep my GUI responsive while all of this is going on, I created a MaintainContactsWorker class which subclasses BackgroundWorker and calls all of the above 10 steps from the OnDoWork method. This is why I am unable to ask the user if they want to commit the changes or not.

To me, everything feels horribly inter-connected and completely untestable so I am hoping you can point out things that I should be doing to break the dependencies and make my code more maintainable and testable.

From writing this post, I one thing that I feel I should have is a repository which would handle the CRUD operations I am performing on the Google contacts. Does this seem like a good place to start?

Thank you for any suggestions you can offer, hopefully I've explain myself well enough.

share|improve this question
2  
Consider writing a shorter question. As is, after reading it, I have no idea of what is the question. Are you asking how to communicate between the workflow and the UI? Or maybe how to unit test your code? Or maybe how to refactor your code? –  MainMa Dec 31 '11 at 16:52
1  
Interesting, I thought the intent of the question was fairly obvious (-: Refactoring is the key... but its a bit more than that –  Murph Dec 31 '11 at 17:04
    
@MainMa, I was worried about the length of the post but I thought it was necessary to explain what I needed to happen and what I had already done so that I didn't give the impression of asking for it to be done for me. If you can offer a way to structure the program starting from scratch then you can ignore about 75% of my post and just take the "what I need it to do" part. I'll edit my original post so that it's obvious which bits you can ignore. –  Stu Dec 31 '11 at 17:24
    
@Murph, do you have any suggestions of what refactoring I should perform? Any patterns I can follow? –  Stu Dec 31 '11 at 17:25
1  
The total number of classes should be under 10, in my opinion. The steps 8 through 10 worry me. Perhaps you are trying to learn "the OO way", but the OO way is not always all that. I do not think that you need a separate Contract Creator and a ContractUpdater class, particularly given that there is no ContractDeletor class. Also, you create a brand new object just for the purpose of performing an action, and then you delete it right away? I would have a single ContactManipulator class. Instead of being obsessed with object orientation, concentrate on the simple building blocks - functional way –  Job Dec 31 '11 at 18:58

2 Answers 2

up vote 5 down vote accepted

Your process:

  • Parse a word document to extract a list of contacts.
  • Get a list of contacts in a specified group from my Google account.
  • Compare the contacts from the word document to the Google contacts and either:
  • Update the contacts on Google with the details from the word document or
  • Create a new contact on Google if one doesn't exist or
  • Delete any contacts on Google that are not in the word document.

One possible structure:

WordDocParser docParser = new WordDocParser(path);
IList<Contact> wordContacts = docParser.ParseContacts();

GoogleContactsManager googleManager = new GoogleContactsManager(accountInfo);
IList<Contact> googleContacts = googleManager.GetContacts();

ContactComparer comparer = new ContactComparer();
IList<ContactUpdateAction> updateActions = 
    comparer.Compare(wordContacts, googleContacts);

//user can edit/alter update actions here

googleContacts.UpdateContacts(updateActions);

For a responsive GUI, showing what step is happening with a progress bar should suffice up to the point where the user can see/alter the update actions. You can use a background/worker thread for each step, with a button to allow the user to cancel the process. The background thread can update a flag (Mutex or similar) when it is done to signal for the launch of the next background step.

Each UpdateAction can specify whether to add, delete, or change the contact information. This information can be displayed to the user before making changes to the Google account (at least in theory!) When the user is satisfied, they can click a Commit Changes button to execute the last step, or Cancel.

share|improve this answer
    
+1, less code/classes is more! –  Job Jan 1 '12 at 16:23
    
Thanks very much Steven. I've taken your answer and implemented it almost exactly as you suggested. It's a very clean way of writing the code. I'm struggling however, to get the responsive UI I was hoping for. You mentioned that I could use a background worker/thread and update a flag when each step was done to move on to the next one. I tried to use ManualResetEvent but had no joy - the UI was still unresponsive. Could you either point me in the direction of an example (my Google-fo has failed me) or post some more information about how to accomplish that please? –  Stu Jan 2 '12 at 17:22
    
I managed to get something working to keep the UI responsive but I'm not altogether too happy with it so I've posted over at codereview.se. You can see it here. Thank you for your help, I'm going to accept this as the answer. –  Stu Jan 2 '12 at 22:46
    
@Stu you're welcome, glad it worked for you. "responsive" is a relative term - if your processing is running in the background, the user can click on the GUI, but unless you're checking for an interruption at every atomic operation the user may have to wait a while for something to finish before they see anything happen. I tend to use SafeThread (codeproject.com/KB/threads/SafeThread.aspx) for background processes and haven't played with the newer worker/task classes yet. –  Steven A. Lowe Jan 5 '12 at 20:55

(I'm a Java programmer, not C#.)

I would have a class ContactSet, with subclasses GoogleContactSet and WordFileContactSet. The interface would have 4 methods: iterator(), add(Contact c), remove(Contact c) and update(Contact c). Internally, ContactSet would store a Set (in Java, but I guess C# has an equivalent unordered set). The method iterator() just returns the iterator of the underlying Set, and the same for add(...) and remove(...). The contacts from Google at read in the creator new GoogleContactSet(googleID) and those from the file are read in the creator new WordFileContactSet(fileName).

I would also add a fifth method in ContactSet: findExtraneous(ContactSet d) that returns a ContactSet of all Contacts that are in the current set, but not in d. This would be implemented as a double loop over the iterators of this and d.

It seems quite easy to devise some tests for the above classes and methods.

findExtraneous(...) would be called at the start of the GUI code. Then you have some code that seeks user confirmation and puts the Contacts that will be added/removed/updated in 3 corresponding ContactSet's. Finally, you call googleContactSet.add(someContact) or remove or update on the corresponding set of contacts. You'll have to set this up in some methods in the C# GUI (I'm not familiar with that part).

You don't need classes ContactUpdater, ContactRemover; that would be excessive OO.

Testing GUI code is always difficult, if not impossible.

EDIT: on top of contactSet.add(Contact c), put(Contact c) and remove(Contact c), you could add 3 corresponding methods that take a set as argument, e.g. contactSet.add(ContactSet set). But I would not add a method that directly returns the underlying Set instead of the iterator, since the Set could then be manipulated directly by anyone.

share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.