Skip to content

Commit

Permalink
Change Index and TextPos to use enum constructors for extents
Browse files Browse the repository at this point in the history
The static member variable version was flawed as it suffered from SIOF,
since TextPos' static END made use of Index's static LAST. Often, END
would be initialized before LAST resulting in END being invalid/garbage.

This approach, while slightly less terse, prevents that issue.

The only other option is to use static member functions that return a
type constructed to be that extent, which would enact RAII.

The interface would look like: Index32::last();
instead of Index32(Index32::Last).
  • Loading branch information
oblivioncth committed Aug 2, 2023
1 parent a490904 commit 695b08c
Show file tree
Hide file tree
Showing 9 changed files with 111 additions and 75 deletions.
54 changes: 30 additions & 24 deletions lib/core/include/qx/core/qx-index.h
Original file line number Diff line number Diff line change
Expand Up @@ -22,32 +22,46 @@ class Index
//-Class Types----------------------------------------------------------------------------------------------------
private:
enum class Type {Null, End, Value};
struct LastKey {};

//-Class Members----------------------------------------------------------------------------------------------------
public:
static const Index<T> FIRST;
static const Index<T> LAST;
enum Extent
{
First,
Last
};

//-Instance Members----------------------------------------------------------------------------------------------------
private:
Type mType;
T mValue;

//-Constructor----------------------------------------------------------------------------------------------
private:
Index(LastKey) :
mType(Type::End),
mValue(std::numeric_limits<T>::max())
{}

public:
Index() :
constexpr Index() :
mType(Type::Null),
mValue(0)
{}

Index(T value) :
constexpr Index(Extent e)
{
switch(e)
{
case First:
mType = Type::Value;
mValue = 0;
break;

case Last:
mType = Type::End;
mValue = std::numeric_limits<T>::max();
break;

default:
qCritical("Invalid extent");
}
}

constexpr Index(T value) :
mType(Type::Value),
mValue(value)
{
Expand Down Expand Up @@ -93,7 +107,7 @@ class Index
if(other.mType == Type::End)
return 0;
else if(mType == Type::End)
return LAST;
return Index(Last);
else
return constrainedSub(mValue, other.mValue, 0);
}
Expand All @@ -103,7 +117,7 @@ class Index
Index operator+(const Index& other)
{
return (mType == Type::End || other.mType == Type::End) ?
LAST : constrainedAdd(mValue, other.mValue, 0);
Index(Last) : constrainedAdd(mValue, other.mValue, 0);
}


Expand All @@ -117,7 +131,7 @@ class Index
if(other.mType == Type::End)
return mType == Type::End ? 1 : 0;
else if(mType == Type::End)
return LAST;
return Index(Last);
else
return constrainedDiv(mValue, other.mValue, 0);
}
Expand All @@ -129,7 +143,7 @@ class Index
if(mValue == 0 || other.mValue == 0)
return 0;
else if(mType == Type::End || other.mType == Type::End)
return LAST;
return Index(Last);
else
return constrainedMult(mValue, other.mValue, 0);

Expand Down Expand Up @@ -289,14 +303,6 @@ class Index
// friend T& operator*=(N& integer, const Index<T>& index) { integer *= static_cast<N>(index.mValue); return integer; }
};

//-Outer Class Definitions----------------------------------------------------------------------------------
template<typename T>
requires std::signed_integral<T>
const Index<T> Index<T>::FIRST = Index<T>(0);
template<typename T>
requires std::signed_integral<T>
const Index<T> Index<T>::LAST = Index<T>(LastKey{});

//-Outer Class Types----------------------------------------------------------------------------------------
typedef Index<qint8> Index8;
typedef Index<qint16> Index16;
Expand Down
34 changes: 15 additions & 19 deletions lib/core/src/qx-index.dox
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ namespace Qx
* use -1 to mean 'none' in the context of searches.
*
* The Index type is designed to remedy this issue by providing a more distinct representation of a set's final index
* via the member variable Index<T>::LAST, which for completeness is paired with Index<T>::FIRST. The type also
* via the constructor Index(Extent), which can also be used to more explicitly reference the first index. The type also
* can be null, providing further clarity for the 'none' case.
*
* To minimize unintentional interactions, Index<T> only has comparison and arithmetic operators defined for when
Expand Down Expand Up @@ -52,20 +52,18 @@ namespace Qx
* @li 2) 'last' indices are greater than everything except other 'last' indices (highest possible value)
*/

//-Class Members----------------------------------------------------------------------------------------------------
//-Class Types----------------------------------------------------------------------------------------------------
//Public:
/*!
* @var Index<T> Index<T>::FIRST
* @enum Index<T>::Extent
*
* Used to refer to the first index of a dataset.
* Used to refer to indicies of special significance.
*
* This is equivalent to @c Index<T>(0).
*/

/*!
* @var Index<T> Index<T>::LAST
* @var Index<T>::Extent Index<T>::First
* The first index, equivalent to @c Index<T>(0).
*
* Used to refer to the last index of a dataset.
* @var Index<T>::Extent Index<T>::Last
* The last index.
*/

//-Constructor----------------------------------------------------------------------------------------------
Expand All @@ -76,6 +74,12 @@ namespace Qx
* Creates an null index.
*/

/*!
* @fn Index<T>::Index(Extent e)
*
* Creates an index at the given extent @a e.
*/

/*!
* @fn Index<T>::Index(T value)
*
Expand All @@ -97,15 +101,7 @@ namespace Qx
*
* Returns @c true if the index represents the last index of an arbitrary dataset; otherwise returns @c false.
*
* This is equivalent to <tt>*this == Index<T>::LAST</tt>.
*/

/*!
* @fn bool Index<T>::isLast() const
*
* Returns @c true if the index represents the last index of an arbitrary dataset; otherwise returns @c false.
*
* This is equivalent to <tt>*this == Index<T>::LAST</tt>.
* This is equivalent to <tt>*this == Index<T>(Last)</tt>.
*/

//-Operators-------------------------------------------------------------------------------------------------------
Expand Down
10 changes: 5 additions & 5 deletions lib/io/include/qx/io/qx-common-io.h
Original file line number Diff line number Diff line change
Expand Up @@ -62,10 +62,10 @@ QX_IO_EXPORT IoOpReport textFileAbsolutePosition(TextPos& textPos, QFile& textFi
QX_IO_EXPORT IoOpReport findStringInFile(QList<TextPos>& returnBuffer, QFile& textFile, const TextQuery& query, ReadOptions readOptions = NoReadOptions);
QX_IO_EXPORT IoOpReport fileContainsString(bool& returnBuffer, QFile& textFile, const QString& query, Qt::CaseSensitivity caseSensitivity = Qt::CaseSensitive, bool allowSplit = false);
QX_IO_EXPORT IoOpReport readTextFromFile(QString& returnBuffer, QFile& textFile, TextPos startPos, int count, ReadOptions readOptions = NoReadOptions);
QX_IO_EXPORT IoOpReport readTextFromFile(QString& returnBuffer, QFile& textFile, TextPos startPos = TextPos::START, TextPos endPos = TextPos::END, ReadOptions readOptions = NoReadOptions);
QX_IO_EXPORT IoOpReport readTextFromFile(QStringList& returnBuffer, QFile& textFile, Index32 startLine = 0, Index32 endLine = Index32::LAST, ReadOptions readOptions = NoReadOptions);
QX_IO_EXPORT IoOpReport writeStringToFile(QFile& textFile, const QString& text, WriteMode writeMode = Truncate, TextPos startPos = TextPos::START, WriteOptions writeOptions = NoWriteOptions);
QX_IO_EXPORT IoOpReport writeStringToFile(QSaveFile& textFile, const QString& text, WriteMode writeMode = Truncate, TextPos startPos = TextPos::START, WriteOptions writeOptions = NoWriteOptions);
QX_IO_EXPORT IoOpReport readTextFromFile(QString& returnBuffer, QFile& textFile, TextPos startPos = TextPos(TextPos::Start), TextPos endPos = TextPos(TextPos::End), ReadOptions readOptions = NoReadOptions);
QX_IO_EXPORT IoOpReport readTextFromFile(QStringList& returnBuffer, QFile& textFile, Index32 startLine = 0, Index32 endLine = Index32(Index32::Last), ReadOptions readOptions = NoReadOptions);
QX_IO_EXPORT IoOpReport writeStringToFile(QFile& textFile, const QString& text, WriteMode writeMode = Truncate, TextPos startPos = TextPos(TextPos::Start), WriteOptions writeOptions = NoWriteOptions);
QX_IO_EXPORT IoOpReport writeStringToFile(QSaveFile& textFile, const QString& text, WriteMode writeMode = Truncate, TextPos startPos = TextPos(TextPos::Start), WriteOptions writeOptions = NoWriteOptions);
QX_IO_EXPORT IoOpReport deleteTextFromFile(QFile& textFile, TextPos startPos, TextPos endPos);

// Directory:
Expand All @@ -81,7 +81,7 @@ QX_IO_EXPORT IoOpReport calculateFileChecksum(QString& returnBuffer, QFile& file
QX_IO_EXPORT IoOpReport fileMatchesChecksum(bool& returnBuffer, QFile& file, QString checksum, QCryptographicHash::Algorithm hashAlgorithm);

// Binary Based
QX_IO_EXPORT IoOpReport readBytesFromFile(QByteArray& returnBuffer, QFile& file, Index64 startPos = 0, Index64 endPos = Index64::LAST);
QX_IO_EXPORT IoOpReport readBytesFromFile(QByteArray& returnBuffer, QFile& file, Index64 startPos = 0, Index64 endPos = Index64(Index64::Last));
QX_IO_EXPORT IoOpReport writeBytesToFile(QFile& file, const QByteArray& bytes, WriteMode writeMode = Truncate, Index64 startPos = 0, WriteOptions writeOptions = NoWriteOptions);
QX_IO_EXPORT IoOpReport writeBytesToFile(QSaveFile& file, const QByteArray& bytes, WriteMode writeMode = Truncate, Index64 startPos = 0, WriteOptions writeOptions = NoWriteOptions);
}
Expand Down
12 changes: 10 additions & 2 deletions lib/io/include/qx/io/qx-textpos.h
Original file line number Diff line number Diff line change
Expand Up @@ -7,15 +7,22 @@
// Extra-component Includes
#include "qx/core/qx-index.h"

// TODO: This class should probably be constexpr construable, though this
// would mean it cant have separate implementation files which is somewhat ugly.
// probably should reconsider this once using modules.

namespace Qx
{

class QX_IO_EXPORT TextPos
{
//-Class Types------------------------------------------------------------------------------------------------------
public:
static const TextPos START;
static const TextPos END;
enum Extent
{
Start,
End
};

//-Instance Variables------------------------------------------------------------------------------------------------
private:
Expand All @@ -25,6 +32,7 @@ class QX_IO_EXPORT TextPos
//-Constructor-------------------------------------------------------------------------------------------------------
public:
TextPos();
TextPos(Extent e);
TextPos(Index32 line, Index32 character);

//-Instance Functions------------------------------------------------------------------------------------------------
Expand Down
26 changes: 13 additions & 13 deletions lib/io/src/qx-common-io.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -378,9 +378,9 @@ IoOpReport textFileLineCount(int& returnBuffer, QFile& textFile, bool ignoreTrai

/*!
* Converts any relative component of @a textPos to an absolute one. I.e. determines the actual line
* and or/character that Index32::LAST references for the given @a textFile, if present.
* and or/character that Index32(Index32::Last) references for the given @a textFile, if present.
*
* If neither the line or character component of @a textPos contain the value Index32::LAST,
* If neither the line or character component of @a textPos contain the value Index32(Index32::Last),
* @a textPos is left unchanged.
*
* @param[in,out] textPos The text position to translate into an absolute position.
Expand Down Expand Up @@ -459,7 +459,7 @@ IoOpReport findStringInFile(QList<TextPos>& returnBuffer, QFile& textFile, const

// Query tracking
TextPos trueStartPos = query.startPosition();
TextPos currentPos = TextPos::START;
TextPos currentPos = TextPos(TextPos::Start);
TextPos possibleMatch = TextPos();
int hitsSkipped = 0;
QString::const_iterator queryIt = query.string().constBegin();
Expand All @@ -469,7 +469,7 @@ IoOpReport findStringInFile(QList<TextPos>& returnBuffer, QFile& textFile, const
QTextStream fileTextStream(&textFile);

// Translate start position to absolute position
if(trueStartPos != TextPos::START)
if(trueStartPos != TextPos(TextPos::Start))
{
IoOpReport translate = textFileAbsolutePosition(trueStartPos, textFile, readOptions.testFlag(IgnoreTrailingBreak));
if(translate.isFailure())
Expand All @@ -489,7 +489,7 @@ IoOpReport findStringInFile(QList<TextPos>& returnBuffer, QFile& textFile, const
QScopeGuard fileGuard([&textFile](){ textFile.close(); });

// Skip to start pos
if(trueStartPos != TextPos::START)
if(trueStartPos != TextPos(TextPos::Start))
{
int line;
// Skip to start line
Expand Down Expand Up @@ -739,7 +739,7 @@ IoOpReport readTextFromFile(QString& returnBuffer, QFile& textFile, TextPos star
TextStream fileTextStream(&textFile);

// Cover each possible range type
if(startPos == TextPos::START && endPos == TextPos::END) // Whole file is desired
if(startPos == TextPos(TextPos::Start) && endPos == TextPos(TextPos::End)) // Whole file is desired
{
returnBuffer = fileTextStream.readAll();

Expand Down Expand Up @@ -999,7 +999,7 @@ namespace

// Fill beforeNew
TextPos beforeEnd = TextPos(startPos.line(), startPos.character() - 1);
IoOpReport readBefore = readTextFromFile(beforeNew, auxFile, TextPos::START, beforeEnd);
IoOpReport readBefore = readTextFromFile(beforeNew, auxFile, TextPos(TextPos::Start), beforeEnd);
if(readBefore.isFailure())
return readBefore;

Expand Down Expand Up @@ -1178,27 +1178,27 @@ IoOpReport deleteTextFromFile(QFile& textFile, TextPos startPos, TextPos endPos)
IoOpReport transientReport;

// Determine beforeDeletion
if(startPos == TextPos::START) // (0,0)
if(startPos == TextPos(TextPos::Start)) // (0,0)
beforeDeletion = u""_s;
else if(startPos.character().isLast())
{
transientReport = readTextFromFile(beforeDeletion, textFile, TextPos::START, startPos);
transientReport = readTextFromFile(beforeDeletion, textFile, TextPos(TextPos::Start), startPos);
beforeDeletion.chop(1);
}
else
transientReport = readTextFromFile(beforeDeletion, textFile, TextPos::START, TextPos(startPos.line(), startPos.character() - 1));
transientReport = readTextFromFile(beforeDeletion, textFile, TextPos(TextPos::Start), TextPos(startPos.line(), startPos.character() - 1));

// Check for transient errors
if(!transientReport.isNull() && transientReport.result() != IO_SUCCESS)
return IoOpReport(IO_OP_WRITE, transientReport.result(), textFile);

// Determine afterDeletion
if(endPos == TextPos::END)
if(endPos == TextPos(TextPos::End))
afterDeletion = u""_s;
else if(endPos.character().isLast())
transientReport = readTextFromFile(afterDeletion, textFile, TextPos(endPos.line() + 1, 0), TextPos::END);
transientReport = readTextFromFile(afterDeletion, textFile, TextPos(endPos.line() + 1, 0), TextPos(TextPos::End));
else
transientReport = readTextFromFile(afterDeletion, textFile, TextPos(endPos.line(), endPos.character() + 1), TextPos::END);
transientReport = readTextFromFile(afterDeletion, textFile, TextPos(endPos.line(), endPos.character() + 1), TextPos(TextPos::End));

// Check for transient errors
if(transientReport.result() != IO_SUCCESS)
Expand Down
4 changes: 2 additions & 2 deletions lib/io/src/qx-common-io_p.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -132,10 +132,10 @@ IoOpReport writePrep(const QFileInfo& fileInfo, WriteOptions writeOptions)
void matchAppendConditionParams(WriteMode& writeMode, TextPos& startPos)
{
// Match append condition parameters
if(startPos == TextPos::END)
if(startPos == TextPos(TextPos::End))
writeMode = Append;
else if(writeMode == Append)
startPos = TextPos::END;
startPos = TextPos(TextPos::End);
}

/*! @endcond */
Expand Down
2 changes: 1 addition & 1 deletion lib/io/src/qx-common-io_p.h
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ void matchAppendConditionParams(WriteMode& writeMode, Index<T>& startPos)
if(startPos.isLast())
writeMode = Append;
else if(writeMode == Append)
startPos = Index<T>::LAST;
startPos = Index<T>(Index<T>::Last);
}
/*! @endcond */
}
Expand Down
Loading

0 comments on commit 695b08c

Please sign in to comment.