Whole Tomato Software Forums
Whole Tomato Software Forums
Main Site | Profile | Register | Active Topics | Members | Search | FAQ
User name:
Password:
Save Password
Forgot your password?

 All Forums
 Visual Assist
 Technical Support
 Refactoring Issues
 New Topic  Reply to Topic
 Printer Friendly
Author Previous Topic Topic Next Topic  

swinefeaster
Tomato Guru

310 Posts

Posted - Nov 03 2006 :  11:11:31 AM  Show Profile  Reply with Quote
original function:

void
CDataServer::GetDatasetPointData(WORD datasetId, 
 const CGetGraphPointDataInfo& GetInfo, 
 CGetGraphPointDataResultInfo& ReturnedResultInfo)const
{
  CGraphDataServerAutoLock AutoLock(this, datasetId);
  {
    CPointDouble PreviousInRangeDataPoint(0, 0);
    bool previousInRangePointValid = false;

    if(GetInfo.m_xMaxPeripheral && ReturnedResultInfo.m_found
     && !ReturnedResultInfo.m_foundPeripheral)
    {
      PreviousInRangeDataPoint = ReturnedResultInfo.m_DataPoint;
      previousInRangePointValid = true;
    }

    bool previousPointFoundWasMinPeripheral = false;
    {
      if(GetInfo.m_xMinPeripheral && ReturnedResultInfo.m_found
       && ReturnedResultInfo.m_foundPeripheral)
      {
        previousPointFoundWasMinPeripheral = true;
      }
    }

    ReturnedResultInfo.m_found = false;
    ReturnedResultInfo.m_foundPeripheral = false;

    const CDataset* Dataset = GetDatasetById(datasetId);

    if(Dataset)
    {
      int searchIndex = 0;
      const int increment = 1;
      {
        if(Dataset->IsValidIndex(GetInfo.m_nextIterationHint))
        {
          // Start searching at this index.
          searchIndex = GetInfo.m_nextIterationHint;
        }
      }

      CPointDouble PreviousOutOfRangeDataPoint(0, 0);
      bool previousOutOfRangePointValid = false;
      int previousRangeArea = -2;

      while(Dataset->IsValidIndex(searchIndex))
      {
        CPointDouble DataPoint(0, 0);
        Dataset->GetDataPoint(searchIndex, DataPoint);

        if(!GetInfo.IsYValueInRange(DataPoint.y))
        {
          searchIndex += increment;
        }
        else
        {
          int rangeArea = GetInfo.GetXRangeArea(DataPoint.x);
          if(rangeArea == 0)
          {
            // Got it.

            if(GetInfo.m_xMinPeripheral && previousOutOfRangePointValid)
            {
              // But return the peripheral point.
              ReturnedResultInfo.m_DataPoint = PreviousOutOfRangeDataPoint;
              ReturnedResultInfo.m_found = true;
              ReturnedResultInfo.m_foundPeripheral = true;
              ReturnedResultInfo.m_currentIterationHint = searchIndex - 1;
              ReturnedResultInfo.m_nextIterationHint = searchIndex;
            }
            else
            {
              // Return a point in range.
              ReturnedResultInfo.m_DataPoint = DataPoint;
              ReturnedResultInfo.m_found = true;
              ReturnedResultInfo.m_currentIterationHint = searchIndex;
              ReturnedResultInfo.m_nextIterationHint = searchIndex + increment;
            }

            break;
          }
          else
          {
            searchIndex += increment;
          
            if(GetInfo.m_xMaxPeripheral)
            {
              if(rangeArea == 1)
              {
                // Value was too high.

                if(previousRangeArea == -1 && 
                 GetInfo.m_xMinPeripheral && previousOutOfRangePointValid &&
                 !previousPointFoundWasMinPeripheral)
                {
                  // There we no points in range (we went from too low to too
                  // high). 

                  // Return a minimal peripheral point.
                  ReturnedResultInfo.m_DataPoint = PreviousOutOfRangeDataPoint;
                  ReturnedResultInfo.m_found = true;
                  ReturnedResultInfo.m_foundPeripheral = true;
                  ReturnedResultInfo.m_currentIterationHint = searchIndex;
                  ReturnedResultInfo.m_nextIterationHint = searchIndex + 1;
                }
                else
                {
                  // Return a maximal peripheral point.
                  ReturnedResultInfo.m_DataPoint = DataPoint;
                  ReturnedResultInfo.m_found = true;
                  ReturnedResultInfo.m_foundPeripheral = true;
                  ReturnedResultInfo.m_currentIterationHint = searchIndex;
                  ReturnedResultInfo.m_nextIterationHint = searchIndex + increment;
                }

                break;
              }

              if(GetInfo.m_xMinPeripheral)
              {
                PreviousOutOfRangeDataPoint = DataPoint;
                previousOutOfRangePointValid = true;
              }
            }

            previousRangeArea = rangeArea;
          }
        }
      }
    }
  }
}


i selected the code inside the while loop and did a refactor. and got:

void
CDataServer::GetDatasetPointData(WORD datasetId, 
 const CGetGraphPointDataInfo& GetInfo, 
 CGetGraphPointDataResultInfo& ReturnedResultInfo)const
{
  CGraphDataServerAutoLock AutoLock(this, datasetId);
  {
    CPointDouble PreviousInRangeDataPoint(0, 0);
    bool previousInRangePointValid = false;

    if(GetInfo.m_xMaxPeripheral && ReturnedResultInfo.m_found
     && !ReturnedResultInfo.m_foundPeripheral)
    {
      PreviousInRangeDataPoint = ReturnedResultInfo.m_DataPoint;
      previousInRangePointValid = true;
    }

    bool previousPointFoundWasMinPeripheral = false;
    {
      if(GetInfo.m_xMinPeripheral && ReturnedResultInfo.m_found
       && ReturnedResultInfo.m_foundPeripheral)
      {
        previousPointFoundWasMinPeripheral = true;
      }
    }

    ReturnedResultInfo.m_found = false;
    ReturnedResultInfo.m_foundPeripheral = false;

    const CDataset* Dataset = GetDatasetById(datasetId);

    if(Dataset)
    {
      int searchIndex = 0;
      const int increment = 1;
      {
        if(Dataset->IsValidIndex(GetInfo.m_nextIterationHint))
        {
          // Start searching at this index.
          searchIndex = GetInfo.m_nextIterationHint;
        }
      }

      CPointDouble PreviousOutOfRangeDataPoint(0, 0);
      bool previousOutOfRangePointValid = false;
      int previousRangeArea = -2;

      while(Dataset->IsValidIndex(searchIndex))
      {
        Refactored(Dataset, searchIndex, GetInfo, increment, previousOutOfRangePointValid, ReturnedResultInfo, PreviousOutOfRangeDataPoint, previousRangeArea, previousPointFoundWasMinPeripheral);
        return;

      }
    }
  }
}


and in the header:


void Refactored( const CDataset* Dataset, int &searchIndex, const CGetGraphPointDataInfo &GetInfo, const int increment, bool &previousOutOfRangePointValid, CGetGraphPointDataResultInfo &ReturnedResultInfo, CPointDouble &PreviousOutOfRangeDataPoint, int &previousRangeArea, bool previousPointFoundWasMinPeripheral )
    {
      CPointDouble DataPoint(0, 0);
      Dataset->GetDataPoint(searchIndex, DataPoint);

      if(!GetInfo.IsYValueInRange(DataPoint.y))
      {
        searchIndex += increment;
      }
      else
      {
        int rangeArea = GetInfo.GetXRangeArea(DataPoint.x);
        if(rangeArea == 0)
        {
          // Got it.

          if(GetInfo.m_xMinPeripheral && previousOutOfRangePointValid)
          {
            // But return the peripheral point.
            ReturnedResultInfo.m_DataPoint = PreviousOutOfRangeDataPoint;
            ReturnedResultInfo.m_found = true;
            ReturnedResultInfo.m_foundPeripheral = true;
            ReturnedResultInfo.m_currentIterationHint = searchIndex - 1;
            ReturnedResultInfo.m_nextIterationHint = searchIndex;
          }
          else
          {
            // Return a point in range.
            ReturnedResultInfo.m_DataPoint = DataPoint;
            ReturnedResultInfo.m_found = true;
            ReturnedResultInfo.m_currentIterationHint = searchIndex;
            ReturnedResultInfo.m_nextIterationHint = searchIndex + increment;
          }

          break;
        }
        else
        {
          searchIndex += increment;

          if(GetInfo.m_xMaxPeripheral)
          {
            if(rangeArea == 1)
            {
              // Value was too high.

              if(previousRangeArea == -1 && 
                GetInfo.m_xMinPeripheral && previousOutOfRangePointValid &&
                !previousPointFoundWasMinPeripheral)
              {
                // There we no points in range (we went from too low to too
                // high). 

                // Return a minimal peripheral point.
                ReturnedResultInfo.m_DataPoint = PreviousOutOfRangeDataPoint;
                ReturnedResultInfo.m_found = true;
                ReturnedResultInfo.m_foundPeripheral = true;
                ReturnedResultInfo.m_currentIterationHint = searchIndex;
                ReturnedResultInfo.m_nextIterationHint = searchIndex + 1;
              }
              else
              {
                // Return a maximal peripheral point.
                ReturnedResultInfo.m_DataPoint = DataPoint;
                ReturnedResultInfo.m_found = true;
                ReturnedResultInfo.m_foundPeripheral = true;
                ReturnedResultInfo.m_currentIterationHint = searchIndex;
                ReturnedResultInfo.m_nextIterationHint = searchIndex + increment;
              }

              break;
            }

            if(GetInfo.m_xMinPeripheral)
            {
              PreviousOutOfRangeDataPoint = DataPoint;
              previousOutOfRangePointValid = true;
            }
          }

          previousRangeArea = rangeArea;
        }
      }
    }


problems:

1. There's an added "return" in the while loop. Where did that come from?
2. The new function needs to be const, otherwise it won't compile.

cheers,

Matus

swinefeaster
Tomato Guru

310 Posts

Posted - Nov 03 2006 :  11:12:36 AM  Show Profile  Reply with Quote
forgot to mention this is 1539 on windows xp visual studio 2005.
Go to Top of Page

feline
Whole Tomato Software

United Kingdom
18943 Posts

Posted - Nov 03 2006 :  1:11:26 PM  Show Profile  Reply with Quote
I am seeing the same thing here, thank you for the clear example! I suspect the odds of reproducing this without the code are rather low

I am currently trying to find the smallest possible function that produces this stray return statement, which is taking a little while

zen is the art of being at one with the two'ness
Go to Top of Page

feline
Whole Tomato Software

United Kingdom
18943 Posts

Posted - Nov 03 2006 :  2:18:22 PM  Show Profile  Reply with Quote
Found it. In the comment // But return the peripheral point. change "return" to "Return" to fix this problem. Rather unexpected, but there we go.

case=3429

the extract function not being const is

case=3430

zen is the art of being at one with the two'ness
Go to Top of Page

swinefeaster
Tomato Guru

310 Posts

Posted - Nov 08 2006 :  6:56:44 PM  Show Profile  Reply with Quote
WOW good job :) :) thanks
Go to Top of Page

support
Whole Tomato Software

5566 Posts

Posted - Nov 24 2006 :  9:24:34 PM  Show Profile  Reply with Quote
Case 3429 is fixed in 1541.
Go to Top of Page

support
Whole Tomato Software

5566 Posts

Posted - Sep 10 2009 :  3:16:19 PM  Show Profile  Reply with Quote
case=3430 is fixed in build 1734

Whole Tomato Software, Inc.
Go to Top of Page
  Previous Topic Topic Next Topic  
 New Topic  Reply to Topic
 Printer Friendly
Jump To:
© 2023 Whole Tomato Software, LLC Go To Top Of Page
Snitz Forums 2000