[Last Call] Learn how to a build a cloud-first strategyRegister Now

x
  • Status: Solved
  • Priority: Medium
  • Security: Public
  • Views: 187
  • Last Modified:

Returning a value correctly from STL list

Hi,

I would like to know if this is being done correctly, I believe it is but I just want to make sure!


bool ABC::GetCarSpec(int carSpecIndex,
                       SCarSpec **carSpec)
{
      // Initialise the output parameters
      if (carSpec)
        *carSpec= NULL;


      list<SCarSpec *>::iterator itlistpCurrentCarSpec = m_listpsCarSpec.begin();

      // Skip to the required car spec in list
      for (unsigned int uiIndex = 0; uiIndex < carSpecIndex; uiIndex++)
            itlistpCurrentCarSpec++;


     // Return the required car spec

     if (!(*itlistpCurrentCarSpec))
       return (false);

     *carSpec = *itlistpCurrentCarSpec;


      return (true);
}


Many thanks!
0
Brent-Campbell
Asked:
Brent-Campbell
  • 2
  • 2
  • 2
2 Solutions
 
rajeev_devinCommented:
I would have done something like this

bool GetCarSpec(int carSpecIndex,
                      SCarSpec **carSpec)
{
     // Initialise the output parameters
     if (carSpec)
       *carSpec = NULL;

   if (carSpecIndex >= m_listpsCarSpec.size())
        return (false);

     list<SCarSpec *>::iterator itlistpCurrentCarSpec = m_listpsCarSpec.begin();

     // Skip to the required car spec in list
     for (unsigned int uiIndex = 0; uiIndex < carSpecIndex; uiIndex++)
           itlistpCurrentCarSpec++;

     // Return the required car spec
     *carSpec = *itlistpCurrentCarSpec;
     return (true);
}
0
 
rajeev_devinCommented:
>> for (unsigned int uiIndex = 0; uiIndex < carSpecIndex; uiIndex++)
>>           itlistpCurrentCarSpec++;

Check if carSpecIndex is greater than the size of the list.
0
 
mrblueCommented:
Instead of this

for (unsigned int uiIndex = 0; uiIndex < carSpecIndex; uiIndex++)
           itlistpCurrentCarSpec++;

you could use

std::advance(...);

Shouldn't it be

if (itlistpCurrentCarSpec == m_listpsCarSpec.end())
       return (false);

instead of:

if (!(*itlistpCurrentCarSpec))
       return (false);

And instead of:

if (carSpec)
       *carSpec = NULL;

use rather

if (!carSpec) return FALSE;

*carSpec = NULL;




0
VIDEO: THE CONCERTO CLOUD FOR HEALTHCARE

Modern healthcare requires a modern cloud. View this brief video to understand how the Concerto Cloud for Healthcare can help your organization.

 
mrblueCommented:
Go back

It should be

if (itlistpCurrentCarSpec == m_listpsCarSpec.end())
       return (false);

if (!(*itlistpCurrentCarSpec))                 // and also this
       return (false);
0
 
rstaveleyCommented:
Why not use a vector of pointers or have a list of objects rather than a list of pointers to objects?
0
 
rstaveleyCommented:
Split points rajeev_devin and mrblue (and e-mail a beer to rstaveley)
0

Featured Post

Free Tool: Site Down Detector

Helpful to verify reports of your own downtime, or to double check a downed website you are trying to access.

One of a set of tools we are providing to everyone as a way of saying thank you for being a part of the community.

  • 2
  • 2
  • 2
Tackle projects and never again get stuck behind a technical roadblock.
Join Now