Boost logo

Boost :

Subject: Re: [boost] [Boost.Breakable] Any interest in a Boost Breakable library?
From: Stewart, Robert (Robert.Stewart_at_[hidden])
Date: 2009-09-08 10:14:32


Pierre Morcello wrote:
>
> Thanks Michael, I did not think sooner to the 'else if'. But

I like the "else if" version better, too.

> there is one problem with the 'else if' : you can not do
> calculations between the tests.
>
> When I work often my code looks like :
>
> // retrieve meshes from prototype
> Breakable
> {
> // find loaded meshes
> std::set<Mesh*> lLoadedMeshes;
> {
> // some calculations that fill lLoadedMeshes
> }
>
> if(lLoadedMeshes.empty())
> {
> break;
> }
>
> // find loadable scenes that uses the already loaded meshes
> std::set<Scene*> lLoadableScenes;
> {
> // some calculations that fill lLoadableScenes
> // it uses the result of lLoadedMeshes
> }
>
>
> if(lLoadedScenes.empty())
>
> {
>
> break;
>
> }
>
> // and so on..
> }

The whole point of your scheme is to skip subsequent code if you
encounter some sort of error. Therefore, this will do nicely:

{
   std::set<Mesh*> lLoadedMeshes;
   if (!find_loaded_meshes(lLoadedMeshes))
   {
      // complain and return or throw
   }
   std::set<Scene*> lLoadableScenes;
   if (!find_loadable_scenes(lLoadableScenes, lLoadedMeshes))
   {
      // complain and return or throw
   }
   // and so on..
}

bool find_loaded_meshes(std::set<Mesh*> & _meshes)
{
   // some calculations that fill _meshes
   if (_meshes.empty())
   {
      return false;
   }
   // do more
   return true;
}

bool find_loadable_scenes(std::set<Scene*> & _scenes,
   std::set<Mesh*> const & _meshes)
{
   {
      // some calculations that fill lLoadableScenes
      // it uses the result of _meshes
   }
   if (_scenes.empty())
   {
      return false;
   }
   // do more
   return true;
}

You said before that splitting things into multiple functions
reduced readability. My version is more readable for several
reasons. Each function has a single purpose rather than one
function doing many things. Notice that I removed some comments
in your version because the function names obviated them (self-
documenting code). Finally, the high level logic, in the initial
block, is clearer, while the low level logic is segregated into
separate functions.

If you'd prefer to avoid early returns, "else" is handy:

{
   std::set<Mesh*> lLoadedMeshes;
   if (!find_loaded_meshes(lLoadedMeshes))
   {
      // complain and return or throw
   }
   else
   {
      std::set<Scene*> lLoadableScenes;
      if (!find_loadable_scenes(lLoadableScenes, lLoadedMeshes))
      {
         // complain and return or throw
      }
      // and so on..
   }
}

_____
Rob Stewart robert.stewart_at_[hidden]
Software Engineer, Core Software using std::disclaimer;
Susquehanna International Group, LLP http://www.sig.com

IMPORTANT: The information contained in this email and/or its attachments is confidential. If you are not the intended recipient, please notify the sender immediately by reply and immediately delete this message and all its attachments. Any review, use, reproduction, disclosure or dissemination of this message or any attachment by an unintended recipient is strictly prohibited. Neither this message nor any attachment is intended as or should be construed as an offer, solicitation or recommendation to buy or sell any security or other financial instrument. Neither the sender, his or her employer nor any of their respective affiliates makes any warranties as to the completeness or accuracy of any of the information contained herein or that this message or any of its attachments is free of viruses.


Boost list run by bdawes at acm.org, gregod at cs.rpi.edu, cpdaniel at pacbell.net, john at johnmaddock.co.uk