MoleculeFile / Open Babel wrapper

Development
1

I think the new MoleculeFile class is great and we should use it
throughout – particularly in MainWindow.

A few things on code review:

  1. We should have a mechanism to return QString for error messages.
    For example, everything in mainwindow.cpp should be there – can’t
    read the file, can’t load the file format, can’t recognize the file
    format, etc.

  2. We should store the OBMol internally and return the
    Avogadro::Molecule or OBMol depending on desire. For example,
    mainwindow.cpp checks for 2D and does a cleanup.

  3. We should also add Save/Write capabilities. It’s not too bad – if
    you’re currently editing molecule #10, you copy out everything in the
    file up to that stream position to a new file. Then you write the new
    molecule #10, then you copy everything starting at the streampos for
    molecule #11. Then you move the new file to replace the old filename.

  4. Of course it’d be great to have a QListWidget in Avogadro –
    letting the user flip between different molecules in the file. Seems
    like that should be pretty quick to add now too.

-Geoff

2

Hi,

On Fri, May 22, 2009 at 9:44 PM, Geoffrey Hutchison geoffh+@pitt.edu wrote:

I think the new MoleculeFile class is great and we should use it
throughout – particularly in MainWindow.

A few things on code review:

  1. We should have a mechanism to return QString for error messages.
    For example, everything in mainwindow.cpp should be there – can’t
    read the file, can’t load the file format, can’t recognize the file
    format, etc.

Yes, the bool return value is too limited. I’ll get it in before the
string freeze.

  1. We should store the OBMol internally and return the
    Avogadro::Molecule or OBMol depending on desire. For example,
    mainwindow.cpp checks for 2D and does a cleanup.

Sounds good.

  1. We should also add Save/Write capabilities. It’s not too bad – if
    you’re currently editing molecule #10, you copy out everything in the
    file up to that stream position to a new file. Then you write the new
    molecule #10, then you copy everything starting at the streampos for
    molecule #11. Then you move the new file to replace the old filename.

Yes, the function names also need to be a bit more consistent. We have:

Molecule* OpenbabelWrapper::openFile(filename, type, options) // added by Marcus
bool OpenbabelWrapper::saveFile(molecule, filename, type) // added by Marcus

bool OpenbabelWrapper::writeConformers(molecule, filename, type)
MoleculeFile* readFile(filename, type, options, wait)

openFile & readFile overlap but having a simple function which
directly returns the molecule is useful. Should we use read/write or
open/save? I prefer read/write and suggest the following:

  • openFile → readMolecule
  • saveFile → writeMolecule
  • Add MoleculeFile::writeMolecule to do the “insert writing”
  1. Of course it’d be great to have a QListWidget in Avogadro –
    letting the user flip between different molecules in the file. Seems
    like that should be pretty quick to add now too.

Yes, you can get the list of titles as QSringList which can be
displayed directly in a QListWidget.

Tim

-Geoff

Register Now for Creativity and Technology (CaT), June 3rd, NYC. CaT
is a gathering of tech-side developers & brand creativity professionals. Meet
the minds behind Google Creative Lab, Visual Complexity, Processing, &
iPhoneDevCamp asthey present alongside digital heavyweights like Barbarian
Group, R/GA, & Big Spaceship. http://www.creativitycat.com

Avogadro-devel mailing list
Avogadro-devel@lists.sourceforge.net
avogadro-devel List Signup and Options

3

On May 28, 2009, at 7:03 PM, Tim Vandermeersch wrote:

I think the new MoleculeFile class is great and we should use it
throughout – particularly in MainWindow.

Yes, the function names also need to be a bit more consistent. We
have:

Molecule* OpenbabelWrapper::openFile(filename, type, options) //
added by Marcus
bool OpenbabelWrapper::saveFile(molecule, filename, type) // added
by Marcus

Incidentally, do we need both OpenbabelWrapper and MoleculeFile? I
prefer the latter name, since it’s more descriptive.

  • openFile → readMolecule
  • saveFile → writeMolecule
  • Add MoleculeFile::writeMolecule to do the “insert writing”

We’ll also need some way to “write just one”… Or I guess if we’re
getting fancy, a first and last slice, and an enum for splitting:

writeMolecule(int first = -1, int last = -1, enum SplitMolecularFile);

I’d be happy to help with anything you don’t have time to do.

Yes, you can get the list of titles as QSringList which can be
displayed directly in a QListWidget.

Do you want me to get that part for mainwindow?

Thanks,
-Geoff

4

On Sun, May 31, 2009 at 12:19 AM, Geoffrey Hutchison geoff.hutchison@gmail.com wrote:

On May 28, 2009, at 7:03 PM, Tim Vandermeersch wrote:

I think the new MoleculeFile class is great and we should use it
throughout – particularly in MainWindow.

Yes, the function names also need to be a bit more consistent. We have:

Molecule* OpenbabelWrapper::openFile(filename, type, options) // added by
Marcus
bool OpenbabelWrapper::saveFile(molecule, filename, type) // added by
Marcus

Incidentally, do we need both OpenbabelWrapper and MoleculeFile? I prefer
the latter name, since it’s more descriptive.

The MoleculeFile was initially meant to represent a single file but we
could add all read/write functions to it… (Marcus suggested this on
irc too) I has some alternatives for OpenbabelWrapper though: Storage,
HardDisk and in the future even Database.

  • openFile → readMolecule
  • saveFile → writeMolecule
  • Add MoleculeFile::writeMolecule to do the “insert writing”

We’ll also need some way to “write just one”… Or I guess if we’re getting
fancy, a first and last slice, and an enum for splitting:

writeMolecule(int first = -1, int last = -1, enum SplitMolecularFile);

I’m now working on a MoleculeFile::replaceMolecule, insertMolecule,
appendMolecule and removeMolecule to work with the cached results. The
splitting might be nice as a static function though.

I’d be happy to help with anything you don’t have time to do.

Yes, you can get the list of titles as QSringList which can be
displayed directly in a QListWidget.

Do you want me to get that part for mainwindow?

Sure, if you are volunteering.

Thanks,
Tim

Thanks,
-Geoff