StringRangeEnumerator cleanup

* do not allow default unbounded min and max, syntax of the input string
requires them
* simplify logic, remove unused methods, make the class immutable, actualize
documentation

2 files changed, 29 insertions, 55 deletions

diff --git a/tools/inc/tools/multisel.hxx b/tools/inc/tools/multisel.hxx
index a979c23b5f6e..8e4879f1c709 100644
--- a/tools/inc/tools/multisel.hxx
+++ b/tools/inc/tools/multisel.hxx
@@ -122,7 +122,9 @@ class TOOLS_DLLPUBLIC StringRangeEnumerator
     sal_Int32                                              mnMin;
     sal_Int32                                              mnMax;
     sal_Int32                                              mnOffset;
+    bool                                                   mbValidInput;
 
+    bool setRange( const rtl::OUString& i_rNewRange, bool i_bStrict = false );
     bool insertRange( sal_Int32 nFirst, sal_Int32 nLast, bool bSequence, bool bMayAdjust );
     bool insertJoinedRanges( const std::vector< sal_Int32 >& rNumbers, bool i_bStrict );
     bool checkValue( sal_Int32, const std::set< sal_Int32 >* i_pPossibleValues = NULL ) const;
@@ -152,10 +154,9 @@ public:
 
     friend class StringRangeEnumerator::Iterator;
 
-    StringRangeEnumerator() : mnCount( 0 ), mnMin( -1 ), mnMax( -1 ), mnOffset( -1 ) {}
     StringRangeEnumerator( const rtl::OUString& i_rInput,
-                           sal_Int32 i_nMinNumber = -1,
-                           sal_Int32 i_nMaxNumber = -1,
+                           sal_Int32 i_nMinNumber,
+                           sal_Int32 i_nMaxNumber,
                            sal_Int32 i_nLogicalOffset = -1
                            );
 
@@ -163,14 +164,7 @@ public:
     Iterator begin( const std::set< sal_Int32 >* i_pPossibleValues = NULL ) const;
     Iterator end( const std::set< sal_Int32 >* i_pPossibleValues = NULL ) const;
 
-    sal_Int32 getMin() const { return mnMin; }
-    void setMin( sal_Int32 i_nMinValue ) { mnMin = i_nMinValue; }
-    sal_Int32 getMax() const { return mnMax; }
-    void setMax( sal_Int32 i_nMaxValue ) { mnMax = i_nMaxValue; }
-    sal_Int32 getLogicalOffset() const { return mnOffset; }
-    void setLogicalOffset( sal_Int32 i_nOffset ) { mnOffset = i_nOffset; }
-
-    bool setRange( const rtl::OUString& i_rNewRange, bool i_bStrict = false );
+    bool isValidInput() const { return mbValidInput; }
     bool hasValue( sal_Int32 nValue, const std::set< sal_Int32 >* i_pPossibleValues = NULL ) const;
 
 
@@ -182,28 +176,28 @@ public:
                       example: a user enters page numbers from 1 to n (since that is logical)
                                of course usable page numbers in code would start from 0 and end at n-1
                                so the logical offset would be -1
-    i_nMinNumber:     the minimum allowed number, a negative number means no minimum check
-    i_nMaxNumber:     the maximum allowed number, a negative number means no maximum check
+    i_nMinNumber:     the minimum allowed number
+    i_nMaxNumber:     the maximum allowed number
 
     @returns: true if the input string was valid, o_rPageVector will contain the resulting sequence
-              false if the input string was invalid, o_rPageVector will be unchanged
+              false if the input string was invalid, o_rPageVector will contain
+                    the sequence that parser is able to extract
 
     behavior:
     - only non-negative sequence numbers are allowed
     - only non-negative values in the input string are allowed
-    - the string "-3" will be either
-      * an error if no minimum is given
-      * or result in the sequence i_nMinNumber to 3
-    - the string "3-" will be either
-      * an error if no maximum is given
-      * or result in the seqeuence 3 to i_nMaxNumber
+    - the string "-3" means the sequence i_nMinNumber to 3
+    - the string "3-" means the sequence 3 to i_nMaxNumber
+    - the string "-" means the sequence i_nMinNumber to i_nMaxNumber
+    - single number that doesn't fit in [i_nMinNumber,i_nMaxNumber] will be ignored
+    - range that doesn't fit in [i_nMinNumber,i_nMaxNumber] will be adjusted
     - an empty string as input is valid and will result in the range [min,max] if given
       or an empty vector, if not
     */
     static bool getRangesFromString( const rtl::OUString& i_rPageRange,
                                      std::vector< sal_Int32 >& o_rPageVector,
-                                     sal_Int32 i_nMinNumber = -1,
-                                     sal_Int32 i_nMaxNumber = -1,
+                                     sal_Int32 i_nMinNumber,
+                                     sal_Int32 i_nMaxNumber,
                                      sal_Int32 i_nLogicalOffset = -1,
                                      std::set< sal_Int32 >* i_pPossibleValues = NULL
                                     );
diff --git a/tools/source/memtools/multisel.cxx b/tools/source/memtools/multisel.cxx
index 4d6d577269c7..13275a863673 100644
--- a/tools/source/memtools/multisel.cxx
+++ b/tools/source/memtools/multisel.cxx
@@ -738,17 +738,16 @@ StringRangeEnumerator::StringRangeEnumerator( const rtl::OUString& i_rInput,
     , mnMin( i_nMinNumber )
     , mnMax( i_nMaxNumber )
     , mnOffset( i_nLogicalOffset )
+    , mbValidInput( false )
 {
-    setRange( i_rInput );
+    // Parse string only if boundaries are valid.
+    if( mnMin >= 0 && mnMax >= 0 && mnMin <= mnMax )
+        mbValidInput = setRange( i_rInput );
 }
 
 bool StringRangeEnumerator::checkValue( sal_Int32 i_nValue, const std::set< sal_Int32 >* i_pPossibleValues ) const
 {
-    if( mnMin >= 0 && i_nValue < mnMin )
-        return false;
-    if( mnMax >= 0 && i_nValue > mnMax )
-        return false;
-    if( i_nValue < 0 )
+    if( i_nValue < 0 || i_nValue < mnMin || i_nValue > mnMax )
         return false;
     if( i_pPossibleValues && i_pPossibleValues->find( i_nValue ) == i_pPossibleValues->end() )
         return false;
@@ -760,10 +759,6 @@ bool StringRangeEnumerator::insertRange( sal_Int32 i_nFirst, sal_Int32 i_nLast,
     bool bSuccess = true;
     if( bSequence )
     {
-        if( i_nFirst == -1 )
-            i_nFirst = mnMin;
-        if( i_nLast == -1 )
-            i_nLast = mnMax;
         if( bMayAdjust )
         {
             if( i_nFirst < mnMin )
@@ -787,25 +782,15 @@ bool StringRangeEnumerator::insertRange( sal_Int32 i_nFirst, sal_Int32 i_nLast,
     }
     else
     {
-        if( i_nFirst >= 0 )
+        if( checkValue( i_nFirst ) )
         {
-            if( checkValue( i_nFirst ) )
-            {
-                maSequence.push_back( Range( i_nFirst, i_nFirst ) );
-                mnCount++;
-            }
-            else
-                bSuccess = false;
+            maSequence.push_back( Range( i_nFirst, i_nFirst ) );
+            mnCount++;
         }
-        else if( i_nLast >= 0 )
+        else if( checkValue( i_nLast ) )
         {
-            if( checkValue( i_nLast ) )
-            {
-                maSequence.push_back( Range( i_nLast, i_nLast ) );
-                mnCount++;
-            }
-            else
-                bSuccess = false;
+            maSequence.push_back( Range( i_nLast, i_nLast ) );
+            mnCount++;
         }
         else
             bSuccess = false;
@@ -1002,12 +987,7 @@ bool StringRangeEnumerator::getRangesFromString( const OUString& i_rPageRange,
 {
     o_rPageVector.clear();
 
-    StringRangeEnumerator aEnum;
-    aEnum.setMin( i_nMinNumber );
-    aEnum.setMax( i_nMaxNumber );
-    aEnum.setLogicalOffset( i_nLogicalOffset );
-
-    bool bRes = aEnum.setRange( i_rPageRange );
+    StringRangeEnumerator aEnum( i_rPageRange, i_nMinNumber, i_nMaxNumber, i_nLogicalOffset ) ;
 
     //Even if the input range wasn't completely valid, return what ranges could
     //be extracted from the input.
@@ -1018,7 +998,7 @@ bool StringRangeEnumerator::getRangesFromString( const OUString& i_rPageRange,
         o_rPageVector.push_back( *it );
     }
 
-    return bRes;
+    return aEnum.isValidInput();
 }
 
 /* vim:set shiftwidth=4 softtabstop=4 expandtab: */