Robin:
http://dpaste.dzfl.pl/3df8694eb47d
I think it's better to improve your code iteratively. First
starting with small simple things:
this(size_t rows, size_t cols) nothrow pure {
=>
this(in size_t rows, in size_t cols) nothrow pure {
this(ref const(Dimension) that) nothrow pure {
=>
this(const ref Dimension that) nothrow pure {
ref Dimension opAssign(ref Dimension rhs) nothrow pure {
Is this method necessary?
bool opEquals(ref const(Dimension) other) nothrow const {
Not necessary.
size_t min() @property nothrow const {
if (this.rows <= this.cols) return this.rows;
else return this.cols;
}
=> (I have changed its name to reduce my confusion
std.algorithm.min):
@property size_t minSide() const pure nothrow {
return (rows <= cols) ? rows : cols;
}
size_t size() @property nothrow const {
=>
@property size_t size() @property pure nothrow {
size_t offset(size_t row, size_t col) nothrow const {
=>
size_t offset(in size_t row, in size_t col) const pure nothrow {
this(in size_t rows, in size_t cols, bool initialize = true)
nothrow pure {
this.dim = Dimension(rows, cols);
this.data =
minimallyInitializedArray!(typeof(this.data))(rows * cols);
if (initialize) this.data[] = to!T(0);
}
=>
this(in size_t rows, in size_t cols, in bool initialize =
true) nothrow pure {
...
if (initialize)
this.data[] = 0;
/**
*
*/
Sometimes I prefer to use just one line of code to reduce
vertical wasted space:
/// Move constructor. From rvalue.
this(ref const(Matrix) that) {
=>
this(const ref Matrix that) pure {
this.data = that.data.dup;
Currently array.dup is not nothrow. This will eventually be fixed.
ref Matrix transpose() const {
return Matrix(this).transposeAssign();
=>
ref Matrix transpose() const pure nothrow {
return Matrix(this).transposeAssign;
ref Matrix transposeAssign() nothrow {
size_t r, c;
for (r = 0; r < this.dim.rows; ++r) {
for (c = 0; c < r; ++c) {
std.algorithm.swap(
this.data[this.dim.offset(r, c)],
this.data[this.dim.offset(c, r)]);
}
}
return this;
}
Use immutable foreach loops.
And even when you use for loops always define the loop variable
inside the for: "for (size_t r = 0; ...".
Also in general I suggest you to write a little less code.
A first try (not tested), but probably this can be done with much
less multiplications:
ref Matrix transposeAssign() pure nothrow {
foreach (immutable r; 0 .. dim.rows) {
foreach (immutable c; 0 .. r) {
swap(data[dim.offset(r, c)], data[dim.offset(c,
r)]);
}
}
return this;
}
Matrix opMul(ref const(Matrix) other)
Never use old-style operator overloading in new D code. Use only
the new operator overloading style.
auto s = Matrix(other).transposeAssign();
=>
auto s = Matrix(other).transposeAssign;
size_t r1, r2, c;
Ditto.
foreach (ref T element; this.data) {
element *= scalar;
}
Try to use:
data[] *= scalar;
for (auto i = 0; i < this.dim.size; ++i) {
this.data[i] += other.data[i];
}
Try to use:
data[] += other.data[];
for (auto i = 0; i < this.dim.size; ++i) {
this.data[i] -= other.data[i];
}
Try to use:
data[] -= other.data[];
bool opEquals(ref const(Matrix) other) nothrow const {
if (this.dim != other.dim) {
return false;
}
for (auto i = 0; i < this.dim.size; ++i) {
if (this.data[i] != other.data[i]) return false;
}
return true;
}
Try (in general try to write less code):
return dim == other.dim && data == other.data;
auto stringBuilder = std.array.appender!string;
=>
Appender!string result;
stringBuilder.put("Matrix: "
~ to!string(this.dim.rows) ~ " x " ~
to!string(this.dim.cols)
=>
result ~= text("Matrix: ", dim.rows, " x ", dim.cols
auto m = Matrix(rows, cols);
for (auto i = 0; i < m.dim.min; ++i) {
m[i, i] = 1;
}
See the comment by John Colvin.
auto generator = Random(unpredictableSeed);
What if I want to fill the matrix deterministically, with a given
seed? In general it's better to not add a random matrix method to
your matrix struct. Better to add an external free function that
uses UFCS and is more flexible and simpler to reason about.
writeln("\tTime required: " ~ to!string(secs) ~ " secs, " ~
to!string(msecs) ~ " msecs");
Use .text to strinify, but here you don't need to stringify at
all:
=>
writeln("\tTime required: ", secs, " secs, ", msecs, "
msecs");
sw.reset();
=>
sw.reset;
void multiplicationTest() {
In D there is also unittest {}. A benchmark is not exactly an
unit test, so do as you prefer.
Bye,
bearophile
