HiveBrain v1.2.0
Get Started
← Back to all entries
patterncppModerate

Randomizing and mutating algo class - style

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
mutatingalgorandomizingstyleandclass

Problem

I've updated this question to be about code style only, as all of it's answered focused on this aspect. For the codes function, see Randomizing and mutating algo class - functionality

algo is an algorithm that's dynamic. algo instances can be created, asked to become random, mutated and also run. They can be set to remember changes they make to themselves between runs or not. They also output values, these could be used for anything from value sequences such as mathematical number sequences to controls for a bot in a game. It's straightforward to specify limits on memory or computation steps for each as well and needless to say are entirely sandboxed.

By sandboxed I mean that they only compute and produce output as described, they cannot for example use local or global variables, print to the console, #include or write to files.

algos can be used where algorithms need to be portable and must be only able to calculate/compute. There is no distinction between data and instructions in an algo.

A use is as values for directed search for algorithms such as with evolutionary algorithms, MCTS or others.

Another is in a data file that includes algorithms, like an image that includes its own way to decompress itself, that can therefore be constructed using the specific image that is to be decompresed.

They are deliberately general, being a component that could be used in many contexts and conceptually simple, as a number is.

Can this code be reviewed?

```
// by Alan Tennant

#include
#include
#include
#include // for rand

class algo
{
public:
std::vector code;
std::vector output;
bool debug_info;

algo()
{
reset1(false);
instructions_p = 11;
}

void random(unsigned int size)
{
code.clear();
output.clear();
for(unsigned int n = 0; n largest_program)
{
cont = false;
can_resume_p = false;
out_of_space_p =

Solution

For starters, code as written won't compile in GCC. :-(

"void main() is explicitly prohibited by the C++ standard and shouldn't be used"

https://stackoverflow.com/questions/204476/what-should-main-return-in-c-c

Rather than #include , #include . That will correctly give you rand and abs. There used to be some flak about including anything that ends in ".h" out of the standard library and instead use the "c-prefixed-and-non-suffixed" versions, but last I checked it didn't actually make a difference. Still, some people think it does, so best not to upset them.

Read this post of mine about numeric_limits, and replace your USHRT_MAX and
UINT_MAX appropriately.

http://hostilefork.com/2009/03/31/modern_cpp_or_modern_art/

GCC doesn't like this line:

unsigned int most_mutation = unsigned int(size * 0.1);


I'm not sure what the point of that is supposed to be. If you want to static cast it for clarity, I guess that's fine:

unsigned int most_mutation = static_cast(size * 0.1);


With those changes, it compiles in GCC. Speaking of which: whatever compiler you are using...it can be helpful to have a virtual machine around to use some different ones on your code (Clang, GCC, MSVC) and see what they report.

The next best-practices step is to bump the warnings all-the-way-up. I'm now using settings I got from an answer by David Stone. Here's what that gives us:

test.cpp:226:0: error: ignoring #pragma region  [-Werror=unknown-pragmas]
test.cpp:272:0: error: ignoring #pragma endregion  [-Werror=unknown-pragmas]
test.cpp: In member function ‘void algo::run(long unsigned int, unsigned int, unsigned int, bool)’:
test.cpp:107:86: error: use of old-style cast [-Werror=old-style-cast]
test.cpp:67:27: error: switch missing default case [-Werror=switch-default]
test.cpp: In member function ‘void algo::mutate(unsigned int)’:
test.cpp:212:40: error: conversion to ‘unsigned int’ from ‘int’ may change the sign of the result [-Werror=sign-conversion]
test.cpp:215:77: error: conversion to ‘unsigned int’ from ‘int’ may change the sign of the result [-Werror=sign-conversion]
test.cpp:218:50: error: conversion to ‘unsigned int’ from ‘int’ may change the sign of the result [-Werror=sign-conversion]
test.cpp:220:35: error: conversion to ‘size_t {aka unsigned int}’ from ‘int’ may change the sign of the result [-Werror=sign-conversion]
test.cpp: In function ‘int main()’:
test.cpp:277:34: error: use of old-style cast [-Werror=old-style-cast]


You can address those on your own, but I'll cover the pragma philosophy. Firstly: I never indent preprocessor directives--it calls them out more clearly. But even better: don't use 'em, especially not for a fluffy IDE feature (why doesn't it use a certain comment to cue that?) Pragmas are "implementation defined", and putting little bits of dirt in your program like this is a slippery slope. GCC used to discourage this:


In some languages (including C), even the compiler is not bound to behave in a sensible manner once undefined behavior has been invoked. One instance of undefined behavior acting as an Easter egg is the behavior of early versions of the GCC C compiler when given a program containing the #pragma directive, which has implementation-defined behavior according to the C standard. In practice, many C implementations recognize, for example, #pragma once as a rough equivalent of #include guards — but GCC 1.17, upon finding a #pragma directive, would instead attempt to launch commonly distributed Unix games such as NetHack and Rogue, or start Emacs running a simulation of the Towers of Hanoi.

Now I'll just ramble about formatting and style, without trying to grok the "big picture" of what your code is for. :)

This is a matter of taste, but in implementation files using namespace std; can make your code less wordy. (Doing it in headers is not considered a good practice, as it would then be inherited by all implementation files that used the headers...giving them less control over potential name collisions.)

Also, it's generally considered a bad idea to make data members in your classes public. Narrowing the interface through methods gives you more wiggle room to modify the implementation without clients of the class to be rewritten. So:

using namespace std;

class algo
{
    private:
        vector code;
        vector output;
        bool debug_info;

    public:
        algo()
        {
        /* stuff... */


I personally like to see spaces between things and logical breaks in output. So following on that I'd turn:

std::cout<<std::endl<<"size: "<<code_size<<std::endl<<
        std::endl<<"output: "<<std::endl;
size_t output_size = output.size();
for (size_t t = 0; t < output_size; t++)
    std::cout<<output[t]<<std::endl;


...into the much-less-claustrophobic:

```
cout << endl;
cout << "size: " << code_size << endl;
cout << endl;
cout << "output: " << endl;
for (size_t t = 0; t < output_size; t++) {
cout

Code Snippets

unsigned int most_mutation = unsigned int(size * 0.1);
unsigned int most_mutation = static_cast<unsigned int>(size * 0.1);
test.cpp:226:0: error: ignoring #pragma region  [-Werror=unknown-pragmas]
test.cpp:272:0: error: ignoring #pragma endregion  [-Werror=unknown-pragmas]
test.cpp: In member function ‘void algo::run(long unsigned int, unsigned int, unsigned int, bool)’:
test.cpp:107:86: error: use of old-style cast [-Werror=old-style-cast]
test.cpp:67:27: error: switch missing default case [-Werror=switch-default]
test.cpp: In member function ‘void algo::mutate(unsigned int)’:
test.cpp:212:40: error: conversion to ‘unsigned int’ from ‘int’ may change the sign of the result [-Werror=sign-conversion]
test.cpp:215:77: error: conversion to ‘unsigned int’ from ‘int’ may change the sign of the result [-Werror=sign-conversion]
test.cpp:218:50: error: conversion to ‘unsigned int’ from ‘int’ may change the sign of the result [-Werror=sign-conversion]
test.cpp:220:35: error: conversion to ‘size_t {aka unsigned int}’ from ‘int’ may change the sign of the result [-Werror=sign-conversion]
test.cpp: In function ‘int main()’:
test.cpp:277:34: error: use of old-style cast [-Werror=old-style-cast]
using namespace std;

class algo
{
    private:
        vector<unsigned short> code;
        vector<unsigned int> output;
        bool debug_info;

    public:
        algo()
        {
        /* stuff... */
std::cout<<std::endl<<"size: "<<code_size<<std::endl<<
        std::endl<<"output: "<<std::endl;
size_t output_size = output.size();
for (size_t t = 0; t < output_size; t++)
    std::cout<<output[t]<<std::endl;

Context

StackExchange Code Review Q#18982, answer score: 10

Revisions (0)

No revisions yet.