Whole Tomato Software Forums

Whole Tomato Software Forums

All Forums

Visual Assist

Technical Support

Refactoring Issues

You must be registered to post a reply.
Click here to register.

Screensize:

UserName:

Password:

Format:

Message:

Forum code is on.
Html is off.

[quote][i]Originally posted by swinefeaster[/i]
[br]original function:

[code]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;
          }
        }
      }
    }
  }
}[/code]

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

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

const CDataset* Dataset = GetDatasetById(datasetId);

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;

}
    }
  }
}
[/code]

and in the header:

[code]
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;
        }
      }
    }
[/code]

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

[/quote]

Check to include your profile signature.
Check to subscribe to this topic.

T O P I C R E V I E W
swinefeaster	Posted - Nov 03 2006 : 11:11:31 AM 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
6 L A T E S T R E P L I E S (Newest First)
support	Posted - Sep 10 2009 : 3:16:19 PM case=3430 is fixed in build 1734
support	Posted - Nov 24 2006 : 9:24:34 PM Case 3429 is fixed in 1541.
swinefeaster	Posted - Nov 08 2006 : 6:56:44 PM WOW good job :) :) thanks
feline	Posted - Nov 03 2006 : 2:18:22 PM 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
feline	Posted - Nov 03 2006 : 1:11:26 PM 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
swinefeaster	Posted - Nov 03 2006 : 11:12:36 AM forgot to mention this is 1539 on windows xp visual studio 2005.

Snitz Forums 2000