Implementing a C++11 raw string class I'm trying to implement a RawString class that conforms to C++11:

#include <iostream>
#include <string>
#include <memory>

class RawString
{
    private:
        char* buff_;
    public:
        RawString()
        {
            std::cout << "rawstring def ctor\n";
            buff_ = nullptr;
        }
        ~RawString()
        {
            delete [] buff_;
        }
        RawString(const char* str) {
            std::cout << "rawstring normal char* ctor\n";
            buff_ = new char[strlen(str) + 1];
            strcpy(buff_, str);
            buff_[strlen(str)] = '\0';
        }
        RawString(const RawString& str) : RawString(str.buff_) {
            std::cout << "rawstring copy ctor\n";
        }
        RawString(RawString&& other) {
            std::cout << "rawstring move ctor\n";
            buff_ = other.buff_;
            other.buff_ = nullptr;
        }
        RawString(char*&& other) {
            std::cout << "rawstring move ctor\n";
            buff_ = other;
            other = nullptr;
        }
        void swap(RawString& other) {
            std::swap(other.buff_, buff_);
        }
//      RawString& operator=(const char* str) {
//          std::cout << "operator=char*\n";
//          buff_ = new char[strlen(str) + 1];
//          strcpy(buff_, str);
//          buff_[strlen(str)] = '\0';
//          return *this;
//      }
        RawString& operator=(RawString other) {
            std::cout << "opeartor=\n";
            swap(other);
            return *this;
        }
        friend const RawString operator+(const RawString& lhs, const RawString& rhs) {
            std::cout << "friend opt+\n";
            char* buff = new char[strlen(lhs.buff_) + strlen(rhs.buff_) + 1];
            strcpy(buff, lhs.buff_);
            strcat(buff, rhs.buff_);
            buff[strlen(lhs.buff_) + strlen(rhs.buff_)] = '\0';
            return std::move(buff);
//          RawString newStr(buff);
//          delete[] buff;
//          return newStr;
        }
};

int main()
{
    RawString str1("how are you");
    RawString str2(str1);
    RawString str3 = str1;
    RawString str4;
    str4 = str1;
    str4 = "how are you";
    RawString str5 = str1 + str2;
    return 0;
}

Is my implementation correct?
Should swap be friend?
Should I implement the RawString& operator=(const char* str) assignment operator?
Do I need to implement the char* move constructor?
Could you give me some additional suggestions?

#include <string>

strcpy and its friends reside within the C library header <string.h>, which should be included as <cstring> into a C++ program. Those functions then should be prepended with std::, e.g. std::strcpy. The C++ library versions defined in the C++ headers have overloads, which fixes some issues.

char* buff_;

You can replace that with std::unique_ptr<char[]>. You won't have to implement the move functions nor the destructor when using a unique_ptr data member.

Arguably, you should use a separate class for the whole memory management, following the Single Responsibility Principle. std::vector<char> is a candidate, for example.

RawString(const char* str) {
    std::cout << "rawstring normal char* ctor\n";
    buff_ = new char[strlen(str) + 1];
    strcpy(buff_, str);
    buff_[strlen(str)] = '\0';
}

The C library string functions need to compute the length of the strings at each step. This imposes additional, unnecessary work. Additionally, the last step null-terminating buff_ should be unnecessary. Both strlen and strcpy already require that str is null-terminated, and strcpy copies the null-terminator.

RawString(const char* const str) {
    std::cout << "rawstring normal char* ctor\n";

    auto const src_len = std::strlen(str);
    auto const src_len_with_nullterminator = len + 1;

    buff_ = new char[src_len_with_nullterminator];

    std::copy(str, str+src_len_with_nullterminator, buff_);
}

Also note that a user might already know the length of the string passed in; yet, there's no way to tell RawString this length.

RawString(const RawString& str)

If you do not have an very memory-constrained system, consider adding a data member that stores the size of the string. You won't have to recompute it in this and other member functions.

RawString(RawString&& other)

Your move constructor and separate move assignment-operator (see below) as well as swap should be marked as noexcept. Not only exposes this a useful guarantee to the user via a language feature, but it is also important for other types that make their behaviour dependent on the noexcept-ness of those special member functions. The usual example is std::vector<RawString>, which will provide some more efficient functions (push_back etc.) when the value type has noexcept-move-functions.

RawString(char*&& other) {
    std::cout << "rawstring move ctor\n";
    buff_ = other;
    other = nullptr;
}

For convenience, the converting constructor char const* -> RawString (similarly to std::string) is typically implicit, even though it (potentially) is not cheap to construct a RawString.

Your "moving" converting constructor is a constructor that takes ownership of a piece of memory. In my humble opinion, constructors that take ownership should be explicit. For example, it is rather simple to write this bug:

struct filesystem_entry
{
    char name[FILENAME_MAX];
};

auto x = filesystem_entry{};
RawString s = x.name;
// operate on `s` for convenience

Similarly, implicit conversions appear e.g. for returning or passing arguments to a function (where the target type is not immediately visible). The issue is not solved entirely by making the constructor explicit, though:

RawString s( name_of_file() );

If one uses direct-initialization by default, this could be realistic code in my opinion.

Of course, one could argue that such behaviour is defined precisely by the interface of RawString. I'd be surprised by such behaviour, though.

It is much safer to explicitly request a RawString to take ownership of a raw pointer:

RawString(std::unique_ptr<char[]> str) noexcept
    : buff_( str.release() )
{}

void swap(RawString& other) {
    std::swap(other.buff_, buff_);
}

Nowadays, the guideline is: Implement swap as a non-member friend function. This allows a generic usage of swap as follows:

using std::swap;
swap(x, y);

This works for both built-in types and user-defined types alike.

Note that since you have an efficient move ctor and move assignment-operator, implementing swap yourself is an optimization.

// RawString& operator=(const char* str)

With the OP's current implementation of operator=(RawString), implementing this assignment-operator is a more efficient than the implicit conversion used when it is not declared (const char* -> RawString implicit conversion, then using RawString::operator= (RawString)). However, if you implement an operator=(RawString&&), the current implementation is no more efficient than the implicit conversion used without operator=(const char*).

It is possible though to implement an optimization for this as well as the RawString::operator=(RawString const&) operator (see below), which makes this function useful.

RawString& operator=(RawString other)

Although this copy-and-swap technique is simple, it is not the most efficient(*): If the right hand side of the assignment is an lvalue, and the left hand side already has enough capacity to store the string of the RHS, you waste an allocation:

RawString myString("Hello World");
auto cpy = myString;
myString = cpy; // unnecessary allocation

It is therefore useful to define both separately:

RawString& operator=(RawString const&);
RawString& operator=(RawString&&);

If you follow my advice from above and move the memory management to another class, you can use the compiler-provided implementation for both in RawString.

(*) Howard Hinnant often refers to this, e.g. in this StackOverflow answer. See also: Herb Sutter on "Don't overuse pass-by-value"

friend const RawString operator+(const RawString& lhs, const RawString& rhs)

Although it's almost a convention now, Alexander Stepanov (the inventor of the STL) opposes the usage of + to concatenate strings. He argues that string concatenation is not commutative.

Anyway, in this implementation, we can see a possible inefficiency in the OP's allocation scheme. Classes like std::string and std::vector typically manage a block of memory that is only partially filled. This allows resizing by a certain factor once more memory is needed. An amortized constant complexity can be reached for growth this way.

Also, the OP's implementation uses C library string functions again.

Another inefficiency is that operator+ ignores the value category of its operands. Although an operator like + is supposed to not modify its operands, it can do so if they're rvalues. This allows stealing the resources of any operand. For a similar reason, returning const objects by values is detrimental. Consider:

RawString x("Hello");
auto s = x + " " + "World!";

We get a temporary string from x + " ", but cannot use its resources for the concatenation of + "World!", even if its buffer had enough capacity. Furthermore, the initialization of s requires yet another copy, since the returned RawString is const. s will be a const RawString, which prevents of course modification, even though that could be useful. Because of copy elision, no copy is made for the initialization of s. Similarly, no copy is made for an assignment of the same form, if the operator= takes a RawString by value - on the other hand, declaring operator=(RawString&&) and returning a non-const RawString from operator+ should be no less efficient.

If you provide operator overloads, you should also make sure that there is some consistency. For example, operator+ corresponds to an operator+=.

Speaking about operator overloads, an operator== and operator< (as well as its counterparts like >) are generally very useful, e.g. for sorting.

(This leads to regular types.)

Oh, and by the way: There's currently no public interface to do something with the contents of the string, sadly. Depending on this interface, the default constructor might be dangerous: It does not initialize buff_ with the address of a null-terminated string (this is a nice class invariant).

Late addition: Oops, I missed a critical bug, related to that "nice class invariant" I mentioned above: This class invariant / guarantee is assumed to exist e.g. in the copy constructor:

RawString(const RawString& str) : RawString(str.buff_)

The converting constructor that is the target of the constructor delegation requires the argument to be a null-terminated string (because of the use of strlen and friends).

Similarly, operator+ uses the same assumption.

However, this invariant is violated by the default constructor:

RawString()
{
    std::cout << "rawstring def ctor\n";
    buff_ = nullptr;
}

I can see two relatively simple solutions that maintain the class invariant:

RawString()
{
    std::cout << "rawstring def ctor\n";
    buff_ = new char[1]();
}

This makes the default constructor not noexcept (even though it's unlikely to throw), and it's probably not an efficient solution because of the allocation.

Solution two requires some more work in the destructor:

private:
static char* empty_string() {
    static char str[] = {0};
    return &str;
}

public:
RawString()
{
    std::cout << "rawstring def ctor\n";
    buff_ = empty_string();
}

~RawString()
{
    if(empty_string() != buff_) delete[] buff_;
}

Another possibility is to give up on that class invariant, and allow buffer_ == nullptr. As Loki Astari mentions in a comment to the OP, the bug can be fixed in that case by altering the code in RawString(char const*) and operator+.


Generate points for smooth movement between two given points This is one of the methods in a project for generating points between two given points. We add points of route with a 25ms delay in an array and a camera on Google Maps is always at the last point. So we need to move it smoothly. This method adds those extra points between any two given points to make it move smoothly.

public List<Tuple<double, double>> GeneratePointsForSmoothMovement(double x1, double y1, double x2, double y2, double divider)
{
    // If points are same, return empty list
    if (x1 == x2 && y1 == y2)
        return new List<Tuple<double, double>>();

    // get difference between x and y cordinates
    double xi = (x2 - x1);
    double yi = (y2 - y1);

    // divide difference into n parts
    xi = xi / divider;
    yi = yi / divider;

    // set new temp vars equal to first point
    double xf = x1;
    double yf = y1;

    // initialize list to save new points
    List<Tuple<double, double>> points = new List<Tuple<double, double>>();

    // increment temp vars by difference-division parts
    for (int i = 0; i < divider; i++)
    {
        xf += xi;
        yf += yi;

        points.Add(Tuple.Create(xf, yf));
    }

    // last cordinate in the list will be equal to point 2
    // return
    return points;
}

Is this the most efficient way ?

Dividor

dividor is ill-defined. It acts as a factor for quantization:

 xi = xi / divider;

But also as threshold for yielding smoothed results:

for (int i = 0; i < divider; i++)

While this seems fine if dividor is \$integer >= 2\$, it could yield unwanted results for fractions (as it is a double precision integer). I would at least change its type to int or uint instead and add an out-of-bound guard. If you would decide to allow fractions, make it clear in the spec how the edge case near the end is handled, since you might get a scenario where you don't synchronize with the end value.

Usability

GeneratePointsForSmoothMovement is a low level generator. The caller code should still think about technical parameters to provide. And if a caller requires several levels of smoothness, multiple calls with different parameters have to be executed. In order to provide the consumer a solution that avoids most overhead, you could write an extension method on IEnumerable<Tuple<Double,Double>> and preferrably also on IDictionary<Double,Double> for faster lookup.

In pseudo code:

public static Func<Double, Double> ToContinuousFunction(
    this IDictionary<Double,Double> points)
{
    return new Func<Double, Double>(x =>
    {
        if (points.TryGetValue(x, out var y))
        {
            return y;
        }

        // .. perform lineair interpolation or extrapolation based on nearby points
    });
}

Let's say we have points (x: 1, y: 10) and (x: 2, y: 20):

var f = points.ToContiniousFunction();
var yValues = new []
{
    f(1)    // 10  (from cache)
   ,f(1.5)  // 15  (interpolation)
   ,f(2)    // 20  (from cache)
   ,f(3)    // 30  (extrapolation)
};


Perturbed phase oscillator integration I am integrating a system of perturbed phase oscillators. I define the system of equation and also the Jacobian matrix. I have to reshape the one dimensional vector of system state to a 2 dimensional vector then do the matrix production.

Is there any single-threaded way to make the program faster?

using dim1 = std::vector<double>;
using dim2 = std::vector<std::vector<double>>;

dim2 multiply_matrices(dim2 &A, dim2 &B)
{
    int r1 = A.size();
    int c1 = A[0].size();
    int r2 = B.size();
    int c2 = B[0].size();

    assert(c1==r2 && "bad dimension for matrix multiplication");

    dim2 mult(r1, dim1(c2));

    for (int i = 0; i < r1; ++i) {
        for (int j = 0; j < c2; ++j) {
            mult[i][j] = 0;
        }
    }

    // Multiplying matrix A and B and storing in array mult.
    for (int i = 0; i < r1; ++i) {
        for (int j = 0; j < c2; ++j) {
            for (int k = 0; k < c1; ++k) {
                mult[i][j] += A[i][k] * B[k][j];
            }
        }
    }

    return mult;
}

class kuramoto_exe
{
    vector<vector<double>> &adj;
    dim1 &omega;
    int kind;
    int N;
    double coupling;

public:
    kuramoto_exe (
        int N_, int kind_, double coupling_,
        dim1& omega_, dim2& adj_) : adj(adj_), omega(omega_),
            kind(kind_), coupling(coupling_), N(N_) { }

        void operator() (const dim1 &X, dim1& dXdt, const double /*t*/)
    {

        dim1 x(X.begin(), X.begin() + N); // initial condition

        // reshaping:every colomn is a vector (reshape order "F")
        dim2 Y(N, dim1(N));
        for (int j=0; j<N; j++)
            for (int i=0; i<N; i++)
                Y[i][j] = X[N*j+i+N];

        dim2 Jac(N, dim1(N));

        // definition of unperturbed model
        for (int i=0; i<N; i++) {
            double sumj =0.0;
            for (int j=0; j<N; j++) {
                if ((i != j) && (adj[i][j] > 1e-8))
                        sumj += adj[i][j] * sin(x[j]-x[i]);

                dXdt[i] =  omega[i]+ coupling * sumj;
            }
        }
        // calculation of the Jacobian
        for (int i=0; i<N; i++) {
            for(int j=0; j<N; j++) {
                if (i!=j)
                    Jac[i][j] = coupling * adj[i][j] * cos(x[j] - x[i]);
                else {
                    double sumj = 0.0;
                    for (int jj=0; jj<N; jj++) {
                        if (jj!=i)
                            sumj += adj[i][jj] * cos(x[jj] - x[i]);
                    }
                    Jac[i][j] = -1.0 * coupling * sumj;
                }
            }
        }

        dim2 M = multiply_matrices(Jac, Y);

        for (int j=0; j<N; j++)
            for (int i=0; i<N; i++)
                dXdt[N*j+i+N] = M[i][j]; // revese of reshape order "F"
    }
};

First off, be sure you're compiling with optimizations enabled.

In multiply_matrices you don't need to explicitly fill your matrix with zeros, as the vector constructor will do that.

In the kuramoto_exe constructor, the member initializer list has the member variables in the wrong order.  They will be constructed in the order they are declared, so N will be constructed before coupling despite its appearing later in the list.  While not an issue here, it can be if there are dependencies among the member variables, and some compilers will issue a warning when you do this.

In your matrix multiply, accumulate the result to a local variable, then store the result in mult[i][j].  This avoids the potential of multiple index calculations.

When calculating the Jacobian, the diagonal members are the negative of the sum of the horizontal members.  You're doing twice as much work as you need to.  Accumulate the sum as you process the row (skipping the diagonal element) then store the diagonal element when you're done with the row (Jac[i][i] = -rowsum;).

When you're doing the initial reshaping (filling the Y matrix), it is probably better to put the i loop on the outside, so that your writes are sequential.  It is easier for the CPU to handle nonsequential reads then nonsequential writes.

Rather than using a vector of vectors, consider creating a matrix class that stores all elements contiguously (in one vector) and computes the appropriate index.  This avoids the double lookup you currently have.


Display Object as a node tree in HTML with clickable trace back to root node I was given the following code and instructions for a technical test for a potential technical role I had applied for:

var input = {
    label: 'Earth',
    type: 'planet',
    children: [{
        label: 'Australia',
        type: 'country',
        children: [{
            label: 'Sydney',
            type: 'city',
            children: [{
                label: 'Nuix HQ',
                type: 'office',
                children: []
            }]
        }]
    }, {
        label: 'USA',
        type: 'country',
        children: [{
            label: 'San Franscisco',
            type: 'city',
            children: [{
                label: 'Nuix SF',
                type: 'office',
                children: []
            }]
        }, {
            label: 'Philadelphia',
            type: 'city',
            children: [{
                label: 'Nuix Philly',
                type: 'office',
                children: []
            }]
        }]
    }]
};

Task:

  Using Javascript please render HTML element that displays above JSON data.
  It should reflect parent-child relationship by appropriate element nesting and indentation.
  The structure should be right-justified, meaning the indentation should start from the right and move to the left as the nodes descend.
  Additionally, when a node is clicked, it should print (to console) all labels on a path from clicked node to the root (Earth).

  CSS visual styling is optional (nice to have).
  You can use any open-source utilities and libraries available in the fiddle, however we also value simplicity of the solution.
  The solution should be delivered in a fork of this fiddle.

I supplied the additional code as a fork of the original code:

Javascript:

let node = 1;
var result = [];
var arrNodeParent = [];
var arrNodeName = [];
tree = (lvl, parent = 0, node, data) =>
{
    let div = `<div class="node lvl${lvl}" node="${node}" parent="${parent==0?0:parent-1}">${data.label}</div>`;

    arrNodeParent[node] = parent==0?0:parent-1;
    arrNodeName[node] = data.label;

  n = node+=1;
  (data.children.length)&&data.children.forEach(e=>tree(lvl+1, node, n, e));
  result[node]=div;
}

tree(0,0,node,input)

$('.tree').append(...result);

$(".node").click(function(){
    complete = false;
        currNode = $(this);
  while(!complete){
    n = +currNode.attr("node");
    p = +currNode.attr("parent");
        console.log(arrNodeName[n]);
    currNode = $("div[node='"+arrNodeParent[n]+"']");
    complete = (p==0);
  }
});

HTML:

CSS:

.tree {
width: 100%;
}
.node {
  font-size: 12px;
  padding: 3px;
  margin-bottom: 1px;
  border: 1px solid black;
  background-color: rgb(255, 255, 128);
  width: 100px;
  margin-left: auto;
  margin-right: 0px;
}
.node:hover {
  cursor: pointer;
}
.lvl0 {
  margin-right: 0;
}
.lvl1 {
  margin-right: 20px;
}
.lvl2 {
  margin-right: 40px;
}
.lvl3 {
  margin-right: 60px;
}
.lvl4 {
  margin-right: 80px;
}
.lvl5 {
  margin-right: 100px;
}

Now, I was happy with my result, I felt I passed the brief... The client has come back saying I didn't meet the brief... I'm confused here... What specifically have I missed? And is there anything I could have done? I have asked for feedback, but I have yet to receive anything in response... Could someone possibly shed some light?

It's always hard to guess, what the interviewer actually wanted. However, here are some thoughts mostly on style and markup.

Arrow function

You're using an arrow function in global scope, where it doesn't add value by binding this:

tree = (lvl, parent = 0, node, data) => {}

I would stick with a normal function here. It is also more readable. There's a discussion on SO – When should I use Arrow functions in ECMAScript 6? – which also explain why and where arrow functions are beneficial.

Readability

Here are some parts, that are hard to read:

arrNodeParent[node] = parent==0?0:parent-1;
n = node+=1;
(data.children.length)&&data.children.forEach(e=>tree(lvl+1, node, n, e));

Especially the last line. While it makes use of JavaScript's way to handle logical expressions, I would say: This is not Code Golf but production code. So readability and quick understanding of the code, even tomorrow, is more important then saving a few bytes by creating one-liners.

Inconsistent use of keywords

Sometimes you declare variables using let, often you don't. Sometimes const could be used instead. Also, think twice before using var and check whether globals are really necessary for your function.

Indentation

Your Indentation is inconsistent. Sometimes it's two, sometimes it's four characters. Some lines are totally off, like here:

    complete = false;
        currNode = $(this);
  while(!complete){

HTML

Semantics/nesting

  It should reflect parent-child relationship by appropriate element nesting and indentation.

A div and also span are the elements with no semantic meaning, which make them not the first choice for a relationship like that. You also don't nest the elements to reflect that, you simply list them:

Earth

Australia

Sydney

Adding a class like level-n does not add semantics to HTML.

You can change the markup to a nested list for example:

        Earth

                Australia

Sydney

Invalid markup

You add two attributes node and parent to each element. These aren't valid HTML attributes. If you want to use your own custom ones, use data-*-attributes, like:

Earth

jQuery

Yes, the task says, you can use any open source library you need. But it also says they like simplicity. So adding 87 kB of JavaScript to simply attach an event handler and to select/create a few HTML elements seems like a huge overhead to me. If the library doesn't help building the actual tree, I would stick to vanilla JavaScript in this case.


Number in words: recursive function I would appreciate some feedback on the below code I wrote for a Project Euler problem. The answer comes out correct but I'd like feedback on the design of the function and implementation of docstrings. It seems to me that it may be overly complicated but I'm not sure how to simplify.

"""
Number letter counts

Problem 17
If the numbers 1 to 5 are written out in words: one, two, three, four, five, then there are
3 + 3 + 5 + 4 + 4 = 19 letters used in total.

If all the numbers from 1 to 1000 (one thousand) inclusive were written out in words, how many
letters would be used?

NOTE: Do not count spaces or hyphens. For example, 342 (three hundred and forty-two) contains
23 letters and 115 (one hundred and fifteen) contains 20 letters. The use of "and" when writing
out numbers is in compliance with British usage.
Answer = 21124
"""

num_word_map = {0: '', 1: 'one', 2: 'two', 3: 'three', 4: 'four', 5: 'five',
                6: 'six', 7: 'seven', 8: 'eight', 9: 'nine', 10: 'ten',
                11: 'eleven', 12: 'twelve', 13: 'thirteen', 14: 'fourteen',
                15: 'fifteen', 16: 'sixteen', 17: 'seventeen', 18: 'eighteen',
                19: 'nineteen', 20: 'twenty', 30: 'thirty', 40: 'forty',
                50: 'fifty', 60: 'sixty', 70: 'seventy', 80: 'eighty',
                90: 'ninety', 100: 'hundred', 1000: 'thousand'}

def convert_to_word(x, spacing=''):
    """
    Recursive function that converts number to it's equivalent word up to a limit of 9999.
    Function is called recursively until x < 20.

    Parameters
    ----------
    x : number to convert to word
    spacing : keeps track appropriate value to enter between words. This will be 'and' or an empty string

    Returns
    -------
    Equivalent word for the number x

    """
    if x / 1000 == 1:
        return convert_to_word(x // 1000) + num_word_map[1000]
    if 0 < x//100 < 10:
        if x % 100:
            return convert_to_word(x//100) + num_word_map[100] + convert_to_word(x % 100, 'and')
        else:
            return convert_to_word(x//100) + num_word_map[100]
    elif 2 <= x//10 < 10:
        return spacing + num_word_map[x - (x % 10)] + convert_to_word(x % 10)
    # 0 included to prevent TypeError on a value like 80
    elif 0 <= x < 20:
        return spacing + num_word_map[x]
    else:
        "Value out of bounds"

def count_letters(limit=1000):
    """
    Counts the number of letters in a word corresponding to a number from one to limit

    Parameters
    ----------
    limit : upper limit of numbers to convert and count to

    Returns
    -------
    None

    """
    count = 0
    for val in range(1, limit + 1):
        s = convert_to_word(val)
        print(s)
        count += len(s)
    print(count)

if __name__ == '__main__':
    count_letters()

Nice use of """docstrings""".

This code is useless:

else:
    "Value out of bounds"

If the else clause is reached, the string statement is executed, with no effect.  Just like executing a docstring statement has no effect.

You want:

else:
    raise ValueError("Value of out bounds")

The function convert_to_word() returns strange values like "sevenhundredandtwentyfive", which is only useful for counting letters, and nothing else.  A more flexible return might be ["seven", "hundred", "and", "twenty", "five"], allowing the caller to count words, or letters ... or ... join the words with spaces.

Several recursive calls are unnecessary.

convert_to_word(x//100) could simply be num_word_map[x // 100], and convert_to_word(x % 10) could be num_word_map[x % 10].

Comparison of a floating point value and an integer for equality is dangerous x / 1000 == 1.  Simply use x == 1000.

For this problem, instead of storing the words in num_word_map, you could simply store the lengths of the words.

Your """docstring""" usage is good.  You describe what the function does, what the parameters are, and what the return value is.  Someone wishing to use your functions would only need to type help(convert_to_word) and they should have all the information required to use the function.

There is no formal "right way" to format the docstrings, but there are some tools which expect things a certain way.

For instance

"""
... omitted ...

NOTE: Do not count spaces or hyphens. For example, 342 (three hundred and forty-two) contains
23 letters and 115 (one hundred and fifteen) contains 20 letters. The use of "and" when writing
out numbers is in compliance with British usage.
Answer = 21124
"""

This docstring contains 2 examples with answers, and one answer to a large question.  You can embed examples in a """docstring""", and it can become a built-in test for your module.

"""
... omitted ...

Answer = 21124

NOTE: Do not count spaces or hyphens.  The use of "and" when writing
out numbers is in compliance with British usage.  For example:

>>> len(convert_to_word(342))     # three hundred and forty-two
23

>>> len(convert_to_word(115))     # one hundred and fifteen
20
"""

Starting at the >>>, the text look like lines straight out of what you might see if you typed commands in a Python REPL.  The lines beginning with >>> are the commands, and the lines below that are the command output.  The doctest module can read the docstrings in a module, find lines formatted this way, execute the commands, and verify the output matches the expected output.  You might run this as:

python3 -m doctest numbers-letters.py

The output of the above command should be nothing, because the tests would pass.  Let's add one more test in the doc string:

"""
... omitted ...

>>> len(convert_to_word(342))     # three hundred and forty-two
23

>>> len(convert_to_word(115))     # one hundred and fifteen
20

>>> convert_to_word(115)
'one hundred and fifteen'
"""

Now running the doctest will produce output due to the failing test:

python3.8 -m doctest number-letters.py
**********************************************************************
File "/Users/aneufeld/Documents/Stack Overflow/number-letters.py", line 23, in number-letters
Failed example:
    convert_to_word(115)
Expected:
    'one hundred and fifteen'
Got:
    'onehundredandfifteen'
**********************************************************************
1 items had failures:
   1 of   3 in number-letters
***Test Failed*** 1 failures.

The output doesn't match because the test output contains spaces, which the function doesn't actually produce.

Other tools exist which read the docstrings, such as the Sphinx documentation generator.  It has formatting requirements; read the docs for details.


Simple array "manager" I'm new to Rust, coming from C++, and so to increase my knowledge of Rust I made a small program to manage an array. You can add, insert, delete, print and more an array or its elements.

I don't really care much about performance at this point, but more on readability, good practices and so on.

So, what could I have done better?

use std::io;

fn get_element() -> i64 {
    println!("Enter the element: ");

    let mut element = String::new();
    io::stdin().read_line(&mut element).expect("Failed to read line!");

    match element.trim().parse() {
        Ok(num) => num,
        Err(_) => {
            println!("Invalid number! Try again.");
            get_element()
        }
    }
}

fn get_index(length: usize) -> usize {
    println!("Enter the index: ");

    let mut index = String::new();
    io::stdin().read_line(&mut index).expect("Failed to read line!");

    match index.trim().parse() {
        Ok(index) if index < length => index,
        Ok(index) => {
            println!("Invalid index! {} is not in range [0, {})! Try again.", index, length);
            get_index(length)
        },
        Err(_) => {
            println!("Invalid number! Try again.");
            get_index(length)
        }
    }
}

fn main() {
    println!("Welcome to the Vector application!");

    let mut vec: Vec<i64> = Vec::new();

    loop {
        println!("=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=--=-=-");
        println!("What do you want to do?");
        println!("1. Clear the vector");
        println!("2. Add element");
        println!("3. Insert element at pos");
        println!("4. Delete an element at index");
        println!("5. Print an element");
        println!("6. Print vector");
        println!("*. Exit");

        let mut input = String::new();
        io::stdin().read_line(&mut input).expect("Failed to read line");
        let input: u8 = match input.trim().parse() {
            Ok(num) if num < 7 => num,
            Ok(num) => {
                println!("{} is not in the range [0, 6]!", num);
                continue;
            }
            Err(_) => {
                break;
            }
        };

        let length = vec.len();

        if input == 1 {
            vec.clear();
        }
        else if input == 2 {
            vec.push(get_element());
        }
        else if input == 3 {
            vec.insert(get_index(length), get_element());
        }
        else if input == 4 {
            vec.remove(get_index(length));
        }
        else if input == 5 {
            println!("The element is: {}", match vec.get(get_index(vec.len())) {
                Some(element) => element,
                None => {
                    println!("Invalid index!");
                    continue;
                }
            });
        }
        else if input == 6 {
            for i in &vec {
                println!("{} ", i.to_string());
            }
        }

    }
}

Become familiar with rustfmt. For example, else goes on the same line as the previous closing brace:

-        }
-        else if input == 2 {
+        } else if input == 2 {

Rust doesn't do tail call optimization, so unbounded recursive calls are better replaced with iteration.
I might extract a function that does the reading from standard input, trimming, and parsing.
No need for a type on the vec; inference will kick in.
You can use match with values like numbers. This can be used to replace the large if-else chain at the end of main.
Move the command out-of-range check/warning from the "parse" match to the match of the command value. This groups the error text closer to the place that will change.
I wouldn't embed the match inside the println!; it's not obvious enough. Instead, put the print inside the match.
Can then remove the continue because that's the normal flow
Don't convert the number to a string to print it; numbers implement Display.
You check the index twice; once when getting the value, once when actually indexing. Remove one or the other.

use std::io;

fn parsed_line<T>() -> Result<T, T::Err>
    where T: std::str::FromStr
{
    let mut element = String::new();
    io::stdin().read_line(&mut element).expect("Failed to read line!");
    element.trim().parse()
}

fn get_element() -> i64 {
    loop {
        println!("Enter the element: ");

        match parsed_line() {
            Ok(num) => return num,
            Err(_) => println!("Invalid number! Try again."),
        }
    }
}

fn get_index(length: usize) -> usize {
    loop {
        println!("Enter the index: ");

        match parsed_line() {
            Ok(index) if index < length => return index,
            Ok(index) => {
                println!("Invalid index! {} is not in range [0, {})! Try again.", index, length);
            },
            Err(_) => {
                println!("Invalid number! Try again.");
            }
        }
    }
}

fn main() {
    println!("Welcome to the Vector application!");

    let mut vec = Vec::new();

    loop {
        println!("=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=--=-=-");
        println!("What do you want to do?");
        println!("1. Clear the vector");
        println!("2. Add element");
        println!("3. Insert element at pos");
        println!("4. Delete an element at index");
        println!("5. Print an element");
        println!("6. Print vector");
        println!("*. Exit");

        let input: u8 = match parsed_line() {
            Ok(num) => num,
            Err(_) => break,
        };

        let length = vec.len();

        match input {
            1 => vec.clear(),
            2 => vec.push(get_element()),
            3 => vec.insert(get_index(length), get_element()),
            4 => {
                vec.remove(get_index(length));
            },
            5 => println!("The element is: {}", vec[get_index(length)]),
            6 => {
                for i in &vec {
                    println!("{} ", i);
                }
            }
            other => println!("{} is not in the range [0, 6]!", other),
        };
    }
}


Functional Programming Style in Javascript Picking a Random Item from an Array I'm trying to understand the Functional Programming Paradigm (or style) implemented with Javascript. There are a few things I have difficulties with. The naming conventions, and dependency injections. I would be super happy if you could help me out with these two problems.
const items = ['', '', '', ''];

const pickRandomArrayNumber = (floorFunction, randomFunction) => array => floorFunction(randomFunction() * array.length);
const pickRandomItemFrom = pickRandomArrayNumber(Math.floor, Math.random);
console.log(items[pickRandomItemFrom(items)]);

This piece of code grabs a random number between 0 and the length of the provided array. However, I wanted the getRandomArrayNumber function to be independent.
I'm not happy with the names of the functions as they are right now. Mainly because getRandomArrayNumber doesn't really get an array number, but instead provides a function that does.
Is this Functional Programming style future proof and does it follow the Functional Programming Principles correctly? And how would you name these functions? Or perhaps, handle dependency injection in a functional style?
Thanks in advance!

Naming
The name pickRandomItemFrom is too long

pick implies removal which the function does not do
from is redundant.

Thus pickRandomItemFrom can simply be randomItem with Item implying an array
Ignoring the obvious problems and removing the unnecessary currying we can rewrite your code as
const randomItem =  array => array[Math.floor(Math.random() * array.length]);
console.log(randomItem(items));

Math.random is not pure
The function Math.random is not pure because you can not control the seed value. Using it in any function will make that function impure.
To be pure a function must return the same value given the same input.
You can create a pure random function but the function must take a seed value from which to calculate a random value.
Example
const seededRandom = (() => {
    const max = 2576436549074795, add = 8507698654323524, scale = 8765432352450986;
    return seed => ((scale * Math.abs(Math.floor(seed))) + add) % max;
})();

Selecting random item
You can then create the randomItem function using
const randomItem = ((rand) => (arr, seed) => arr[rand(seed) % arr.length])(seededRandom);

Used as follows
console.log(randomItem([1,2,3], Date.now()));

Note that the function can be called many times every ms. To ensure a unique seed modify the seed each call.
Example

const seededRandom = (() => {
    const max = 2576436549074795, add = 8507698654323524, scale = 8765432352450986;
    return seed => ((scale * Math.abs(Math.floor(seed))) + add) % max;
})();

const randomItem = (rand => (arr, seed) => arr[rand(seed) % arr.length])(seededRandom);

const test = [1, 2, 3, 4, 5, 6, 7, 8];

var seed = Date.now();
console.log(randomItem(test, seed++));
console.log(randomItem(test, seed++));
console.log(randomItem(test, seed++));
console.log(randomItem(test, seed++));

// or
console.log("-------------------------");
console.log(randomItem(test, seed = seededRandom(seed)));
console.log(randomItem(test, seed = seededRandom(seed)));

// or curried
const randomizeItems = ((random) => (arr, seed) => () => arr[seed = random(seed) % arr.length])(seededRandom);

const randomTestItem = randomizeItems(test, seed);

console.log("-------------------------");
console.log(randomTestItem())
console.log(randomTestItem())
console.log(randomTestItem())
console.log(randomTestItem())

Note Random means pseudo random in context of this answer


Angular Box Color Exercise I have completed an exercise where there are 4 different color boxes and 1 main box below them that is blank, when you click on one of the colored boxes, the main box changes to the color of the box you just clicked on. Also there should be working undo and redo buttons.

Am I doing this the "Angular" way by using $event or no?

(function() {
  'use strict';

function angularBoxesCtrl() {
  var vm = this;
  vm.mainBoxColor;

  var colorHistory = [];
  var undoRedo = [];

  var changeColor = function() {
    vm.mainBoxColor = colorHistory[colorHistory.length - 1];
  }

  vm.logClass = function(e) {
    var boxColor = e.target.classList[1];
    //mainBox is already same color as clicked box
    if (boxColor === vm.mainBoxColor) {
      return;
    }
    colorHistory.push(boxColor)
    changeColor();
  }

  vm.undo = function() {
    if (!colorHistory.length) {
      return;
    }
    var undoColor = colorHistory.pop();
    undoRedo.unshift(undoColor);
    changeColor();
  }

  vm.redo = function() {
    if (!undoRedo.length) {
      return;
    }
    var redoColor = undoRedo.shift();
    colorHistory.push(redoColor);
    changeColor();
  }
}

angular.module('angularBoxes', [])
  .controller('angularBoxesCtrl', [
    angularBoxesCtrl
    ]);

})();
div {
  box-sizing: border-box;
}

.boxHolder {
  border: 1px solid black;
  text-align: center;
  font-size: 0;
}

.buttonHolder {
  text-align: center;
  position: relative;
  top: 40px;
}

.box {
  width: 180px;
  height: 180px;
  display: inline-block;
  border: 1px solid black;
  margin: 4px;
}

.green {
  background-color: green;
}

.blue {
  background-color: blue;
}

.red {
  background-color: red;
}

.yellow {
  background-color: yellow;
}

.mainBox {
  width: 400px;
  height: 400px;
  margin: 0 auto;
  border: 1px solid black;
  position: relative;
  top: 80px;
}
<!DOCTYPE html>

  <meta charset="UTF-8">
  <title>Angular Boxes</title>
  <link rel="stylesheet" href="styles.css">

    <button ng-click="vm.undo()">Undo</button>
    <button ng-click="vm.redo()">Redo</button>

I would pass the className to the logClass method. Because how you wrote the method you enforce that the color name is the second class applied on the control. This is very fragile, as there is no direct relation between the class order in the an element and the result apply to them. So I would do:

vm.logClass = function(className) {
...
}
//template


Find minecraft colour codes in string I've written a simple util-method in my spigot plugin to check if a message contains a valid minecraft colour code. A valid minecraft colour code consists of a & followed by a hex digit: 0 to f. Minecraft itself exchanges all & by a §, for reasons that would take too long to explain, it's not possible for the users to send messages to the server directly. Therefore & is generally used as a replacement.
I now have to check at some point, if the message the user sent to the server is coloured, which is the reason this method exists. Here are some quick examples, how it does work:
string value - return value

"Simple String" - false
"Smith&Wesson" - false
"&4This would be red." - true
"%aGreen &2Darker Green &0 Darkest Green" - true
"&& Omega &&" - false

Here's my code. It does what it's supposed to do, but for some reason it still seems unneccesary clunky to me. Any improvement is welcome :)
    public static boolean isMessageColoured(String message) {
        char[] arr = message.toCharArray();
        for (int i = 0; i < message.length() - 2; i++) {
            if (arr[i] == '&' && "0123456789abcdef".contains(Character.toString(arr[i + 1]))) {
                return true;
            }
        }
        return false;
    }

Welcome to Code Review.

Your code returns false for the message "hello&9", because the method doesn't check the last character. Change the condition in the for-loop to message.length() - 1.

An alternative is to use a regular expression:
public static boolean isMessageColoured(String message) {
    return message.matches(".*&[a-f0-9].*");
}

This regular expression matches at least one occurrence of &[a-f0-9] in the message.


Generating sequence that uses the previous outcome for the current I do not like to use while loops in Ruby. I was wondering how I can generate a "squared sequence" (e.g., squares the first input, then squares the outcome, etc.) in a more idiomatic Ruby way than this one:

value = 2
while value < 10000
    puts value = value**2
end

# => 4
#    16
#    256
#    65536

I have a suspicion that there is a way/method to do this, but I do not know which one. I hope someone can point me in the right direction.

Using while would be idiomatic in almost any language. while is basically the way to iterate, uh, while a condition is true. Hence the name - it's practically plain English.

You can postfix the while and save a couple of lines, but that's about it

puts value = value**2 while value < 10000

or use until if you want, but same difference

puts value = value**2 until value >= 10000

If you were dealing with a fixed or known number of iterations, you could do something like

4.times.inject(2) do |memo, _|
  puts memo = memo ** 2
  memo
end

but the whole point here is really that you don't know the number of iterations.


Using Mutex to prevent multiple WPF application launches I've inherited an existing application which is a WPF implementation of toast notifications. This application is installed to multiple client PCs across the estate and, once installed, runs in the background. The notifier then works regardless of who is logged onto the machine. One of the improvements I have been asked to make is to prevent the application from being started more than once.

I am considering using a mutex to do this. I've implemented a solution that functionally works at startup using inspiration from MSDN and Stack Overflow but would like to identify any potential weaknesses or scenarios which could break the code. Ultimately, I don't want some crash to occur that might fail to dispose the mutex or leave the application unable to start, although I think I have catered as best possible.

public partial class MainWindow : Window
{
    // unique id for application mutex
    private static string mutexId = string.Format("Local\\{{{0}}}", "1109F104-B4B4-4ED1-920C-F4D8EFE9E833");

    private static string applicationName = "My App";

    private bool hasHandle;

    private Mutex mutex;

    /// <summary>
    /// Initializes a new instance of the MainWindow class.
    /// N.B. Inspiration for this implementation sourced from:
    /// https://stackoverflow.com/questions/229565/what-is-a-good-pattern-for-using-a-global-mutex-in-c/229567
    /// </summary>
    public MainWindow()
    {
        this.mutex = new Mutex(false, mutexId);

        // Allow all users of the system to access the mutex with full control.
        var allowEveryoneRule = new MutexAccessRule(new SecurityIdentifier(WellKnownSidType.WorldSid, null),
            MutexRights.FullControl, AccessControlType.Allow);
        var securitySettings = new MutexSecurity();
        securitySettings.AddAccessRule(allowEveryoneRule);
        mutex.SetAccessControl(securitySettings);

        try
        {
            try
            {
                hasHandle = this.mutex.WaitOne(1000, false);
                if (!hasHandle)
                {
                    MessageBox.Show("Application is already running.", applicationName);
                    Application.Current.Shutdown();
                }
            }
            catch (AbandonedMutexException)
            {
                // Log the fact the mutex was abandoned in another process, it will still get aquired
                hasHandle = true;
            }

            // Perform your work here.
            InitializeComponent();
            Closing += (s, e) => Shutdown();
        }
        catch (Exception e)
        {
            //If InitializeComponent throws an error, gracefully cleanup the application.
            Shutdown();
        }

    }

    private void Shutdown()
    {
        ViewModelLocator.Cleanup();

        // Release the mutex if a handle was aquired while the application was running.
        if (this.hasHandle)
        {
            this.mutex.ReleaseMutex();
        }

        this.mutex.Dispose();
    }
}

For brevity I've omitted using statements and namespace declarations, as well as the .xaml files and existing solution, as I want to focus on this new improvement, which is self-contained. I'm also conscious of trying to keep startup and shutdown as simple as possible, so would happily consider improvements to remove unneeded code.

Finally, I have seen this similar question and would be happy with an answer to use this example if this is the best way of meeting the requirement.

Thing that make me more unhappy is mixing UI logic of your MainWindow class with this single instance logic that really belongs to process (and then Application).

First thing I'd do is to introduce an explicit Main() method (follow any tutorial out there) and leave MainWindow to deal with Window things. I'd then introduce a SingleInstanceApplicationLock class (pick a better name, nothing came to my mind in this moment). I'd like to use it like this:

public static void Main()
{
    using (var appLock = new SingleInstanceApplicationLock())
    {
        if (!appLock.TryAcquireExclusiveLock())
            return;

        var app = new App();
        app.InitializeComponent();
        app.Run();
    }
}

SingleInstanceApplicationLock will then be an internal sealed class implementing IDisposable (where you release resources you acquired). AcquireExclusiveLock() may even be lazy-load the mutex (I'd avoid to complicate things if unneeded but I like even less to put too much complex logic in constructors). Proof of concept:

sealed class SingleInstanceApplicationLock : IDisposable
{
    ~SingleInstanceApplicationLock()
    {
        Dispose(false);
    }

    void IDisposable.Dispose()
    {
        Dispose(true);
        GC.SuppressFinalize(this);
    }

    public bool TryAcquireExclusiveLock()
    {
        try
        {
            if (!_mutex.WaitOne(1000, false))
                return false;
        }
        catch (AbandonedMutexException)
        {
            // Abandoned mutex, just log? Multiple instances
            // may be executed in this condition...
        }

        return _hasAcquiredExclusiveLock = true;
    }

    private const string MutexId = @"Local\{1109F104-B4B4-4ED1-920C-F4D8EFE9E833}";
    private readonly Mutex _mutex = CreateMutex();
    private bool _hasAcquiredExclusiveLock, _disposed;

    private void Dispose(bool disposing)
    {
        if (disposing && !_disposed && _mutex != null)
        {
            try
            {
                if (_hasAcquiredExclusiveLock)
                    _mutex.ReleaseMutex();

               _mutex.Dispose();
            }
            finally
            {
                _disposed = true;
            }
        }
    }

    private static Mutex CreateMutex()
    {
        var sid = new SecurityIdentifier(WellKnownSidType.WorldSid, null);
        var allowEveryoneRule = new MutexAccessRule(sid,
            MutexRights.FullControl, AccessControlType.Allow);

        var securitySettings = new MutexSecurity();
        securitySettings.AddAccessRule(allowEveryoneRule);

        var mutex = new Mutex(false, MutexId);
        mutex.SetAccessControl(securitySettings);

        return mutex;
    }
}


Lucas Lehmer Primality Test I have coded the Lucas-Lehmer primality test following Wikipedia's description. I used the mod 2^n - 1 suggested in the article but was wondering if there were any other improvements I could make. I am using the GMP library for arbitrary precision integers. The program asks an integer n as input, then checks every Mersenne number M(i) for all i less than n. n = 10000 takes around 25 seconds on my computer.

#include <gmp.h>
#include <stdio.h>

_Bool mersenne_prime(unsigned long exponent) {
    mpz_t sequence, number, temp;
    // Intialize variables
    mpz_init_set_ui(sequence, 4);
    mpz_init(temp);
    // Set number to 2^n - 1
    mpz_init_set_ui(number, 1);
    mpz_ui_pow_ui(number, 2, exponent);
    mpz_sub_ui(number, number, 1);
    // Repeat exponent-2 times
    for (unsigned long counter = exponent; --counter - 1;) {
        mpz_mul(sequence, sequence, sequence);
        // Modulus suggested by wikipedia
        while (mpz_cmp(sequence, number) > 0) {
            // Most significant bits of sequence
            mpz_div_2exp(temp, sequence, exponent);
            // Least significant bits of sequence
            mpz_mod_2exp(sequence, sequence, exponent);
            mpz_add(sequence, sequence, temp);
        }
        // Erratic case
        if (mpz_cmp(number, sequence) == 0) {
          mpz_set_ui(sequence, 0);
        }
        mpz_sub_ui(sequence, sequence, 2);
    }
    // sequence == 0 means prime
    _Bool result = mpz_sgn(sequence) == 0;
    // Clear variables
    mpz_clears(sequence, number, temp, NULL);
    return result;
}

int main() {
    mpz_t num;
    unsigned long limit;
    mpz_init_set_ui(num, 3);
    // Get limit
    scanf("%lu", &limit);
    printf("Searching for Mersenne primes...\nM2 is prime!\n");
    while (mpz_cmp_ui(num, limit) < 0) {
        unsigned long num_ui = mpz_get_ui(num);
        if (mersenne_prime(num_ui)) {
            printf("M%lu is prime!\n", num_ui);
        }
        mpz_nextprime(num, num);
    }
    // Clear num
    mpz_clear(num);
    return 0;
}

The GMP code on RosettaCode is about 2x faster if you're looking for performance improvements.

It has a reasonable number of comments explaining the various optimizations.  Most of the help will be in the pre-tests, where we know easy ways to find out if the result will be composite.  The test itself is not too dissimilar, with a few different choices made.

I don't particularly like your for loop, which you wrote more like a while loop -- mixing the iteration and test together.


Computing value sums in a custom grid I have written a helper method which computes the sum of values in some custom grid, given the column indexes. The method appears to work (for a decimal - as Anthony pointed out, I need to test this further), but I am not at ease with the following conversions:

decimal valIfNull = (decimal)(object)valueIfNull;
return (T)(object)sum;

They seem too indirect, but Convert.ChangeType also returns an object type, so another cast is needed anyway. Also, I believe that Convert.ChangeType can convert a string such as "123.456" into a proper decimal value, but this is not the behavior I want in this case. I would like to see an invalid cast exception in that case. So, is what I have good enough, or is there a better way?

    private T SumColumns<T>(T valueIfNull, params short[] columnIndexes)
        where T : struct, IConvertible
    {
        // Since we do not know what exactly T is, we will compute things as if it was a decimal.
        // If this works reasonably well, then this trick should be used elsewhere.

        // I am not sure if this is the best way to go about casting
        decimal valIfNull = (decimal)(object)valueIfNull;

        // Here this.GetCurrentRowValue<decimal> is another generic function I wrote ...
        decimal sum = columnIndexes.Select(columnIndex => this.GetCurrentRowValue<decimal>(columnIndex: columnIndex, valueIfNull: valIfNull)).Sum();
        return (T)(object)sum;
    }

You've already placed a constraint on the first parameter that it should be IConvertible, so the methods of that interface is available to you.

decimal decimalValueIfNull = valueIfNull.ToDecimal(CultureInfo.InvariantCulture);

Converting back to the generic type won't be as straight forward.  When casting between an object and a value type, no conversions will be made and you are just boxing/unboxing the value.  Since you have a constraint that the type be a value type, the casts won't really work here.  In fact, I don't think it's even possible to do so generically.  If a conversion exists from decimal to the type you wouldn't be able to express that here.  And you won't be able to use the Convert class to do the conversion, there's no way to "add" conversions to existing convertible types.

Your best (and probably only) option would be to convert it to a dynamic variable and let the runtime figure out whether or not that conversion exists.  That way if you have a custom ValueType here that has an implicit or explicit conversion from a decimal, it will be used.

return (T)(dynamic)sum;

If you know for sure that the types that are used here can be converted from a decimal, then you could convert back using IConvertible.ToType().

return ((IConvertible)sum).ToType(typeof(T), CultureInfo.InvariantCulture);


Code minimizer - Remove unnecessary whitespace I often do golf challenges, and thought it might be advantageous to write some code that automatically removes all the unnecessary whitespace from a program.

After a few refactors, this is what I ended up with. In all the tests I've run it through, it's correctly removed only the whitespace that doesn't change the meaning of the program.

Example:

(minimize-code (fn [a] (+ a 1)))

"(fn[a](+ a 1))"

Limitations:

It doesn't handle line-comments well. It doesn't remove comments, so anything after the first ; is commented out.
It only removes unnecessary whitespace. Names still need to be manually shortened (maybe a later project).

What I what advice on:

Really, I'm looking for any advice. It actually ended up being much simpler than I anticipated, and I'm fairly happy with the end result. I'm sure there's room for improvement though!

(ns bits.golf.helpers.minimizer)

(def safe-neighbors
  "The characters that indicate neighboring spaces are safe to remove."
  (set "(){}[]\" \t\n"))

(def minimizer-output-path "./minimized-code.clj")

(defn can-remove-cur-char?
  "Returns whether or not a character is safe to remove.
   A character is considered unsafe to remove if it doesn't change the meaning of the program.
   Expects that if prev-char or next-char aren't available, nil will be passed."
  [prev-char cur-char next-char]
  (cond
    (not (Character/isWhitespace ^Character cur-char))
    false

    (or (nil? prev-char) (nil? next-char))
    true

    :else
    (or (safe-neighbors prev-char) (safe-neighbors next-char))))

(defn safe-remove-whitespace
  "Removes any whitespace characters that don't effect the meaning of the program."
  [code-str]
  (loop [[chr & rest-str] code-str
         acc ""]
    (if-not chr
      acc
      (let [prev-chr (if (empty? acc) nil (last acc))
            next-chr (if rest-str (first rest-str) nil)
            can-remove? (can-remove-cur-char? prev-chr chr next-chr)
            replacement-chr (if (Character/isWhitespace ^Character chr)
                              \space chr)]
        (recur rest-str
               (if can-remove? acc (str acc replacement-chr)))))))

(defn minimize-code*
  "Removes unnecessary whitespace and outputs it to minimizer-output-path, then returns the minimized code."
  [code-str]
  (let [minimized-code (safe-remove-whitespace code-str)]
    (spit minimizer-output-path minimized-code)
    minimized-code))

(defmacro minimize-code
  "Removes unneccesary whitespace and outputs it to minimizer-output-path, then returns the minimized code."
  [& body]
  (minimize-code* (apply str body)))

Here's something that gets the same answer using different techniques:

(def safe-neighbors
  (set "(){}[]\" \t\n"))

(defn whitesp? [ch]
  (Character/isWhitespace ^Character ch))

(defn del-white [s]
  (reduce #(if (and (whitesp? %2)
                    (whitesp? (last %1)))
             %1
             (str %1 %2)) "" s))

(defn squeezable? [[a b c]]
  (and (whitesp? b) (or (safe-neighbors a) (safe-neighbors c))))

(defn elide [triples]
  (let [squeezabilities (drop-last (cons true (map (complement squeezable?) triples)))]
    (->> (map vector squeezabilities triples)
         (filter first)
         (map (comp first second)))))

(defn squeeze-safe [s]
  (apply str (apply str (->> s (partition 3 1) elide)) (take-last 2 s)))

(defn minimize-code-2* [code-str]
  (->> code-str
       del-white
       squeeze-safe))

(defmacro minimize-code [& body]
  (minimize-code-2* (apply str body)))

(defn x-1 []
  (minimize-code (fn [a]    (+ a   1))))

In particular it removes recursion from being the crux of the code. Loop/recur recursion can solve most problems, but can quickly become liable to frailty and difficult to reason about as more and more loop variables are added.

Here the starting off point was to think that partition can be used to look at the neighbouring characters by only grabbing three characters at a time. Thus partition can replace recursion. You know exactly what is going on with partition, however recursion is less focused, takes more time to comprehend and is prone to atrophy.

Here removing white-space and seeing if the final squeeze(*) can be done are two different operations, done on different parses through the input. Thus when writing (and reading) the code you can concentrate on one simple thing at a time.

Here squeezable? can be replaced by can-remove-cur-char?, as the latter is almost certainly more well thought out than my effort!

(*) Squeeze as in squeeze out the unnecessary white-space between neighbouring characters


Function equivalent to strcmp() I'm starting out and need feedback in order to discover the things I'm doing wrong.

This is supposed to behave like strcmp():

int compare(char *str1, char *str2)
{
    while(*str1 || *str2)
    {
        if(*str1 != *str2)
        {
           break;
        }
        ++str1;
        ++str2;
    }
    return *str1 - *str2;
}

How can it be improved? Are there any flaws?

Also, would any of the following styles be considered better?

int compare(char *str1, char *str2)
{
    while((*str1 || *str2) && *str1 == *str2)
    {
        ++str1;
        ++str2;
    }
    return *str1 - *str2;
}

int compare(char *str1, char *str2)
{
    while((*str1 || *str2) && *str1 == *str2) ++str1, ++str2;
    return *str1 - *str2;
}

You can skip the check for the end of one of the strings. If the other string ends before the one that you check the length for, the comparison of the characters will catch the difference:

int compare(char *str1, char *str2) {
  while (*str1 && *str1 == *str2) {
    str1++;
    str2++;
  }
  return *str1 - *str2;
}

You can write the same using a for statement if you want it shorter (and less readable):

int compare(char *str1, char *str2) {
  for (;*str1 && *str1 == *str2; str2++) str1++;
  return *str1 - *str2;
}

Just for the sake of it, you can make is shorter (mis)using recursion, but that is a rather terrible solution:

int compare(char *str1, char *str2) {
  return (*str1 && *str1 == *str2) ? compare(++str1, ++str2) : *str1 - *str2;
}


Changing look and feel for all current frames I am using JGoodies Looks to change the look and feel of the application I'm currently working on.

I've just created a JMenu with some different themes options for the user to choose.

The thing is, I want to make sure the changes affect the program instantly, on all current windows, so I did this:

@Override
public void doActionPerformed(ActionEvent event) {
    PlasticLookAndFeel lookAndFeel = new PlasticXPLookAndFeel();
    lookAndFeel.setPlasticTheme(new SkyBlue());
    try {
        UIManager.setLookAndFeel(lookAndFeel);
        for(Frame f: Frame.getFrames()) {
            SwingUtilities.updateComponentTreeUI(f);
        }
    } catch (UnsupportedLookAndFeelException e) {
        e.printStackTrace();
    }
}

It works perfectly, like magic. But I don't know if it is a good practice or not, or if there is a better or easier (if possible) way.

I didn't even know that Frame had that method. I've just tried (thanks to IntelliJ magic code completion) and it worked so easy on the first try. So maybe that's why I'm concerned about it.

Also, what is the difference between the code above and this one (as they appear to work the exact same way):

for(Window window : JFrame.getWindows()) {
    SwingUtilities.updateComponentTreeUI(window);
}

I mean, when and why should I be using Frame over JFrame, Frame over Window, or vice versa?

Java GUIs: Swing & AWT

In standard Java there are two ways to make GUIs: Swing and AWT.

To be short, Swing is platform-independent and AWT is platform-dependent. Typically you pick either AWT or Swing and stick to it. I believe Swing is the more popular choice today as it is easier to work with.

You can tell Swing code from AWT code by the fact that Swing component classes are prefixed with a J. For example Frame is AWT and JFrame is the Swing equivalent.

The Swing components inherit from the AWT components so a JFrame is a Frame.

Windows and Frames

Now the difference between Window and Frame  (and JWindow and JFrame respectively) is that a Window is a plain window without any decorations; no borders, no title bar, no window management buttons. A Frame is a Window with all these decorations. And I put emphasis on is a in the above as Frame actually inherits Window.

So the inheritance tree looks like this:

          lang.Object
              |
           awt.Component
              |
           awt.Container
              |
           awt.Window
              |
     +--------+-------+
     |                |
swing.JWindow      awt.Frame
                      |
                 swing.JFrame

To answer your question about which of the methods is better. Note that getWindows() is defined in Window and inherited by JWindow, Frame and JFrame. Calling getWindows() will get all windows, even owner-less dialogues and system windows associated with the application, regardless of if they have decorations or not. On the other hand getFrames() is defined in Frame so calling getFrames() will get all windows with decorations (frames). If your application doesn't have any frame-less windows, the two pieces of code you posted will be equivalent.

The Code

Your code is likely fine as it is wither either approach as plain Windows are kind of rare and often transient. The approach you're using is the standard one.

If you want to be absolutely sure you get every window there is and be picky about it, this is how I would write it:

for(Window window: Window.getWindows()) {
    SwingUtilities.updateComponentTreeUI(window);
}

For your reference, see the Java Docs for JWindow  and JFrame and their super classes.


Can not clearing a local java.util.Vector cause a memory leak? In the code below, does not clearing or setting the local variable completed to null create the potential for a memory leak?

private Vector<Element> elements;

private void update()
{
    Vector<Element> completed = new Vector<Element>();
    for( Element e : elements )
    {
        if( e.isComplete() )
        {
           completed.add( e );
        } else {
           e.update();
        }
    }

    elements.removeAll( complete );
}

Should I add complete.clear() before the update() method exits?

The elements Vector is an instance variable on a class that is populated with Element objects elsewhere in the program.

Because complete is used and is referenced entirely within the context of the method update(), it will become unreferenced after the method returns.

This will eventually cause it to be garbage collected.

After it has been garbage collected, some or all of the elements that belong to it will also become unreferenced and then garbage collected.

So technically, no, it is not necessary to call complete.clear().

On the other hand, getting into the habit of insuring that things are properly cleaned up can be important in other circumstances. As such, it would not be wrong to add the clear() call to the end of the method. To do so, or not do so could be either a stylistic choice, or something to consider addressing in a coding standards document. I would probably add it myself.


jQuery on-change using multiple selectors What is the best way to refactor the following code to avoid duplication?

    // JQuery FORM Functions
$(document).on('change','.submittable',function(){
     //save current input field into hidden input 'focus_field' if it exists
        if($('input#focus_field').length){
            var $focus_field = $(this).attr('id')
            $('input#focus_field').val($focus_field)
        }
    $('input[type=submit]').click();
    return false;
});

$(document).on('change','.submittable_wait',function(){
    $('#please_wait').show();
    $('input[type=submit]').click();
    return false;
});

$(document).on('change','.submittable_wait_bar',function(){
    $('#ajax_working').show();
    $('input[type=submit]').click();
    return false;
});

$(document).on('change','.submittable_wait_bar_single',function(){
    $('#ajax_working').show();
    $( this ).closest("form").submit();
    return false;
});

// nested_form - fields added
$(document).on('nested:fieldAdded', function(event) {
    $('#ajax_working').show();
    $('#reload_form').val('true');
    $('input[type=submit].btn-primary').click()
});

// nested_form - fields removed
$(document).on('nested:fieldRemoved', function(event) {
    $('#please_wait').show();
    $('#reload_form').val('true');
    $('input[type=submit].btn-primary').click()
});

Some general points:

Watch your indentation
Watch your whitespace
An id is unique on a page - there is no benefit to 'element#id' vs '#id' as a selector
It is usually better to store references to elements that don't get removed

Let's apply some of these on the change handler to store the focus field. Here's what we're starting with:

$(document).on('change','.submittable',function(){
    if($('input#focus_field').length){
        var $focus_field = $(this).attr('id')
        $('input#focus_field').val($focus_field)
    }
    $('input[type=submit]').click();
    return false;
});

We don't want to search the DOM for the element every time (unless it can be added/removed) so let's pull it out:

var $focusField = $('#focus_field');

Notice that I've simplified the selector to just the Id. Now we can use our knowledge of jQuery to simplify the next bit of the function.

$(document).on('change', '.submittable', function(e) {
     e.preventDefault();
     $focusField.val(this.id);
     $('input[type=submit]').click();
}

The reason this works is that selecting a set of elements that don't exist in the DOM returns an empty jQuery array which is more than happy to be chained - it just won't do anything. I've also modified your return false to use the recommended preventDefault function on the event object.

If you want you can pull out common functionality into separate functions. For example:

var showAndReload = function (selector) {
    $(selector).show();
    $('#reload_form').val('true');
    $('input[type=submit].btn-primary').click()
};

// nested_form - fields added
$document.on('nested:fieldAdded', function() {
    showAndReload('#ajax_working')
});

// nested_form - fields removed
$document.on('nested:fieldRemoved', function() {
    showAndReload('#please_wait');
});

I'm not really sure it adds much though.


Searching faster for a file on the drive using Python I am trying to write a code that should look on the laptop for a file, open it, if found, and read the context of that file (it should be .txt or .doc). So far, I wrote this code which is working:
def search(drive, target):
    file_path = ''
    if drive == "D":
        for root, dirs, files in os.walk(r"D:\\"):
            for name in files:
                if name == target:
                    file_path = os.path.abspath(os.path.join(root, name))
    elif drive == "C":
        for root, dirs, files in os.walk(r"C:\\"):
            for name in files:
                if name == target:
                    file_path = os.path.abspath(os.path.join(root, name))
    return file_path

def path():
    file_name = input("Enter the name of the desired file: ")
    path = os.path.abspath(file_name)
    file_path = search(path[0], file_name)
    return file_path

The problem is that this piece of code is very slow, as it has to look on each directory on the drive (the only trick I used to save some time was to take the abspath and to look on what drive it is saved, thus it doesn't look on each existing drive).
I am thinking of using multiprocessing, but I do not know how to use it. Can anybody help? Thank you very much for your time!

Since you are already requiring the filename to start with the correct drive name, your target will already look like a normal valid path. So you could at least check if what the user entered is actually the correct path, short-circuiting the whole search:
def search(drive, target):
    if os.path.isfile(target):
        return os.path.abspath(target)
    ...

You should also stop looking as soon as you have found your target and use the in keyword instead of manually checking all files yourself:
    ...
    for root, dirs, files in os.walk(r"D:\\"):
        if target in files:
            return os.path.abspath(os.path.join(root, target))

Other than that, I am afraid you are mostly out of luck. There are only two ways to quickly find something in a big pile of somethings:

You search through all somethings until you find the something you are looking for.
You have a datastructure, called an index, which tells you how to find each (or at least some, including hopefully the one something you are looking for) of the somethings.

In the Unix world there is the command line tool locate, which uses a periodically updated database of files to make finding files fast (as long as they are in that database). I am not sure what the equivalent Windows (command line) tool is, though you may find some solution here. You can then call that tool using subprocess.
Of yourse, if you need this search very often (twice is probably already often enough), you could also built such an index yourself, meaning you only have to check all files once:
from collections import defaultdict

INDEX = {}

def build_index(drive):
    index = defaultdict(set)
    for root, _, files in os.walk(drive):
        for file in files:
            index[file].add(root)
    return dict(index)

def search(drive, target):
    if drive not in INDEX:
        INDEX[drive] = build_index(drive)
    root = next(iter(INDEX[drive][target]))
    return os.path.abspath(os.path.join(root, target))

Note that this arbitrarily chooses one result if a file name appears multiple times, similarly to your code which chooses the last one encountered (with the order of processing arbitrarily defined). On my machine, for example, there are 8708 files called index.js and it is not clear which one you want. It might be a better interface to return all of them instead of one arbitrary one.


Checking a board to see if a player has three in a row The code checks many more conditions like the one below. I was thinking to memoize it, but I can't think about how (writers block). How else could I optimize this? I know it seems silly, but my code is spending the majority of the time in the function.

def has_three(p, board):
    """Checks if player p has three in a row"""
    # For every position on the board
    for i in xrange(6):
        for j in xrange(7):
            if board[i][j] == p:
                if i<=2 and board[i+1][j]==p and board[i+2][j]==p and board[i+3][j]==0:
                    return True
                if i>=3 and board[i-1][j]==p and board[i-2][j]==p and board[i-3][j]==0:
                    return True
                if j<=3 and board[i][j+1]==p and board[i][j+2]==p and board[i][j+3]==0:
                    return True
                if j>=3 and board[i][j-1]==p and board[i][j-2]==p and board[i][j-3]==0:
                    return True
                if i<=2 and j<=3 and board[i+1][j+1]==p and board[i+2][j+2]==p and board[i+3][j+3]==0:
                    return True
                if i<=2 and j>=3 and board[i+1][j-1]==p and board[i+2][j-2]==p and board[i+3][j-3]==0:
                    return True
                if i>=3 and j<=3 and board[i-1][j+1]==p and board[i-2][j+2]==p and board[i-3][j+3]==0:
                    return True
                if i>=3 and j>=3 and board[i-1][j-1]==p and board[i-2][j-2]==p and board[i-3][j-3]==0:
                    return True
    return False

We really need to know more about the game to help you properly. Also, why is your program spending so much time in this function? (Are you doing some kind of lookahead, with this function being part of the evaluation?)

If this is a game like gomoku, where players take turns, and in each turn a player places one piece on the board, and the first player to get n in a line wins, then the only way that the board can have a winning line is if that line includes the piece that was just played. Hence there's no point looking at other points in the board.

So in this case you'd write something like this:

DIRECTIONS = [(1,0),(1,1),(0,1),(-1,1)]

def legal_position(i, j, board):
    """Return True if position (i, j) is a legal position on 'board'."""
    return 0 <= i < len(board) and 0 <= j < len(board[0])

def winning_move(player, move, board, n = 3):
    """Return True if 'move' is part of a line of length 'n' or longer for 'player'."""
    for di, dj in DIRECTIONS:
        line = 0
        for sign in (-1, 1):
            i, j = move
            while legal_position(i, j, board) and board[i][j] == player:
                i += sign * di
                j += sign * dj
                line += 1
        if line > n: # move was counted twice
            return True
    return False

But without understanding the rules of your game, it's impossible for me to know if this approach makes any sense.


Check if two binary trees are equal I've recently solved the LeetCode's "Same Tree" problem:

  Given two binary trees, write a function to check if they are equal or
  not.

  Two binary trees are considered equal if they are structurally
  identical and the nodes have the same value.

The problem itself and the idea is simple: traverse the tree in a way that preserves the structure - returning None for non-existing left or right sub-tree.

Here is the complete working code:

from itertools import izip

def traverse(node):
    if not node:
        yield None
    else:
        yield node.val
        if node.left:
            for val in traverse(node.left):
                yield val
        else:
            yield None
        if node.right:
            for val in traverse(node.right):
                yield val
        else:
            yield None

class Solution(object):
    def isSameTree(self, p, q):
        """
        :type p: TreeNode
        :type q: TreeNode
        :rtype: bool
        """
        try:
            for node_p, node_q in izip(traverse(p), traverse(q)):
                if node_p != node_q:
                    return False
        except StopIteration:
            return True

        return True

It was accepted by the Online Judge, but the code looks a bit lengthy. Is there a way to further simplify or improve the solution? Also, is it the most optimal way to solve the problem?

Note that I cannot use yield from since the OJ is Python 2.7 only.

FYI, here is what LeetCode uses as a Tree/Node:

# Definition for a binary tree node.
class TreeNode(object):
    def __init__(self, x):
        self.val = x
        self.left = None
        self.right = None

Exit the check early

Basically, you have a recursive traversal operation that traverses the two input trees sort of in parallel. The advantage of that approach is that you may exit algorithm as soon as you get to a pair of corresponding nodes with different values:

def bst_trees_are_identical_impl(p, q):
    if not p and not q:
        return True
    if p and q:
        if p.val != q.val:
            return False
        if not bst_trees_are_identical_impl(p.left, q.left):
            return False
        if not bst_trees_are_identical_impl(p.right, q.right):
            return False
        return True
    return False

def bst_trees_are_identical(p, q):
    return bst_trees_are_identical_impl(p, q)

class Solution(object):
    def isSameTree(self, p, q):
        """
        :type p: TreeNode
        :type q: TreeNode
        :rtype: bool
        """
        return bst_trees_are_identical(p, q)

Hope that helps.


Improving efficiency of download code I'm in the process of reviewing some old code released by an old colleague.
We have a cronjob that executes once in an hour to download some thumbnails: the paths to the thumbnails are stored in a plain array.
// $totalThumbs is usually less than 2000 (each about 3kb)
while ($i < $totalThumbs) {

  $imgName = basename($thumbs_array[$i]);

  $fgc = file_get_contents($thumbs_array[$i]);

  $currentFile = __DIR__ . "/" . $imgName;

  // if file_get_contents doesn't return false
  if ($fgc !== false) {

    // if the file is not in that directory, write the file
    if (!file_exists($currentFile)) {

      file_put_contents($currentFile, $fgc);
      clearstatcache(true, $currentFile);

    }

  }

  $i++;

  sleep(1);
}

This code works but, for example, we can't use CURL multi_exec because of the limited resources of our server.
Is there a way to improve it (more efficient and/or more secure), considering our hardware limits? We don't need speed, but eventually less memory consumption because the same server is at the same time busy with many other 'jobs'.
Thanks
EDIT (for Mast): one important thing to say is the current idea is to remove this part and use a cronjob to directly store an array in a file, so that the file we are talking about has only to read that array
$dir = "https://mylocaldir";
$thumbs_array = [];

// this one returns JSONP
$raw_json = 'https://endpoint';
$json = file_get_contents($raw_json);

// JSONP removal
$json = str_replace("jsonOutreachFeed(", "", $json);
$json = substr($json, 0, -1);

$decoded_json = json_decode($json);

$itm = $decoded_json->items;

$totalThumbs = count($itm);

for ($i = 0; $i < $totalThumbs; $i++) {
  $thumbs_array[] = $itm[$i]->media->m;
}

Since the code is only reading the contents of the file to see if there are contents in the file, an optimization for both speed and memory usage would be to use the filesize() function and not read the contents. Reading the contents of the file consumes both time and memory.
Rather than writing the contents read in, copy or move the original file to the new location using either rename() or move_uploaded_file().


Object-Oriented FizzBuzz in Ruby I don't use Ruby on a regular basis, but I felt inspired to write an extensible, object-oriented version of FizzBuzz to sharpen my skills. I welcome all feedback.

The main class:

class FizzBuzzer
  def initialize(*tests)
    @tests = tests
  end

  def run(values)
    result = []
    values.each do |value|
      str = ''
      @tests.each do |test|
        str << test.word if test.satisfied_by? value
      end
      str = value.to_s if str.empty?
      result << "#{str}\n"
    end
    result.join
  end
end

Sample test operation classes:

class ModuloTester
  attr_accessor :word

  def initialize(modulo, word)
    @modulo = modulo.to_i
    @word = word
  end

  def satisfied_by?(number)
    (number % @modulo).zero?
  end
end

class SquareRootTester
  attr_accessor :word

  def initialize(word)
    @word = word
  end

  def satisfied_by?(number)
    (Math.sqrt(number) % 1).zero?
  end
end

In use:

fizz_buzzer = FizzBuzzer.new ModuloTester.new(3, 'Fizz'), ModuloTester.new(5, 'Buzz')
puts fizz_buzzer.run 1..100

puts

fizz_buzzer = FizzBuzzer.new SquareRootTester.new('(PerfectSquare)')
puts fizz_buzzer.run 1..10
puts fizz_buzzer.run 20..30
puts fizz_buzzer.run 45..55

Which produces the expected output. Is this idiomatic Ruby? Is the naming scheme acceptable? What can be improved?

I'd like to suggest two changes to your interface.

Implement Enumerable.  Your object accepts an Enumerable; it would be nice if it also acted as an Enumerable.  All you have to do is include Enumerable and rename run → each, and then it behaves more like the way a Rubyist would like to see.
Remove the separate calls to #satisfied_by? and #word.  I find it awkward that FizzBuzzer should have to know about the workings of the tester objects like that.  Why not just have the tester return a truthy output value to indicate that it has performed a transformation?

In addition, I would prefer to see the invocations of FizzBuzzer.new and fizz_buzzer.run written with parentheses, for readability.  There appears to be a consensus among multiple Ruby style guides that parentheses should only be omitted for simpler or specialized calls such as puts, include, attr_accessor, or methods that take no arguments.

class FizzBuzz
  include Enumerable

  def initialize(*transforms)
    @transforms = transforms
  end

  def each(values=1..Float::INFINITY)
    values.each do |value|
      words = @transforms.map { |transform| transform.transform(value) }
      yield words.any? ? words.join('') : value.to_s
    end
  end
end

class ModuloTransform
  def initialize(modulo, word)
    @modulo = modulo.to_i
    @word = word
  end

  def transform(number)
    (number % @modulo).zero? ? @word : nil
  end
end

class SquareRootTransform
  def initialize(word)
    @word = word
  end

  def transform(number)
    (Math.sqrt(number) % 1).zero? ? @word : nil
  end
end

fb = FizzBuzz.new(ModuloTransform.new(3, 'Fizz'),
                  ModuloTransform.new(5, 'Buzz'))
# FizzBuzz naturally works from 1 to infinity.  This is one way to limit
# the results to the first 100.
fb.take(100).each { |word| puts word }

fb = FizzBuzz.new(SquareRootTransform.new('(PerfectSquare)'))
# This is another way to do FizzBuzz only for certain values.
fb.each(1..100) { |word| puts word }


Finding the person with the biggest loss For a course I following and my code is approved I had to make some code that finds the person with the biggest loss where the input is a string[]

The string arrray looks like this :

{ "Tom, 1, 3, -1", "Gillie, 2, 3, 1", "Thor, 1000, 1001, 1002" };

I solved it this way but it's not a clean way. I must say we were not allowed to use classes. that will be the next chapter of the course.

My solution looks now like this :

 public static string FindPersonWithBiggestLoss(string[] peopleAndBalances)
        {
            if (IsInValidInput(peopleAndBalances))
            {
                return InValidOutput;
            }

            var highestLossEver = decimal.MinValue;
            var personWithHighestLoss = new StringBuilder();

            for (int i = 0; i <= peopleAndBalances.Length - 1; i++)
            {
                var currentPersonData = peopleAndBalances[i].Split(',');
                var allAmounts = currentPersonData[1..];

                // calculateLoss

                if (allAmounts.Length <= 1)
                {
                    return InValidOutput;
                }

                for (int j = 0; j < allAmounts.Length - 1; j++)
                {
                    try
                    {
                        var amount1 = decimal.Parse(allAmounts[j], NumberStyles.Currency | NumberStyles.Number);
                        var amount2 = decimal.Parse(allAmounts[j + 1], NumberStyles.Currency | NumberStyles.Number);
                        var lossForCurrentPerson = amount1 - amount2;

                        //check if loss is greater then the current highest loss
                        if (lossForCurrentPerson > highestLossEver)
                        {
                            highestLossEver = lossForCurrentPerson;
                            personWithHighestLoss.Clear();
                            personWithHighestLoss.Append(currentPersonData[0]);
                        }
                    }
                    catch (FormatException ex)
                    {
                        Console.WriteLine($"there is not a valid character in the database. Reason{ex}");
                        break;
                    }
                }
            }

            if (personWithHighestLoss.ToString().Contains(','))
            {
                ReplaceCommaWithAnd(personWithHighestLoss);
            }

            return $"{personWithHighestLoss} lost the most money. -¤{highestLossEver}.";
    }

it works but also I need a lot of nested control statements to make it work.
Is there a cleaner way to make this work ?

A few things I noticed:

Using the range option of the array index(var allAmounts = currentPersonData[1..];) doesn't really help here.  All that's needed is to use the appropriate indexes of the original array.

It's quite inefficient to use a try block to test if a string is a valid number.  Using the tryParse method is much more efficient.

A hard coded currency symbol means that your output might not format properly on someone else's machine.  I would suggest using the format overload of the ToString method with the currency format.

I appears you're using a StringBuilder to change the name of whichever person has the highest loss.  If I'm not mistaken this will copy the string you already have.  I would suggest just use the string array itself.  This will be assigned by reference and won't require recopying the string.
This:

if (personWithHighestLoss[0].Contains(','))
{
    ReplaceCommaWithAnd(personWithHighestLoss);
}

doesn't really make much sense.  The Split method already removes the commas.  If you're worried about extra commas use the StringSplitOptions.RemoveEmptyEntries option of the Split method.

Here's what the modified code could look like:

public static string FindPersonWithBiggestLoss(string[] peopleAndBalances)
{
    if (IsInValidInput(peopleAndBalances))
    {
        return InValidInput;
    }

    var highestLossEver = decimal.MinValue;
    string[] personWithHighestLoss = new string[] { "" };

    for (int i = 0; i <= peopleAndBalances.Length - 1; i++)
    {
        var currentPersonData = peopleAndBalances[i].Split(',',StringSplitOptions.RemoveEmptyEntries);

        // calculateLoss

        if (currentPersonData.Length < 4)
        {
            return InValidInput;
        }

        for (int j = 1; j < currentPersonData.Length - 1; j++)
        {
            var amount1 = 0.0M;
            var amount2 = 0.0M;
            if (!(decimal.TryParse(currentPersonData[j], NumberStyles.Currency | NumberStyles.Number, null, out amount1) &&
                decimal.TryParse(currentPersonData[j + 1], NumberStyles.Currency | NumberStyles.Number, null, out amount2)))
            {
                Console.WriteLine($"there is not a valid character in the database");
            }

            var lossForCurrentPerson = amount1 - amount2;

            //check if loss is greater then the current highest loss
            if (lossForCurrentPerson > highestLossEver)
            {
                highestLossEver = lossForCurrentPerson;
                personWithHighestLoss = currentPersonData;
            }
        }

    }
    return $"{personWithHighestLoss[0]} lost the most money. -{highestLossEver.ToString("C")}.";
}


Save on typing while using UTF8 encoding Typing in something like Encoding.UTF8.GetString(...) and Encoding.UTF8.GetBytes(...) everywhere in your code could be eliminated by a helper UTF8 type:

public class UTF8_Should
{
    [Test]
    public void Convert()
    {
        var text = "Hello World";
        byte[] array = (UTF8)text;
        string copy = (UTF8)array;
        Assert.AreEqual(text, copy);
    }
}

Where:

struct UTF8
{
    public static implicit operator UTF8(byte[] array) => new UTF8(Encoding.UTF8.GetString(array));
    public static implicit operator string(UTF8 utf8) => utf8.Text;
    public static implicit operator UTF8(string text) => new UTF8(text);
    public static implicit operator byte[](UTF8 utf8) => Encoding.UTF8.GetBytes(utf8.Text);
    public UTF8(string text) => Text = text;
    string Text { get; }
}

You don't care about null references?

IMO you create an object in order just to do something, that is more suitable for the concept of extensions:

  public static class StringExtensions
  {
    public static byte[] ToUTF8Bytes(this string text)
    {
      return Encoding.UTF8.GetBytes(text);
    }

    public static string ToUTF8(this byte[] bytes)
    {
      return Encoding.UTF8.GetString(bytes);
    }
  }

  Assert.AreEqual(text, text.ToUTF8Bytes().ToUTF8());

The benefit of the extension methods is that you don't have to remember that you've created the UTF8 struct somewhere, because it will show up in the intellisence.


View_messages "class" /**
 *    View_message
 *    Object Model
 */

var View_message = function(div)
{
    this.div = document.getElementById(div);
};

View_message.prototype.messages =
{
    empty: 'Please complete all fields',
    empty_bm: 'Please enter both a title and url',
    name: 'Only letters or dashes for the name field',
    email: 'Please enter a valid email',
    same: 'Please make emails equal',
    taken: 'Sorry that email is taken',
    pass: 'Please enter a valid password, 6-40 characters',
    validate: 'Please contact support to reset your password',
    url:  'Pleae enter a valid url'
};

View_message.prototype.display = function(type)
{
    this.div.innerHTML = this.messages[type];
};

And the Call

obj_view = new View_message('test_id');
obj_view.display('empty');

I believe you are overcomplicating things. Declaring a prototype, will not make your code any more robust or even faster. I'm sure you know, but actually something like the code below will be easier to read, more effective and also faster.

/**
 *    View_message
 *    Object Model
 */
var ViewMessage = function(div) {
    this.div = document.getElementById(div);

    this.messages = {
        empty: 'Please complete all fields',
        empty_bm: 'Please enter both a title and url',
        name: 'Only letters or dashes for the name field',
        email: 'Please enter a valid email',
        same: 'Please make emails equal',
        taken: 'Sorry that email is taken',
        pass: 'Please enter a valid password, 6-40 characters',
        validate: 'Please contact support to reset your password',
        url:  'Pleae enter a valid url'
    };

    this.display = function(type) {
        this.div.innerHTML = this.messages[type];

        return this;
    };

    return this;
};

Then two ways of doing this:

var view = new ViewMessage('divID');
    view.display('email');

OR

var view = new ViewMessage('divID').display('email');