dusktreader 137 Posting Whiz in Training

Thanks dusktreader for answering :)
Actually, I looked at your code while reading the line "it keeps the code from executing a switch on every single iteration of the loop" and therefore got confused.

The switch statement is really just a convenience wrapper for a regular conditional (if). A for loop already has a conditional that is executed in every iteration. Usually this is a check to ensure that the index hasn't exceeded the limit of the loop: for( int i=0; i<n; i++ ){...} If you can help it, you don't want there to be another conditional inside of the for loop, otherwise, the program has to check it on every iteration.
So, this:

for( ){
  if( C == A )
      someOperation();
  if( C == B )
      someOtherOperation();
}

Has to check the value of C on every iteration of the loop. Though it doesn't look as clean, it would be more efficient to write:

if( C == A )
    for()
        someOperation();
if( C == B )
    for()
        someOtherOperation();

This way, the test on the value of C is only performed once. This is a small optimization, but in a large project, simple boosts can gain a lot!

Mat[][] is a private member of class Matrix object other, can it be directly accessed? I don't think so.

Actually, it can directly access the private members of another class of the same type. Try this code out:

#include <iostream>
using namespace std;
class Dummy{
private: …
dusktreader 137 Posting Whiz in Training

What's the compiler error look like?

dusktreader 137 Posting Whiz in Training

So I'm working on this class, which is basically to wrap a char into 8bools, and it works so far, however I'm running into a problem, in the dedication of values, since I'm to use the overloaded array operator, as well as the overloaded assignment operator, at once, however I can't really find the way to do this; this is my code:

Main.cpp

...
    CharBool = alfa; // Missing the possiblility to do CharBool[Number] = bool
                     // and the ability to do; CharBool[Number] = true (or false for that matter).

Boolean.cpp

...
bool Boolean::operator[] (char nIndex)
{
if (nIndex>7){/*SENT ERROR MESSEGE*/return 0;}
return ((*Bool >> nIndex) & 1);
}
...

I don't think you'll have much luck getting b=true to work very well. You need to understand what is happening under the hood first. When you call operator[], you should be getting back a reference to some value. There are no values available that are smaller than a sigle byte. A character is a sigle byte. Therefore, you can't possible fetch a reference to the ith bit in a character. The best you could do is get a reference to the whole character. You can't literally assign a value to a single bit. Instead, you have to play tricks like using the bitwise or operator with an l-shfted 1 to set the bit. You could possibly hack this in a certain way. Include some private variable that would keep the index of the last requested bit, then, in the assignment operator, …

dusktreader 137 Posting Whiz in Training

Are you macro protecting your header files?

#ifndef _SOME_HEADER
#define _SOME_HEADER
class SomeClass{...};
#endif

or

#pragma once
class SomeClass{...};
dusktreader 137 Posting Whiz in Training

No need for two loops here:
Given an array (arr), the starint index (j), the ending index (k) and the number of elements in the array (n):

int sum =0;
for( int i=j; i<k; i++ )
    sum += arr[i];

You will need to ensure that i >= 0 and k <= n

[edit]

I noticed that you use the terminating condition i < n-k. Just substitute this into the termination criteria (middle statement) in the for loop

dusktreader 137 Posting Whiz in Training

Any other comments or suggestions? :)

Line 24 (and others): Watch out for shadowing class members with function arguments. There is a function called init, so you should probably make this argument initVal. This is more readable anywya.

Line 34 (and others): Inside of the scope of the Matrix class, you have access to the mat array member. So, you should use direct access instead of accessor functions within this scope. So use mat[j] = other.mat[j]. This is also more readable, and it is more obvious what is going on.

Line 45 (and others): Move this conditional above the switch. There is no use repeating the same code in all the branches.

Line 49 (and others): Move the initialization above the switch. No use repeating.

Line 53 (and others): Move the return statement below the switch. No use repeating.

Line 69: Why is the conditional for the MLT case different? For a per-element multiplication, the constraints should be the same as the other operations. Also, just use other.row since it is within scope. Cohesion and consistency are as important as readability.

Line 134: I'm surprised your compiler signed off on this. The setw function isn't templated ( on my compiler ), so I couldn't include this. Also, would the setw know what to do if the X was a user defined type. If you had a type that was overloaded for +, -, *, and << you could create a Matrix of this type. …

dusktreader 137 Posting Whiz in Training

@dusktreader - Thanks for the code, I'll take hints from that. I thought of using single dimensional array for representing the matrix but then change to two dimensional to support user's view of the matrix.

notice that I overloaded the () operator to allow access to an item by (row, col), so Mat( i, j ) will return the item at the ith row, jth column. You can easily expose 2d functionality ( or higher D if you are careful and clever ) to a user while preserving the underlying 1D structure.

Also, I didn't got what you meant by this "but it keeps the code from executing a switch on every single iteration of the loop" :-/

@first person suggested a helper function that looked like this:

void _arithmeticHelper(const Matrix& other, const int& Operator){
 for i = 0 to Row
   for j = 0 to Col
    switch( Operator){
     case ADD : _myMatrix[i][j] + other._myMatrix[i][j]; break;
     case SUBTRACT : ... break;
     case MULTIPLY : ... break;
    }
}

Notice that in this code, the switch statement is inside of the nested for loop. This means that the program will be required to branch ( execute an if statement ) in every single iteration of the loop.

I suggest this as an alternative:

Matrix arithmeticOperator( const Matrix& other, ops op ) const{
    if( mat == NULL || other.mat == NULL || other.rows != rows || other.cols != cols )
        return Matrix();
    Matrix neo( rows, cols );
    switch( …
dusktreader 137 Posting Whiz in Training

Here is a working example with a few notes at the bottom:

#include <iostream>

using namespace std;

enum ops{ ADD, SUB, MLT };

template <class T>
class Matrix{
private:
    T* mat;
    int rows;
    int cols;
    int sz;

    void initMat( int rows, int cols ){
        if( rows <= 0 || cols <= 0 ){
            mat = NULL;
            return;
        }
        this->rows = rows;
        this->cols = cols;
        sz = rows * cols;
        mat = new T[sz];
    }


public:
    Matrix(){
        mat = NULL;
    }

    Matrix( int rows, int cols ){
        initMat( rows, cols );
    }

    Matrix( int rows, int cols, T initVal ){
        initMat( rows, cols );
        for( int i=0; i<sz; i++ )
            mat[i] = initVal;
    }

    Matrix( const Matrix& other ){
        initMat( other.rows, other.cols );
        memcpy( mat, other.mat, sz * sizeof(T) );
    }

    ~Matrix(){
        delete [] mat;
    }

    T& operator()( int row, int col ) const{
        return mat[ cols * row + col ];
    }

    Matrix operator+( const Matrix& other ) const{
        return arithmeticOperator( other, ADD );
    }

    Matrix operator-( const Matrix& other ) const{
        return arithmeticOperator( other, SUB );
    }

    Matrix operator*( const Matrix& other ) const{
        return arithmeticOperator( other, MLT );
    }

    Matrix arithmeticOperator( const Matrix& other, ops op ) const{
        if( mat == NULL || other.mat == NULL || other.rows != rows || other.cols != cols )
            return Matrix();
        Matrix neo( rows, cols );
        switch( op ){
            case ADD: for( int i=0; i<sz; i++ ) neo.mat[i] =  mat[i] + other.mat[i]; break;
            case SUB: for( int i=0; i<sz; i++ ) …
vidit_X commented: Thanks. +2
dusktreader 137 Posting Whiz in Training

y=f/2; //This will tell you nothing about the evenness of f
if ( y= 0) //This will always resolve to false because the assignment operator essentially returns the rvalue, which is always 0 in this case. Perhaps you meant y==0 ?
cout<<"even";
else
cout<<" odd";
}

This is a good way to avoid the costly mod operator (if this was your intention) while determining if a number is even:

bool isOdd =  result & 1;

This works because all even values have a 0 in the least significant bit, while odds have a 1 in the LSB. The bitwise operator will essentially test the LSB, and return 1 if the LSB is 1, and 0 otherwise. You can see this for yourself with this code

#include <iostream>
using namespace std;
int main(int argc, char *argv[]){
    for( int i=0; i<14; i++ )
        cout << i << " is " << ( i & 1 ? "odd\n" : "even\n" );
    return 0;
}

This could be easily applied to this problem by using this line

cout << "your result is " << ( result & 1 ? "odd\n" : "even\n" );
dusktreader 137 Posting Whiz in Training

I know the solution to the random reordering has already been handled, but here is a solution that I find more mathematically satisfying.

1. Assign each die a random x, y coordinate
a. The coordinates should be real numbers (i.e. doubles )
b. If you prefer integers, then the range of possible values for each dimension should be significantly larger than the corresponding grid dimension and a multiple of the grid dimension.
For a 6x16 grid, use x=rand()%(6*6), y=rand()%(16*16)
2. Sort the dice by their y coordinate *in place*
3. Partition the list into h equally sized groups where h = height of dice grid
For a 6x16 grid, obviously partition into 16 groups of 6 coordinate pairs
4. Sort the dice in each partition *in place*
5. The list should represent a random distribution representing random re-ordering over a fixed size grid
So, for the die in group 4 at slot 2, it should go to Grid location row 4, column2

It may not be more efficient (haven't done the analysis), and it might be more troublesome to implement, but it is an interesting solution, no?