My own Stack Implementation in Java Please review my Stack implementation and let me know if you have any suggestions.

public class StackImplementation<T> {
private StackNode<T> top;
private StackNode<T> head;
private int size;

static class StackNode<T> {
    private T value;
    private StackNode<T> next;

    public StackNode(T val) {
        this.value = val;
        this.next = null;
    }

    public T getValue() {
        return value;
    }

    public StackNode<T> getNext() {
        return next;
    }

    public void setNext(StackNode<T> nextNode) {
        this.next = nextNode;
    }

    public String toString() {
        if(value == null) {
            return "null";
        }
        return value.toString();
    }
}

public StackImplementation() {
    head = new StackNode(0);
    size = 0;
    top = head;
}

public void push(T val) {
    StackNode<T> cur = new StackNode<T>(val);
    cur.setNext(top);
    top = cur;
    size++;
    return;
}

public StackNode<T> peek() {
    return top;
}

public T pop() {
    if(size == 0 || top.getNext() == null) {
        throw new java.util.NoSuchElementException();
    }

    T poppedElement = peek().getValue();
    this.top = this.top.getNext();
    size--;
    return poppedElement;
}

public void clear() {
    head.setNext(null);
    top = head;
    size = 0;
}

public int size() {
    return size;
}

public boolean isEmpty() {
    return size == 0;
}

public String toString() {
    StringBuilder sb = new StringBuilder();
    StackNode<T> cur = head.getNext();
    sb.append(cur.getValue() + "->");
    while(!cur.equals(top)) {
        sb.append(cur.getValue() + "->");
    }
    sb.append("null");
    return sb.toString();
}

public static void main(String[] args) {
    StackImplementation<Integer> st = new StackImplementation<>();
    System.out.println(st.size());
    //st.pop(); //NoSuchElementException
    st.push(1);
    st.push(10);
    st.push(100);
    System.out.println(st.size());
    System.out.println(st.peek());
    System.out.println(st.pop());
    System.out.println(st.peek());
    System.out.println(st.pop());
    System.out.println(st.isEmpty());
    st.clear();
    System.out.println(st.isEmpty());
    System.out.println(st.size());
    //st.pop(); //NoSuchElementException
}}

I have added comments directly in the code whenever I have something to say.

import java.util.EmptyStackException;
import java.util.Objects;

// 'Implementation' does not add any information.
//public class StackImplementation<T> {
public class Stack<T> {

    private StackNode<T> top;
    // Not really needing 'head':
    // private StackNode<T> head;
    private int size;

    // Added 'private'. Now even the code in the same package cannot get to
    // 'StackNode':
    private static class StackNode<T> {

        private T value;
        private StackNode<T> next;

        // Really do not need 'public' since this class is visible ONLY in the
        // 'Stack':
        /*public*/ StackNode(T val) {
            this.value = val;
            // JVM already initializes reference fields to 'null' by default:
            // this.next = null;
        }

        // Same argument against use of 'public' as in the constructor:
        /*public*/ T getValue() {
            return value;
        }

        /*public*/ StackNode<T> getNext() {
            return next;
        }

        /*public*/ void setNext(StackNode<T> nextNode) {
            this.next = nextNode;
        }

        // '@Override' would not hurt here. If you somehow mistyped the name of
        // the method being override, and it has a @Override, compiler will
        // notify you:
        @Override
        public String toString() {
            // You can do just:
            return Objects.toString(value);
            // if (value == null) {
            //     return "null";
            // }
            // return value.toString();
        }
    }

    // In this case you can omit the constructor altogether. JVM initializes
    // reference fields to 'null' by default, and integer fields to zero by
    // default:
    // public StackImplementation() {
    //     head = new StackNode(0);
    //     size = 0;
    //     top = head;
    // }

    public void push(T val) {
        // Since Java 7, you can do "diamond inference":
        StackNode<T> cur = new StackNode<>(val);
        // StackNode<T> cur = new StackNode<T>(val);
        cur.setNext(top);
        top = cur;
        size++;
        // You can remove the 'return' whenever it is the very last statement of
        // a method:
        // return;
    }

    // peek() does not return the topmost node but rather its value:
    public T peek() {
        checkNotEmpty();
        return top.getValue();
    }

    public T pop() {
        checkNotEmpty();
        // if (size == 0 || top.getNext() == null) {
        //     throw new java.util.NoSuchElementException();
        // }

        T poppedElement = top.getValue();
        // Can omit 'this' here:
        top = top.getNext();
        // this.top = this.top.getNext();
        size--;
        return poppedElement;
    }

    public void clear() {
        top = null;
        size = 0;
        // The rest is up to garbage collector.
    }

    public int size() {
        return size;
    }

    public boolean isEmpty() {
        return size == 0;
    }

    @Override
    public String toString() {
        if (size == 0) {
            return "[]";
        }

        StringBuilder sb = new StringBuilder("[").append(top.getValue());
        StackNode<T> node = top.getNext();

        while (node != null) {
            sb.append(" -> ").append(node.getValue());
            node = node.getNext();
        }

        return sb.append("]").toString();
    }

    private void checkNotEmpty() {
        if (size == 0) {
            throw new EmptyStackException();
        }
    }

    public static void main(String[] args) {
        Stack<Integer> st = new Stack<>();
        System.out.println(st.size());
        st.push(1);
        st.push(10);
        st.push(100);
        st.push(null);
        System.out.println(st);
        System.out.println(st.size());
        System.out.println(st.peek());
        System.out.println(st.pop());
        System.out.println(st.peek());
        System.out.println(st.pop());
        System.out.println(st.isEmpty());
        st.clear();
        System.out.println(st.isEmpty());
        System.out.println(st.size());
        st.peek(); // Exception!
    }
}

Hope that helps!


Return Y integer with same bit sets as X & having | X - Y | minimum value I happened to come across one question:

  Given an integer 'x' (Only positive), write an algorithm that returns
  integer 'y' such that it has the same number of bits set as 'x' and is
  not equal to 'x' and the distance |x-y| is minimized.

  Example:

x: 01
y: 10

Now I wrote some bit manipulation as follows and it's working correctly. But I wanted to know if there is a more compact algorithm (we are not allowed to use any built-in functions).

  Algorithm

  If the Zeroth bit '0':

  find first occurrence of '1'
  find last occurrence of '0' before first '1'
  reverse them (I used OR and AND function to do that)

  If the Zeroth bit '1':

  find first occurrence of '0'
  find last occurrence of '1' before first '0'
  reverse them (I used OR and AND function to do that)

  Input:

x = 324095 <1001111000111111111>

  Output:

324351 <1001111001011111111>

  Input:

x = 12 <1100>

  Output:

10 <1010>

private static int MinDiff(int x)
{
    int y=0,a=0,b=0,countOne=0, countZero=0;
    if(x == 0)
        return -1;

    if( (x&1)!=1)
    {
        y=x;
        while( (y & 1) !=1  )
        {
            countOne++;
            y=y>>1;
        }
        y=x;
        while( (y&3) != 2 )
        {
            countZero++;
            y=y>>1;
        }
        a=1;
        while(countZero != 0)
        {
            a = a<<1;
            countZero--;
        }
        b=1;
        while(countOne != 0)
        {
            b = b<<1;
            countOne--;
        }
    }
    else
    {
        y=x;
        while( (y&1) != 0  )
        {
            countOne++;
            countZero++;
            y=y>>1;
        }
        countOne--;
        y=x;

        a=1;
        while(countZero != 0)
        {
            a = a<<1;
            countZero--;
        }
        b=1;
        while(countOne != 0)
        {
            b = b<<1;
            countOne--;
        }
    }

    b = ~b;
    y = (x | a) & b;
    return y;
}

Your algorithm is logical, but a little trick can make it much simpler too. Because applying the trick makes the code so much shorter, it seems better to just point out some smaller issues in your code, and then show the improved algorithm later....

So, first up.... the method name: MinDiff .... really? Java convention is to have camelCase method names, not PascalCase. The first letter should be a lower-case.

Your variable names also are horrible.... y, x, a, and b. With x and y variables I am always looking for cartesian coordinates or something. Choose more meaningful names.

Your handling of 0 and -1 is also broken. Since there are no bits set in 0, and no unset bits in -1, they are impossible to provide a solution for, so the best thing would be to handle them as exceptions. Returning -1 for an input of 0 is broken, and overflowing on the input of -1 is inconsistent too.

So, having said that, the best algorithm to use is to identify the right-most change in bit values... where bitn and bitn-1 are different. Then, just swap them. A swap is easy to do with and XOR:

public static final int leastBitFlip(final int input) {
    if (input == 0 || input == -1) {
        throw new IllegalArgumentException("0 or -1 are not possible to compute as inputs");
    }
    int mask = 0b11;
    while (sameBits(input, mask)) {
        mask <<= 1;
    }
    // swap the bits.
    return input ^ mask;
}

private static boolean sameBits(final int input, final int mask) {
    final int bits = input & mask;
    final int swap = mask & ((bits << 1) | (bits >> 1));
    return bits == swap;
}

Now, as for determining whether two bits are the same, or not, I think that sameBits() function can be improved, but I have not really run the truth table on it. The code is simple enough to understand though, and readability is important in these things.

Using the hint from ErikR - this algorithm would be faster though too:

public static final int leastBitFlip(int input) {
    if (input == 0 || input == -1) {
        throw new IllegalArgumentException("0 or -1 are not possible to compute as inputs");
    }
    // right-most one-bit.
    int onebit = ((input ^ (input - 1)) + 1) >> 1;
    // right most zero-bit.
    int zerobit = ((input ^ (input + 1)) + 1) >> 1;
    // where the change happens
    int change = onebit > zerobit ? onebit : zerobit;
    // create a mask of the two bits either side of the change.
    int mask = change | (change >> 1);
    // swap them
    return input ^ mask;
}


Erlang: pmap with max number of processes I'm working through Joe Armstrong's Programming Erlang 2nd E. The book has exercises at the end of each chapter. Chapter 26, Question 5 is:

  Write a function called pmap(F, L, Max) that computes the list [F(I) || I <- L] in parallel but is subject to the restriction that no more than Max parallel processes run simultaneously.

My solution is:

-module(pmap_cap).
-export([pmap/3]).

pmap(F, L, Max) ->
    S = self(),
    Ref = make_ref(),
    Pids = lists:map(fun(SubL) ->
                             spawn(fun() -> do_f(S, Ref, F, SubL) end)
                     end,
                     partition(L, min(length(L), Max))),
    gather(Pids, Ref).

partition(L, N) ->
    M = length(L),
    if
        M =< N -> lists:map(fun(X) -> [X] end, L);
        true -> partition(L, M div N, M rem N)
    end.

partition([], _Q, _R) -> [];
partition(L, Q, R) ->
    Extra = if R > 0 -> 1; true -> 0 end,
    [lists:sublist(L, Q + Extra)|
     partition(lists:sublist(L, Q + Extra + 1, length(L)), Q, R-1)].

do_f(Parent, Ref, F, SubL) ->
    Parent ! {self(), Ref, lists:map(fun(X) -> catch F(X) end, SubL)}.

gather([Pid|T], Ref) ->
    receive
        {Pid, Ref, Ret} ->
            lists:append(Ret, gather(T, Ref))
    end;
gather([], _) -> [].

What could be improved? Are there performance issues? Can the code be written more idiomatically?

General

Use standard library as many as possible, usually it given better result therefore function partition can be easily rewritting with wrapper on lists:split/2 and it will be consider as perfomance improvement

split(List,Max)->
    L = length(List),
    split_by(List,L rem Max + L div Max,[]).

split_by([],_N,R)->lists:reverse(R);
split_by(List,N,R)->
    {Part,NewList} = lists:split(N,List),
    split_by(NewList,N,[Part|R]).

Idiomaticness

Add to head of list and reverse instead append. For explanation - SO question.
Tail recursion better for readability and may be faster, so function gather become:

gather([Pid|T], Ref,R) ->
    receive
        {Pid, Ref, Ret} -> gather(T, Ref,[Ret|R])
    end;
gather([], _,R) -> lists:flatten(lists:reverse(R)).

List comprehensions instead lists:map/2:

Min = min(length(L), Max),
Pids = [spawn(
    fun() -> do_f(S, Ref, F, SubL) end) || SubL <- split(L, Min)],
...


Method to generate a List containing permutation of an input string I'm working to understand Big O notation. As such I'm going through examples and the algorithm below generates the permutations for a given input string which contains only distinct characters. IE no duplicated or repeated characters in the input string.

My thought process was to take each character and place it at every position within any previously generated strings.

The single character string "a" can only generate "a".
With "ab" the first character generates "a", as above. The next character 'b' would then be added before and after it to generate "ba" and "ab".
With "abc" this would effectively take the list generated above and repeat the same procedure placing c at position 0, 1, and 2 producing

From "ba": "cba", "bca", "bac"
From "ab": "cab", "acb", "abc"

Etc...

The nested loops, foreach and for, is a big flag as to how inefficient my current implementation is, \$O(n * n * n) = O(n^3)\$ if I've understood the notation correctly. How can I improve the structure to remove this nesting?

public class StringPermutation
{
    public List<string> Generate(string source)
    {
        var list = new List<string>();
        foreach (var c in source) //O(source.Length)
        {
            if (list.Count == 0)
            {
                list.Add(c.ToString());
            }
            else
            {
                var beingBuilt = new List<string>();
                foreach (var tempString in list) //O(list.Count)
                {
                    for (int i = 0; i < tempString.Length + 1; i++) //O(tempString.Length)
                    {
                        var built = tempString.Insert(i, c.ToString());
                        beingBuilt.Add(built);
                    }
                }

                list = beingBuilt;
            }
        }

        return list;
    }
}

Before making some general remarks about the C# code, I would like to explain why the complexity of this algorithm is already optimal, even though it is not the standard approach to tackle this problem.

Complexity Analysis

As already stated in a comment, your complexity analysis is way off. The problem is your assumption that the loop over list only multiplies the runtime by the length \$n\$ of the source. However, the list grows each iteration of the outer loop.

How complex is it really?

To determine the real complexity, we have to have a closer look at what happens inside the outer loop. By design, the list will contain all permutations of the first \$(k-1)\$ elements when the \$k^{th}\$ iteration starts. Each element of list has length \$(k-1)\$, accordingly, and there are \$(k-1)!\$ of them. So, in the \$k^{th}\$ iteration, we generate \$((k-1)+1)\cdot(k-1)! = k!\$ new permutations. Since we have to save a new string of length \$k\$ each time, which takes time \$k\$, up to a constant offset and multiplication with a constant, for each iteration we need \$k\cdot k!\$ character operations. This leaves us with a total runtime of \$\sum_{k=1}^nk\cdot k!\$.

It is not really straight forward that this is smaller than a multiple of \$n\cdot n!\$, i.e. that the complexity of the algorithm is \$O(n\cdot n!)\$. As a first step, we can reduce the problem a bit using the fact that \$k\leq n\$.

\$\sum_{k=1}^nk\cdot k! \leq n \cdot \sum_{k=1}^nk! = n \cdot n! \cdot \sum_{k=1}^n\frac{k!}{n!}\$

Now, let us make a convenient approximation for the items in the sum on the rhs, using that \$ \frac{1}{m} \leq \frac{1}{l} \$ for \$ m \geq l \$.

\$ \frac{k!}{n!} = \frac{1}{\prod_{m=k+1}^n m} \leq \frac{1}{\prod_{m=k+1}^n (m-k)} = \frac{1}{\prod_{l=1}^{n-k}l} = \frac{1}{(n-k)!} \$

This approximation allows us to get on the track of the argument used in the complexity analysis of the approach to generate permutations more often used in literature, namely generating all prefixes of permutations of the string of increasing length via recursion.

\$ \sum_{k=1}^n\frac{k!}{n!} \leq \sum_{k=1}^n\frac{1}{(n-k)!} = \sum_{m=0}^{n-1}\frac{1}{m!} \leq \sum_{m=0}^{\infty}\frac{1}{m!} = e\$

In the second equation, we used the the substitution \$m=n-k\$, which turns the summation order around. Moreover, for the last argument, one has to know that the exponential function is defined as function \$e^x = \sum_{n=0}^\infty \frac{x^n}{n!}\$.

The argument about convergence to a constant is similar in nature to that in the analysis of the average insert time into a dynamic array, which is often presented in CS classes. That argument uses a different series, though.

Why is this optimal?

To see that a \$O(n\cdot n!)\$ is optimal is not too hard. We know that there are \$n!\$ many permutations and that we have to generate a string of length \$n\$ for each, which each takes a number of character operations proportional to the length. So, we end up with at least \$n\cdot n!\$ character operations.

General Remarks

To be honest, I cannot find a lot to improve here regarding the coding style. One could separate the contents of the outer loop into a separate private function Permutations(List<string> permutationsOfPrefix, char nextCharacter), but I think it is equally as valid to keep the algorithm contained in one single method, especially since it is not very long. I also do not think that using LINQ here would improve the readability.

One point I think could be improved is there, though, the naming of variables. 'list' is a very generic name. I think permutationsOfPrefix would be more fitting. Accordingly, tempString could be permutation or permutationOfPrefix and beingBuilt could be permutationsWithNextCharacter. That would describe a bit better what is going on. Similarly, the i in the for loop could be named insertionIndex.

Another possible improvement is to explicitly check whether the input string is null or empty at the start, to return an empty list in this case and, otherwise, to initialize list with the first character contained. This would allow to remove the if-else-statement inside the outer loop. However, it would require to iterate over source.Drop(1) in the outer loop.


Single end point REST API I want to ask question about a better way for implementing single endpoint REST.
I want to build a rest API but I want it to only have single endpoint like: graphql, where you only have like: host/graphql.
I already doing this example in laravel where you need to put a key and method to go to a certain API.
The body look like this (example of calling product API with single endpoint).
Calling get all product:
    {
     "key":"product/all",
     "method":"GET"
    }

And this is the get one (product/{id}):
    {
    "key":"product",
    "method":"GET",
    "payload":"{\"id\":\"1\"}"
    }

This is also work fine with post, put and delete.
Here is the single endpoint code (I use php laravel as example because it fast to make API):
    public function bridge(Request $request)
    {

        $host = "http://127.0.0.1:8000/api/";
        if ($request->has("payload")) {
            $bounds = html_entity_decode($request->payload);
            $payload = json_decode($bounds, true);
        }
        if (strtoupper($request->method) == "POST") {
            try {
                $url = $host . $request->key;
                $request = Request::create($url, 'POST', []);
                $response = Route::dispatch($request);
                return $response;
            } catch (\Throwable $th) {
                $data["data"] = [];
                $data["success"] = false;
                $data["code"] = 500;
                $data["message"] = $th->getMessage();
                return $data;
            }
        } else if (strtoupper($request->method) == "PUT") {
            try {
                $url = $host . $request->key . '/' . $payload['id'];
                $request = Request::create($url, 'PUT', []);
                $response = Route::dispatch($request);
                return $response;
            } catch (\Throwable $th) {
                $data["data"] = [];
                $data["success"] = false;
                $data["code"] = 500;
                $data["message"] = $th->getMessage();
                return $data;
            }
        } else if (strtoupper($request->method) == "DELETE") {
            try {
                $url = $host . $request->key . '/' . $payload['id'];
                $request = Request::create($url, 'DELETE', []);
                $response = Route::dispatch($request);
                return $response;
            } catch (\Throwable $th) {
                $data["data"] = [];
                $data["success"] = false;
                $data["code"] = 500;
                $data["message"] = $th->getMessage();
                return $data;
            }
        } else {
            $url = $host . $request->key;
            try {
                if ($request->has("payload")) {
                    $url = $host . $request->key . "/" . $payload['id'];
                }
                $request = Request::create($url, 'GET');
                $response = Route::dispatch($request);
                return $response;
            } catch (\Throwable $th) {
                $data["data"] = [];
                $data["success"] = false;
                $data["code"] = 500;
                $data["message"] = $th->getMessage();
                return $data;
            }
        }
    }

As you can see there. It is calling the API twice "bridge" and the "key" API.
The cons here:

call request twice;
you can only put one /{id} variable (or more if you declare it).

Pros:

it only one endpoint so it will be easy to make an api helper in the frontend (maybe XD).

Can you show me a better way of doing this?
Tell me your tips and thought or maybe show some code (any language is fine).

I only have a few quibbling remarks:

Don't Repeat Yourself (D.R.Y.)

Declare $url = $host . $request->key unconditionally at the start of your method (say, after $host).  Then append text to it if/when needed $url .= '/' . $payload['id'];.
strtoupper($request->method) is done in each conditional expression.  It is better practice to mutate the string once.  You're also checking the same data for different strings, this is what switch-case blocks are for.
Rather than writing $data (a non-descriptive variable name) over and over, omit the variable declaration entirely and write the data directly into the return.  I regularly advise against single-use variable declarations unless they valuably reduce code width or improve readability.

else if should be elseif in PHP to comply with the PSR-12 coding standard.
All of your thrown exceptions are handled identically, so it seems to be a much cleaner choice to wrap your whole switch block in a single try-catch block.
Maybe you'd like to see the payload when there is a thrown exception; if so, use "data" => $payload ?? [] in the return.

Your code might be rewrite as:
public function bridge(Request $request)
{
    $host = "http://127.0.0.1:8000/api/";
    $url = $host . $request->key;

    if ($request->has("payload")) {
        $payload = json_decode(html_entity_decode($request->payload), true);
    }

    try {
        switch (strtoupper($request->method)) {
            case "POST":
                return Route::dispatch(Request::create($url, 'POST', []));
            case "PUT":
                $url .= '/' . $payload['id'];
                return Route::dispatch(Request::create($url, 'PUT', []));
            case "DELETE":
                $url .= '/' . $payload['id'];
                return Route::dispatch(Request::create($url, 'DELETE', []));
            default:
                if ($request->has("payload")) {
                    $url .= "/" . $payload['id'];
                }
                return Route::dispatch(Request::create($url, 'GET'));
         }
     } catch (\Throwable $th) {
         return [
                    "data" => $payload ?? [],
                    "success" => false,
                    "code" => 500,
                    "message" => $th->getMessage(),
                ];
     }
}


PHP MVC Framework Class In an effort to learn as much as possible about best development practices and software architecture, I created my own MVC framework using PHP (github: https://github.com/alemmedia/miniMVC).
I've found that this has resulted in phenomenal improvements in my programming skills, but I'm still limited by my own limited experience when it comes to reviewing my code.

I'd like to get your critique on one of my least favorite classes (I always feel like there is something off about them):

Here is my Session singleton I use to access/manage session data.

class Session{

/**
 * @var object Holds the single instance of Session
 */
private static $instance;

/**
 * @var string The session id of the current session
 */
protected $id;

/**
 * @var mixed The data to be held in session.
 */
public $data;

/**
 * __construct - Starts session
 *
 *  Establishes Session::data reference to $_SESSION superglobal array, and sets session id.
 *  Privately held an called only by Session::open for singleton functionality
 */
private function __construct(){
    session_start();
    $this -> id   = session_id();
    $this -> data =& $_SESSION;
}

/**
 * open - Creates singleton instance of Session object
 *
 * @return object
 */
public static function open() {
    if ( !isset(self::$instance) )
        self::$instance = new self();
    return self::$instance;
}

/**
 * set - Recieves variables to set to $_SESSION array
 *
 * @param string $property   The property to set to the session
 * @param mixed  $value      The balue to set to the property
 * @param bool   $make_array If set to true the property will be an numeric array
 */
public static function set($property, $value = null, $make_array = false){
    if ( is_array( $property) ){
        foreach($property as $key => $single_property)
            Session::open() -> set($key, $single_property);
    }else{
        if ($make_array == false)
            Session::open() -> data[$property] = $value;
        else
            Session::open() -> data[$property][] = $value;
    }
}

/**
 * del - Recieves variables to delete in $_SESSION array
 *
 * @param string $property   The property to delete from the session
 */
public static function del($property){
    if ( is_array( $property) ){
        foreach($property as $key => $single_property)
            Session::open() -> del($key);
    }else
        unset(Session::open() -> data[$property]);
}

/**
 * get - Returns variables from $_SESSION array.
 *
 * @param string $property   The property to retrieve from the session
 * @return mixed|bool        The retrieved property or false.
 */
public static function get($property){
    if ( isset( Session::open() -> data[$property] ) )
        return Session::open() -> data[$property];
    else
        return false;
}

/**
 * get - Returns variables from $_SESSION array and subsequently deletes them.
 *
 * @param string $property   The property to retrieve and delete from the session
 * @return mixed             The retrieved property.
 */
public static function getThenDel($property){
    $result = Session::open() -> get ($property);
    Session::open() -> del ($property);
    return $result;
}

}

Let me know if you've spotted any mistakes or have any suggestions.
Also, if you've found any code in the GitHub repository to be sub-optimal, I'd be happy to hear your comments.

Use braces {}! Can't say this enough. PHP has made many mistakes, but allowing this syntax is by far the worst. Adding those braces only increases file size by 2 bits! And it increases readability so much!

if ( !isset(self::$instance) ) { self::$instance = new self(); }

I'd check out the magic setter and getter methods (__set() and __get() respectively). Instead of calling a set or get method manually you just pass the variable you want. So...

$sessObj->set('var', 'value');
$sessObj->set('array', 'value', TRUE);
var_dump($sessObj->get('var'));
var_dump($sessObj->get('array'));
//compared to
$sessObj->var = 'value';
$sessObj->array[] = 'value';
var_dump($sessObj->var);
var_dump($sessObj->array);

Why are you not using $this? I mean, I don't use static methods at all really so correct me if I'm wrong, but from what I understand you can still access the internal instance by just using $this. So replace all Session::open() with $this. Even if you can't I would just dump all those static methods just for that alone. Why is everything static? If its not necessary, and I can't imagine why it would be, it shouldn't be static.

$this->set($key, $single_property);

Because my suggestions would change this code so drastically I will finish the review here. Hope this helps!


Most efficient way in C++ to strip strings If I want to strip a string completely of its whitespaces, punctuation and numbers (i.e. anything that is not A-Z, a-z), what is the most efficient way of doing it in C++?

I tried this:

string strip(string in) {
    string final;
    for(int i = 0; i < in.length(); i++) {
        if(isalpha(in[i])) final += in[i];
    }
    return final;
}

It works as expected, but is too slow on strings with ~2000 characters. I figured out that the code causing this slowness is the isalpha() call.

So does anyone know of a better, more efficient way of stripping a string of everything except [A-Z][a-z] in C++?

At most, the string will be 20000 characters long and I need to strip it in <1 second.

Thanks in advance.

EDIT:

If I remove the if condition, the output will display instantly. But with the if condition, it will take about 1.6 seconds to display the output.

For trying out the code, use this: http://pastebin.com/g3NtBFaD and a normal 20k char string. Then try comparing.

A few thoughts which come to my mind, without having actually profiled your code:

Try passing std::string as reference-to-const to avoid a copy (in case your std::string implementation is not Copy-On-Write).
Reserve space in the std::string by calling reserve.
Avoid calling std::string::length repeatedly, memorize the value.
Avoid indexing the string repeatedly, use an iterator instead.

For what it's worth, you could try a different (more functional) way to implement this function. Some may consider this idiomatic, other will find it harder to read. Your call -maybe just for the fun of it, to see how it performs (remember to enable optimizations!):

#include <algorithm>
#include <functional>
#include <locale>
#include <string>

std::string strip( const std::string &s ) {
    std::string result;
    result.reserve( s.length() );

    std::remove_copy_if( s.begin(),
                         s.end(),
                         std::back_inserter( result ),
                         std::not1( std::ptr_fun( isalpha ) ) );

    return result;
}


Image-processing algorithm on mobile device I have written the following algorithm (for Android/NDK) to apply levels to a bitmap. The problem is that it is really very slow.  On a fast device such as the SGSIII, it can take up to 4 seconds for an 8MP image. And on devices with ARMv6 takes ages (over 10 seconds). I know that can be speed up with NEON instructions, but I don't have such knowledge.

Is there any way to optimize it?

void applyLevels(unsigned int *rgb, const unsigned int width, const unsigned int height, const float exposure, const float brightness, const float contrast, const float saturation)
{
    float R, G, B;

    unsigned int pixelIndex = 0;

    float exposureFactor   = powf(2.0f, exposure);
    float brightnessFactor = brightness / 10.0f;
    float contrastFactor   = contrast > 0.0f ? contrast : 0.0f;
    float saturationFactor = 1.0f - saturation;

    for (int y = 0; y < height; y++)
    {
        for (int x = 0; x < width; x++)
        {
            const int pixelValue = buffer[pixelIndex];

            R = ((pixelValue & 0xff0000) >> 16) / 255.0f;
            G = ((pixelValue & 0xff00) >> 8) / 255.0f;
            B = (pixelValue & 0xff) / 255.0f;

            // Clamp values

            R = R > 1.0f ? 1.0f : R < 0.0f ? 0.0f : R;
            G = G > 1.0f ? 1.0f : G < 0.0f ? 0.0f : G;
            B = B > 1.0f ? 1.0f : B < 0.0f ? 0.0f : B;

            // Exposure

            R *= exposureFactor;
            G *= exposureFactor;
            B *= exposureFactor;

            // Contrast

            R = (((R - 0.5f) * contrastFactor) + 0.5f);
            G = (((G - 0.5f) * contrastFactor) + 0.5f);
            B = (((B - 0.5f) * contrastFactor) + 0.5f);

            // Saturation

            float gray = (R * 0.3f) + (G * 0.59f) + (B * 0.11f);
            R = (gray * saturationFactor) + R * saturation;
            G = (gray * saturationFactor) + G * saturation;
            B = (gray * saturationFactor) + B * saturation;

            // Brightness

            R += brightnessFactor;
            G += brightnessFactor;
            B += brightnessFactor;

            // Clamp values

            R = R > 1.0f ? 1.0f : R < 0.0f ? 0.0f : R;
            G = G > 1.0f ? 1.0f : G < 0.0f ? 0.0f : G;
            B = B > 1.0f ? 1.0f : B < 0.0f ? 0.0f : B;

            // Store new pixel value

            R *= 255.0f;
            G *= 255.0f;
            B *= 255.0f;

            buffer[pixelIndex] = ((int)R << 16) | ((int)G << 8) | (int)B;

            pixelIndex++;
        }
    }
}

Just loop over the index.
The first clamp is unnecessary. (X & 0xff) will always be between 0 and 255.
Look at it algebraically. There are some expressions that do not depend on R, B, or B that you can extract from the loop.
You can extract some common expressions and calculate them at the same time as grey.

.

void applyLevels(unsigned int *rgb, const unsigned int width, const unsigned int height, const float exposure, const float brightness, const float contrast, const float saturation)
{
    const float exposureFactor   = powf(2.0f, exposure);
    const float brightnessFactor = brightness / 10.0f;
    const float contrastFactor   = contrast > 0.0f ? contrast : 0.0f;
    const float saturationFactor = 1.0f - saturation;

    const float f01 = exposureFactor / 255.0f * contrastFactor;
    const float f02 = 0.5f * (1.0f - contrastFactor);

    const unsigned int nPixel = width * height;
    for(unsigned int pixelIndex = 0; pixelIndex < nPixel; ++pixelIndex)
    {
        const unsigned int pixelValue = rgb[pixelIndex];
        float R = static_cast<float>((pixelValue >> 16) & 0xff);
        float G = static_cast<float>((pixelValue >>  8) & 0xff);
        float B = static_cast<float>((pixelValue      ) & 0xff);

        R = R * f01 + f02;
        G = G * f01 + f02;
        B = B * f01 + f02;

        const float f03 = ((R * 0.3f) + (G * 0.59f) + (B * 0.11f)) * saturationFactor + brightnessFactor;

        R = (R * saturation + f03) * 255.0f;
        G = (G * saturation + f03) * 255.0f;
        B = (B * saturation + f03) * 255.0f;

        R = R > 255.0f ? 255.0f : R < 0.0f ? 0.0f : R;
        G = G > 255.0f ? 255.0f : G < 0.0f ? 0.0f : G;
        B = B > 255.0f ? 255.0f : B < 0.0f ? 0.0f : B;

        rgb[pixelIndex] =
            (static_cast<unsigned int>(R) << 16) |
            (static_cast<unsigned int>(G) <<  8) |
            (static_cast<unsigned int>(B)      );
    }
}

Other optimizations:

Do you need an entire 8MP image? Can you downscale the image or even grab a portion?
How often do you need to apply all four operations to an image? If it is common for some images to only require exposure and saturation then make separate functions.
Convert the for to a parallel for and use neon and other intrinsics.
Rather than doing the previous suggestion I would recommend using Renderscript. It will use your GPU if it can. If it uses the CPU it is automatically multithreaded.


Parsing s-expression structure into tree and summing the paths I'm new to Python, just attempted the task here.

The part I found hardest was parsing the expression into the tree structure. I was originally trying to build a regular tree structure (i.e. a Node object with left and right nodes), but without any logic for the insertion (i.e. newNode < node then insert left, newNode > node then insert right) I couldn't find a way.

In the end I've used Python's lists to kind of replicate the expression structure, and walk the paths as they're created. Each time I find a leaf, I calculate the cumulative sum, pop the last added node, and carry on.

The one part of the code I really don't like is the way I'm finding leafs:

if tree and expression[i-1:i+3] == ['(',')','(',')']:

and I don't like that I've done:

pair[1].replace('(', ' ( ').replace(')', ' ) ').split()

twice.

Any guidance on any part of this - style or just general approach and logic would be great.

def pairs(value):
    """ Yields pairs (target, expression) """
    nest_level = 0
    expr = ""
    target = 0

    value = value.replace('(', ' ( ').replace(')', ' ) ').split()
    for x in value:
        if x.isdigit() and not expr:
            target = x
        else:
            expr += x

        if x is '(':
            nest_level += 1
        elif x is ')':
            nest_level -= 1
            if nest_level is 0:
                yield target, expr
                expr = ''
                target = 0

def main():
    with open('input') as f:
        expr_input = f.read()

        level = 0
        current_target = 0

        for pair in pairs(expr_input):
            current_target = pair[0]
            # stack representing the 'current path'
            tree = list()
            # store the cumulative total of each path for this expression
            cumulative_totals = list()
            running_total = 0

            expression = pair[1].replace('(', ' ( ').replace(')', ' ) ').split()
            for i, s in enumerate(expression):
                if s is '(':
                    level += 1
                elif s == ')':
                    level -= 1
                    # "is leaf?" ugh.
                    if tree and expression[i-1:i+3] == ['(',')','(',')']:
                        cumulative_totals.append(running_total)
                        # remove the node and carry on down the next path
                        node = tree.pop()
                        running_total = running_total - int(node)
                    if level is 0:
                        if int(current_target) in cumulative_totals:
                            print "yes"
                        else:
                            print "no"
                else:
                    running_total += int(s)
                    tree.append(s)

if __name__ == '__main__':
    main()

Avoid indentation.

                        print "no"

When you need this amount of spaces in front of your code, I'd say no too.

This is clearly a sign that what you're writing could be extracted into a function call, our main could be a chain of function calls instead of a big clump of code.

with open('input') as f:
    parseFile(f)

def parseFile(...):
    ...
    for pair in pairs(expr_input): parsePair(pair)
    ...

def parsePair(...):
    ...
    for i, s in enumerate(expression): addExpressionToSummation(i, s)
    ...

Of course, this won't work out of the box; that's what object-oriented programming will help with.

Also, if you need to map one-to-one, use list comprehensions or functions like map.
Regular expressions might sometimes be valuable.

value = value.replace('(', ' ( ').replace(')', ' ) ').split()

can be

value = re.sub(r'(\(|\))', r' \1 ', value).split();

which can be improved to not add multiple spaces by restricting the match or by doing another replace where you replace r' +' by r' '. Of course this example might not be worth it, but if you've got to do heavier duty then you'll quickly want to resort to a regular expression instead of much longer trial-and-error code.
Conditions can be functions, too.

# "is leaf?" ugh.
if tree and expression[i-1:i+3] == ['(',')','(',')']:

Imagine that'd be

if isLeaf(tree, expression):

Oh look, our documentation line is gone. ;)


Raku multiplication using prompt and say I have extensively consulted Raku documentation and have tried many combinations to finally arrive at this working solution.
#!/usr/bin/perl6
use v6;

my $a;
my $b = 34;
my $c;
$a = prompt "enter number: ";
$c = ($a * $b);
say  "$a multiplied by  $b is $c";

To tighten your code a bit, you can declare my $a and assign in the same line:
my $a = prompt "enter number: ";
my $b = 34;
my $c;
$c = ($a * $b);
say  "$a multiplied by  $b is $c";

The above seems to solve your question, however a user might not respond with the proper input. If a user replies to the prompt with Hello you'll get an ungraceful error:
Cannot convert string to number: base-10 number must begin with valid digits or '.' in '⏏Hello' (indicated by ⏏) in block <unit> at Pedr_Ap_Glyn.p6 line 7

You can add a Type declaration to obviate this issue:
my Int $a = prompt "enter number: ";

...which, if an Int is not returned, will error with the message:
Type check failed in assignment to $a; expected Int but got [WHAT RAKU GOT IS HERE] in block <unit> at Pedr_Ap_Glyn.p6 line 5

Maybe a different pre-declared Type is more useful for your case:
my Numeric $a = prompt "enter number: ";

...which will work with non-Integer input (e.g. 3.14159265).
Or you could declare your own Type subset (below, "PosInt"):
subset PosInt of Int where * > 0;
my PosInt $a = prompt "enter number: ";

...which will only accept Ints greater that zero. Above are just a few options that the Raku language offers--and that you may wish to take advantage of.


Finding a customer's gender I have been playing with Async/parallel execution in entity framework for while. I'm not sure if I'm doing it right.

I have two entity contexts: one for reading and one for writing. The reading context has more than 200000 rows in table Customer. It took 9 minutes on my PC (i7+8GB).

let FindGender(skip : int, take : int) : Async<unit> =
            async {
                try
                    use readContext = new ReadContext()
                    use context = new WriteContext()
                    let! customers = Async.AwaitTask
                                     <| readContext.Customers.OrderBy(fun c -> c.CustomerId).Skip(skip).Take(take)
                                              .ToListAsync()
                    let findGender (c : NA.Data.Entities.Customer) : unit =
                        let name = context.Names.FirstOrDefault(fun i -> i.Name.ToLower() = c.FirstName.ToLower())
                        match name = Unchecked.defaultof<NameTable> with
                        | false ->
                            Console.WriteLine(String.Format("{0}", name.Name))
                            // create customer
                            let customer = new Customer()
                            customer.ExternalCustomerId <- c.CustomerId
                            customer.ExternalCustomerName <- c.FirstName
                            customer.GenderId <- name.Gender
                            customer.GenderName <- name.GenderType.ToString()
                            customer.NameListId <- name.NameId
                            context.Customers.Add(customer) |> ignore
                        | _ -> 0 |> ignore

                    customers |> Seq.iter findGender

                    let! _ = Async.AwaitTask <| context.SaveChangesAsync()
                    return()
                with ex -> printfn "Exception handled. %A" ex
            }
        let sw = new Stopwatch()
        sw.Start()
        let customerCount = 218114
        let step = 1000
        [ 0..step..customerCount ]
        |> Seq.map (fun i ->
               if i + step < customerCount then (i, step)
               else (i, customerCount - i))
        |> Seq.map (fun (step, take) -> FindGender(step, take))
        |> Async.Parallel
        |> Async.RunSynchronously
        |> ignore
        sw.Stop()
        printfn "%A" sw.Elapsed

match name = Unchecked.defaultof<NameTable> with
| false -> …
| _ -> …

If you're checking whether a condition is true, you should use if, not pattern matching. It's simpler and clearer.

0 |> ignore

Instead of this, you can write the unit value: ().


The code is used to add the digits in a digital clock to each other. I duplicated code and could use some help DRYing it out I'm using Codefights to improve my skills. This challenge is to change the number submitted as total minutes into the digital read out on a clock i.e. hh:mm and then add all the numbers to together. So h + h + m + m.

My code works and I've passed all the tests, but it duplicates the same process for variables I've named number and float. What are some ways I can clean this up? At the bottom of the code I put the tests with the correct answers commented out next to them.

def lateRide(n)
  n = n.to_f
  n = n/60
  float = n % 1
  number = n - float
  number = number.to_i

  float = float * 60
  float = float.round.to_i

  if number > 9
    tens = number/10
    ones = number-(tens*10)
  else
    tens = 0
    ones = number
  end

  if float > 9
    ftens = float/10
    fones = float-(ftens*10)
  else
    ftens = 0
    fones = float
  end
  tens + ones + ftens + fones
end

puts lateRide(240) #4
puts lateRide(808) #14
puts lateRide(1439) #19
puts lateRide(0) #0
puts lateRide(23) #5
puts lateRide(8) #8

For your current code:

Don't name variables float and number; name then for what they represent, not their datatype.
Don't use camelCase for methods; Ruby convention is to use snake_case

As for its functionality: There's a much easier way to get hours and minutes from a total number of minutes:

hours, minutes = total.divmod(60)

#divmod is basically a convenient shorthand for "quotient and remainder"; it's the same as this:

hours = (total / 60).floor
minutes = total % 60

just in one method.

Now, #divmod returns an array with 2 numbers in it, and in the code above I simply assign those two numbers to hours and minutes respectively.

If you then use #divmod again on those two but with 10 as the argument, you can get the "ones" and the "tens", the same way you just got "minutes" and "hours" by using 60 as your argument.

So say the total number of minutes is 119, i.e. 1h 59m, and you want to split both the hours and minutes, you can do this:

total
  .divmod(60)                    #=> [1, 59]
  .flat_map { |n| n.divmod(10) } #=> [0, 1, 5, 9]

Note however, that if you've got more than 99 hours, you could run into problems, as you'll only get the ones and tens; not the hundreds. E.g. 6000 minutes is 100 hours, but the above would return [10, 0, 0, 0] (sum 10), rather than all the individual digits, which would be [1, 0, 0, 0, 0] (sum 1).

This might be correct for the challenge still, since the readout is hh:mm, and thus doesn't say anything one way or the other about how 3-or-more hour-digits should be handled. The "digital sum" might be 10 or 1 - or zero if anything above 4 digits is just ignored. Or it could be d:hh:mm, and it'd be 4 days + 4 hours + 0 minutes = 8... there are just a lot of unknowns.

But if we assume that the input will stick to 2-digit hours, the code above will work without any ambiguity - all we need is the sum. For that you can use #reduce (aka #inject):

def late_ride(minutes)
  minutes
    .divmod(60)
    .flat_map { |n| n.divmod(10) }
    .reduce(:+)
end

and bingo.

But if the idea instead is to add up all the digits individually and the hours can have 3 or more digits, this, as mentioned, won't be quite right.

You can use #divmod repeatedly or recursively to get the digits of an arbitrary number of hours, but for this little challenge, I'll just suggest something I usually don't suggest: Convert the digital readout to a string, and get the digits that way. I usually try to avoid this, since it seems inelegant to treat a numeric value as a string only to get another numeric value. Just hey, it works, so:

total
  .divmod(60)  #=> [1, 59]
  .join        #=> "159"
  .chars       #=> ["1", "5", "9"]

From here, you can use #reduce again to get the sum (just call to_i to get actual numbers from the strings). And then this is your method:

def late_ride(minutes)
  minutes
    .divmod(60)
    .join
    .chars
    .reduce(0) { |sum, n| sum + n.to_i }
end

or, written another way:

def late_ride(minutes)
  minutes
    .divmod(60)
    .join
    .chars
    .map(&:to_i)
    .reduce(:+)
end


Count of words in a mutable Map The function should get some text on input and return a map in a following format:

("word1" -> 1, "word2" -> 2 ...)

The keys are words from text and the values are representing the number of times the word appeared in the text. We take into a count only words which at least 4 characters long. Special chars are ignored, words are case insensitive.

import scala.collection.mutable.Map

def wordCounter(text: String) = {
  var map:Map[String, Int] = Map()

  text.toLowerCase.replaceAll("[^a-z ]", "").split(" ").filter(_.length > 3).foreach(x => addWord(map, x))
  map
}

def addWord(map: Map[String, Int], word: String) = {
  map(word) = map.getOrElse(word, 0) + 1
}

Is that fine to use mutable in this case?
Should I always be suspicious about the solutions that require to use mutable and use it only when its problematic to implement it with immutable types?
I'm doing an extra loop to filter words longer than 3 chars. In my mind this way it looks a bit cleaner than to have a condition in foreach. Should I care more about the performance or readability in my code?

Should I always be suspicious about the solutions that require to use mutable and use it only when its problematic to implement it with immutable types?

I'm new to Scala myself, but I would say yes.
Try to use immutable and elegant solutions when possible, but it's OK to resort to mutable alternatives as a second choice.

  Is that fine to use mutable in this case?

I would use an immutable algorithm in this case:

use .groupBy to create groupings
use .map to convert the groupings to a list of word-count pairs
a final .toMap call and the map is ready

  I'm doing an extra loop to filter words longer than 3 chars. In my mind this way it looks a bit cleaner than to have a condition in foreach. Should I care more about the performance or readability in my code?

If the complexity is on the same order, then prefer readability, even if slightly slower. For example, it's OK to choose an \$O(3n)\$ algorithm instead of an \$O(2n)\$ algorithm. When code is more readable, it tends to be also easier to optimize at a higher level of logic, which can lead to far faster solutions than the original alternatives.

Suggested implementation using immutable logic:

  def wordCounter(text: String) = {
    {
      for {
        word <- text.toLowerCase.replaceAll("[^a-z ]", "").split(" ")
        if word.length > 3
      } yield word
    }.groupBy(identity).map { case (word, list) => (word, list.length) }
  }


Work out largest prime factor of a number I am trying to get better at problem solving, so I am going through challenges on Project Euler.
Problem:

The prime factors of 13195 are 5, 7, 13 and 29.
What is the largest prime factor of the number 600851475143 ?

My solution:
My approach to this was the following process:

Get all factors of the number

Identify prime factors by looping from 1 up to each factor. If the factor MOD the numbers going up is equal to 0 and the number is not 1 or the number itself, then it is not a prime. Then another for loop goes through each factor again and checks if it is not in the not primes list, hence making it a prime.

The last item in the primes list gives the biggest prime number

Code:
def get_factors(n):
    factors = []
    for i in range(1,n+1):
        if n % i == 0:
            factors.append(i)

    return factors

def get_prime_factors(factors):
    print(factors)
    primes = []
    not_primes = [1]
    for i in range(len(factors)):
        for k in range(1,factors[i]+1):
            if factors[i] % k == 0 and factors[i] !=1 and k!=1 and k!=factors[i]:
                not_primes.append(factors[i])
    for i in factors:
        if i not in not_primes:
            primes.append(i)
    return primes

def biggest_prime(primes):
    biggest = primes[-1]
    print(biggest)

factors = get_factors(600851475143)
primes = get_prime_factors(factors)
biggest_prime(primes)

Problems with my solution:
It seems like it is too overcomplicated, which is why it can only handle small numbers. The program did not finish executing for the number 600851475143.
What is specifically making my solution so inefficient, is it because I am using too many loops? How can I make it efficient?

There are many questions about Project Euler 3 on this site already.  The trick is to pick an algorithm that…

Reduces n whenever you find a factor, so that you don't need to consider factors anywhere near as large as 600851475143
Only finds prime factors, and never composite factors

from itertools import chain, count

def biggest_prime_factor(n):
    for f in chain([2], count(3, 2)):
        while n % f == 0:
            n //= f
            if f >= n ** 0.5:
                return f if n == 1 else n

print(biggest_prime_factor(600851475143))


A simple implementation of a mutable String in C The following code implements a simple interface to operate on mutable* Strings in C. It is composed of two files: one for the structure definition and the available operations, and the other with the actual implementation.

*This can be inexact, as only adding characters to the end is implemented.

Any comments on coding style and improvements are greatly appreciated.

String.h

#ifndef __STRING_H__
#define __STRING_H__

#include <stdio.h>
#include <stdbool.h>
#include <stdlib.h>
#include <string.h>

#define BLOCK_SIZE 256

typedef struct {
    char *data;
    int length;
    int blocks;
} String;

String * string_create();
void string_dispose(String *str);

bool string_empty(String *str);
void string_append_char(String *str, char c);

char * string_get_all(String *str);
char string_get(String *str, int pos);

int string_find(String *str, char c);

#endif /* __STRING_H__ */

String.c

#include "String.h"

/** Create a String */
String * string_create() {
    String *ans = calloc(1, sizeof *ans);

    if (ans != NULL) {
        ans->data = malloc( BLOCK_SIZE * sizeof *(ans->data) );

        ans->length = 0;
        ans->blocks = (ans->data == NULL) ? 0 : 1;
    }

    return ans;
}

/** Free the memory associated with a String */
void string_dispose(String *str) {
    if (str != NULL) {
        free(str->data);
        free(str);
    }
}

/** Is the String empty? */
bool string_empty(String *str) {
    if (str == NULL) return true;
    if (str->length == 0) return true;
    return false;
}

/** Add a character to the end of the String */
void string_append_char(String *str, char c) {
    if (str != NULL) {
        if (str->length == str->blocks * BLOCK_SIZE) {
            char *new_str = realloc( str->data, BLOCK_SIZE * (str->blocks + 1) * sizeof *(str->data));

            if (new_str != NULL) {
                str->data = new_str;
                ++(str->blocks);
            }
        }

        if (str->length < str->blocks * BLOCK_SIZE) {
            str->data[str->length] = c;
            ++(str->length);
        }
    }
}

/** Get a C-String with the proper null-terminator */
char * string_get_all(String *str) {
    char *res = NULL;

    if (str != NULL) {
        res = malloc((str->length + 1) * sizeof *str->data);

        if (res != NULL) {
            memcpy(res, str->data, str->length);
            res[str->length] = '\0';
        }
    }

    return res;
}

/** Get a character at a given position in the String */
char string_get(String *str, int pos) {
    char res = '\0';

    if (str != NULL) {
        if (pos >= 0 && pos < str->length) {
            res = str->data[pos];
        }
    }

    return res;
}

/** Get where the first occurrence of a character in the String is */
int string_find(String *str, char c) {
    int pos = -1;

    if (str != NULL) {
        pos = 0;
        while (str->data[pos] != c) ++pos;

        if (pos == str->length) pos = -1;
    }

    return pos;
}

This is a test file that uses some of the core methods implemented in a String. This file is called test_string.c.

#include <stdio.h>

#include "String.h"

int main() {
    String *str = string_create();

    char i;
    for (i = 'a'; i <= 'z'; i++)
        string_append_char(str, i);

    char *cstr = string_get_all(str);

    printf("%s\n", cstr);

    int pos = string_find(str, 'f');

    printf("Character 'f' occurs at position %d.\n", pos);
    printf("Reading from the String, we get \"%c\".\n", string_get(str, pos));

    free(cstr);
    string_dispose(str);

    return 0;
}

Unsafe loop

This loop in string_find() is unsafe since it could read past the end of your buffer:

    while (str->data[pos] != c) ++pos;

You should add an additional check like this:

    while (pos < str->length && str->data[pos] != c) ++pos;

Short circuit error conditions

Rather than doing this:

 if (str != NULL) {
     // Rest of function indented
 }

it would be easier to read if you rewrote it like this:

if (str == NULL)
    return NULL;

// Rest of function, not indented

Other things

Your reallocation strategy will lead to \$O(n^2)\$ behavior when appending to long strings.  You might want to double the allocation instead of adding a fixed amount.

You might want to use size_t for your sizes and lengths instead if int, because an int might overflow at 32KB on some platforms.


Stream "on the fly" from matrix I want to produce a Stream<Pixel> from a Pixel[][]. My code for this is extremely verbose so I suspect I am doing something bad.

Please provide me some advice on how to make my code shorter and cleaner.

private Stream<Pixel> stream() {
    Iterable<Pixel> iterable = new Iterable<Pixel>() {
        @Override
        public Iterator<Pixel> iterator() {
            return new Iterator<Pixel>() {
                private int row = 0;
                private int col = 0;

                @Override
                public boolean hasNext() {
                    return row < height && col < width;
                }

                @Override
                public Pixel next() {
                    Pixel pixel = pixels[row][col];

                    if (col == width - 1) {
                        col = 0;
                        row++;
                    } else {
                        col++;
                    }

                    return pixel;
                }

                @Override
                public void remove() {
                    throw new RuntimeException("nope");
                }
            };
        }
    };

    return StreamSupport.stream(iterable.spliterator(), false);
}

For example, in Python I could do something like:

def stream():
    for row in pixels:
        for pixel in row:
            yield pixel

Which is certainly much more concise and understandable.

You can of course use the Arrays.stream() (twice, because it's a 2D array)

Something along these lines:

private Stream<Pixel> stream() {
  return Arrays.stream(pixels).flatMap(Arrays::stream);
}


Searching first non repeating character The program finds the first non repeating character in a character array, array contains only small case letters.

#include<iostream>
#include<cstring>
using namespace std;
int main()
{
char a2z[26]={0};//Hash

char input[100] = "abcbbbcdegh";//sample input, only small case english alphabets
int n = strlen(input);

for(int i=0; i<n; ++i)
{
    if( a2z[  input[i]-97 ]<0 )
    {
         //do nothing, this character has already repeated
    }
    else if( a2z[  input[i]-97 ]==0 )
    {
        a2z[ input[i]-97 ] = i+1; //character encoutered first time
    }
    else
    {
        a2z[ input[i]-97 ] = -(i+1); //repeating character
    }
}

int minAt=n+1;
for(int i=0; i<26; ++i) //finding first non-negative character in the Hash
{
    if( (a2z[i]>0) && (a2z[i]<minAt) )
    {
        minAt=a2z[i];
    }
}

if( minAt == n+1 )
    cout<<"\n\n no unique character";
else
    cout<<"\n\nFirst unique character is "<<input[minAt-1]<<" at position "<<minAt-1<<endl;
}

output ::

First unique character is a at position 0

here, i have taken an character array (i.e. a2z) of 26 characters initialized with zero, using it as Hash.
 if a2z[i] is zero means ith character (of English alphabet) has not encountered till now, postive value of a2z[i] denotes ith character has been encountered only once at position a2z[i]-1, and negative values shows it has repeated.
pls review this code.

This is a bad habbit stop doing it.

using namespace std;

Once your programs are more than 10 lines long it starts adding complexity including the whole namespace. SO best not to do it. Get into the habit of not using now otherwise breaking the habit is hard latter.

Don't put bad comments in you code (in fact try not to put comments). A comment should explain what you are trying to achieve (or potentially why) (NOT HOW). The code explains how so don't duplicate the code in comments. The worst thing to find in code is comments that don't match the code.

char a2z[26]={0};//Hash

This is not a hash. A hash has a very specific meaning and this is not what you are doing.

Prefer to use [] when declaring the array. This means the compiler will try and establish the length of the array for you.

char input[100] = "abcbbbcdegh";//sample input, only small case english alphabets

While we are here. Stop using C stuff in C++. Learn to use C++ constructs and don't mix the languages up. Another bad habbit you should fall into.

int n = strlen(input);

C++ has its own string class.

std::string  input("abcbbbddddd");
// size is input.size()

Learn how to use iterators.

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

It will allow you to be more versatile with your code. Potentially allowing your code to work with multiple container types (not just string). But more importantly it is the first step to using the standard algorithms.

You are making the assumption that your input only contains lower case letters. You should always code defensively. You MUST validate your input.


Lock-free SPMC queue Here is my lock-free queue implementation for single producer with some preallocated memory. T is a simple type with no need for move operations. I don't use bool pop(T& result) in order to avoid double checking. I would love to see your feedback!

template <typename T>
class LockFreeQueue {
public:
    void setup(U8* buffer, U32 size) {
        m_buffer = (T*)buffer;
        m_size = size;
        clear();
    }

    void clear() {
        m_first.store(0, std::memory_order_relaxed);
        m_last.store(0, std::memory_order_relaxed);
    }

    void push(const T& value) {   // only for single producer
        U32 last = m_last.load(std::memory_order_relaxed);
        m_buffer[last] = value;
        U32 next = inc(last);
        ASSERT(next != m_first.load(std::memory_order_relaxed), "Queue overflow");
        m_last.store(next, std::memory_order_release);
    }

    bool isEmpty() {
        return m_first.load(std::memory_order_relaxed) == m_last.load(std::memory_order_relaxed);
    }

    void pop(T& result) {    // for single consumer. check isEmpty() before
        ASSERT(!isEmpty(), "Queue is empty");
        U32 first = m_first.load(std::memory_order_relaxed);
        result = m_buffer[first];
        m_first.store(inc(first), std::memory_order_release);
    }

    bool tryPop(T& result) {    // for multiple consumers. don't use isEmpty()
        while (true) {
            U32 first = m_first.load(std::memory_order_relaxed);
            if (first == m_last.load(std::memory_order_relaxed))
                return false;
            result = m_buffer[first];
            if (m_first.compare_exchange_weak(first, inc(first), std::memory_order_release))
                return true;
        }
    }

private:
    U32 inc(U32 n) { return (n + 1) % m_size; }

    T* m_buffer;
    U32 m_size;
    std::atomic<U32> m_first;
    std::atomic<U32> m_last;
};

Memory ordering problem
This line:

        if (first == m_last.load(std::memory_order_relaxed))
            return false;

should be:
        if (first == m_last.load(std::memory_order_acquire))
            return false;

Otherwise, on the next line you may load the wrong value out of m_buffer due to the load being reordered before the load to m_last.  I spotted this because I noticed a memory_order_release without a corresponding memory_order_acquire.  The pop() function also needs a load of m_last with memory_order_acquire.
You have the same problem in push().  If you don't load m_first with memory_order_acquire, you run the risk of writing to the buffer when it is full, thus overwriting some element.  Even the assertion won't help because you could have overwritten the buffer and then loaded a modified value for m_first.
Example of failure
Since your comment indicated that I didn't explain thoroughly enough, I will give an example of how the pop could go wrong.

The queue is empty, with m_first = m_last = 0.
The consumer calls try_pop().
In try_pop(), m_first is loaded as 0.
Now, the cpu speculatively loads m_buffer[0], even though you haven't reached that line yet.
The producer calls push().
The producer writes m_buffer[0], and then increments m_last to 1, in that order.
The consumer reads m_last as 1.
The consumer uses the value speculatively loaded in step 4 as the value for m_buffer[0], but this value is wrong.

By adding the memory_order_acquire, you prevent the load of m_buffer[0] from happening until after step 7.


Password hashing method I am having some issue to use PHP5 password_hash() function. My server is not supporting it, so I am using a function to hash.  Is this one secure?

public function Pass_Hash ($password) {

      $blowfish_salt = bin2hex(openssl_random_pseudo_bytes(22));
      $hash = md5(crypt($password, "X1;G^COU8U`Bo*A6@9<on5yQ6P87M]".$blowfish_salt));
      return $hash;

}

I read lot of articles regarding password hashing.  Can anyone help me rewrite it more securely?

NO NO NO NO NO!

Besides what others have pointed out, I find some serious flaws in your code.

You are not actually using blowfish. To use blowfish, your salt must begin with something like $2y$07$ see the PHP Documentation
Removing your $blowfish_salt variable from the code makes no difference! This will produce the same output. Therefore, we conclude that you are actually using a constant salt in your code.
The output of crypt is more secure than MD5. MD5 is an algorithm that is known to have a lot of duplicates. Many different inputs will produce the same results. That's not optimal. Using real blowfish algorithm is even more secure than what you currently have.

Here is some testing code I used to come to my conclusions:

<?php
$password = "bubu";
  $blowfish_salt = bin2hex(openssl_random_pseudo_bytes(22));

$blowfish_salt = "11b19e4e03d32f27f9db5646e56f63458113e9da56f7"; // Since this is the only randomness, I have replaced the randomness with this constant.

echo "salt is $blowfish_salt
";

$first = crypt($password, "X1;G^COU8U`Bo*A6@9<on5yQ6P87M]".$blowfish_salt);
echo $first . " ... constant with what you call blowfish
";

$hash = md5($first);
echo $hash . " ... md5-ed version of above. This is the result of your current code
";
echo md5(crypt($password, "X1;G^COU8U`Bo*A6@9<on5yQ6P87M]")) . " ... and oops, that was exactly the same!
";

$first = crypt($password, "X1;G^COU8U`Bo*A6@9<on5yQ6P87M]");
echo $first . " ... constant salt
";

$first = crypt($password, $blowfish_salt);
echo $first . " ... only blowfish salt
";
echo "
";
echo "BELOW USING 2Y PADDING:
";
echo "
";
$first = crypt($password, "\$2y\$07\$X1;G^COU8U`Bo*A6@9<on5yQ6P87M]".$blowfish_salt);
echo $first . " ... a lot, with blowfish. Causes error because salt is too long
";

$first = crypt($password, "\$2y\$07$\X1;G^COU8U`Bo*A6@9<on5yQ6P87M]");
echo $first . " ... constant salt. Still causes error because salt is too long
";

$first = crypt($password, "\$2y\$07\$".$blowfish_salt);
echo $first . " ... only blowfish salt. And now THAT is blowfish!
";

echo "
";
echo "Now back to md5 with this result again:
";

$hash = md5($first);
echo $hash;
?>

The output of this is:

salt is 11b19e4e03d32f27f9db5646e56f63458113e9da56f7
X154A2VqJVf4I ... constant with what you call blowfish
7b5f3730488255efcff9441a238d5c5d ... md5-ed version of above. This is the result of your current code
7b5f3730488255efcff9441a238d5c5d ... and oops, that was exactly the same!
X154A2VqJVf4I ... constant salt
11Xw1fe5HDjyo ... only blowfish salt

BELOW USING 2Y PADDING:

*0 ... a lot, with blowfish. Causes error because salt is too long
*0 ... constant salt. Still causes error because salt is too long
$2y$07$11b19e4e03d32f27f9db5uyeLJeUJh/VUoi1RFtAoDAE.eegj4bwO ... only blowfish salt. And now THAT is blowfish!

Now back to md5 with this result again:
e35cb490b47d3211dac9f85acfc2fb15

Please, take your time and study these results. Look at the code. Look at the results. Look at the code again. Look at the results again. Now look at the code one more time: Do you see the flaws? If you don't, I pointed them out at the beginning of my answer.

What you should do:

Generate a salt, store that somewhere (as suggested by Chris). Get rid of that constant-salt you have.
Use the real blowfish, and don't use MD5 afterwards

public function Pass_Hash ($password, $blowfish_salt) {
      $hash = crypt($password, "\$2y\$07\$" . $blowfish_salt));
      return $hash;
}
$generated_salt = bin2hex(openssl_random_pseudo_bytes(22));
Pass_Hash("a very secret password", $generated_salt);

Now we're talking.

However, I should add that I am not a security experts. There are likely several more aspects that one can think of. If you are interested in security and secure stuff, I recommend paying our StackExchange friends at Information Security a visit. But in the end I think that the question is: How secure do you want to be?


Sum of squares of 2 largest numbers out of a list of 3 numbers The problem is:

  Define a procedure that takes 3 number arguments and returns the sum of the squares of the 2 largest numbers.

I am a CS graduate and I am revising my university notes. This is my code so far:

(define (square n) (* n n))

(define (square_of_larger_numbs a b c)
  (cond ((and (> a b) (> a c))
         (cond ((> b c)
         (+ (square a) (square b)))
               (else (+ (square a) (square c))))
        )(else (cond ((> b c)
                      (+ (square b) (square c)))
                      (else (+ (square b) (square c))))
               )
         )
  )

(display (square_of_larger_numbs 4 3 3))

It seems to be working but the code looks terrible unreadable. Please note that I am not allowed to sort yet, as the book I am reading hasn't mentioned sorting yet, so I assumed that sorting is not welcomed as a part of the solution.

How can I make this more efficient/ readable?

Specification

It is helpful to specify the solution before writing code. The mathematics of sorting three values (a b c) means there are six (3!) permutations:

(define abc (a b c))
(define acb (a c b))
(define bac (b a c))
(define bca (b c a))
(define cab (c a b))
(define cba (c b a))

The code must cover all six possibilities.

There are three possible outputs:

(define output-ab (+ (* a a) (* b b)))
(define output-ac (+ (* a a) (* c c)))
(define output-bc (+ (* b b) (* c c)))

The code must map the six permutations to the three outputs.

abc || bac -> output-ab
acb || cab -> output-ac
bca || cba -> output-bc

Now the code can reflect the specification

   (cond

     ((or (and (>= a b)
               (>= b c))
          (and (>= b a))
               (>= a c)))
      output-ab)

     ((or (and (>= a b)
               (>= c b))
          (and (>= a b))
               (>= c a)))
      output-ac)

     ((or (and (>= b a)
               (>= c a))
          (and (>= c b))
               (>= b a)))
      output-bc)

     (else (error "unhandled condition")))

Review

Style

Scheme has formatting conventions. The code does better with indentation than with parenthesis.

Logic

The lack of a specification and tests based upon it leads to an incorrect implementation and poorly organized code. There is a branch which returns the same value on both paths:

 (else (cond ((> b c)
                (+ (square b) (square c)))
              (else (+ (square b) (square c))))

Efficiency

Correct code is the first order objective. Writing clear code facilitates that because it makes debugging and testing and maintenance easier.

Speed of execution matters only when it matters. That is always after the code is correct and after there is an objective performance issue, not just a theoretical one. Getting a wrong answer fast may be worse than no answer at all if we mistakenly believe the wrong answer is correct.


Implementing a memory scanner in python I'm trying to implement a memory scanner in python.
The problem is, it's too slow.
What it does it gets a number from the command line and compares it to every address, then writes it to an output file.
How can I improve my algorithm?

import ctypes as c
from ctypes import wintypes as w
from struct import *
from time import *
import datetime
import sys

pid = 1234

k32 = c.windll.kernel32

OpenProcess = k32.OpenProcess
OpenProcess.argtypes = [w.DWORD,w.BOOL,w.DWORD]
OpenProcess.restype = w.HANDLE

ReadProcessMemory = k32.ReadProcessMemory
ReadProcessMemory.argtypes = [w.HANDLE,w.LPCVOID,w.LPVOID,c.c_size_t,c.POINTER(c.c_size_t)]
ReadProcessMemory.restype = w.BOOL
PAA = 0x1F0FFF
address = 0x4000000
ph = OpenProcess(PAA,False,int(pid)) #program handle

buff = c.create_string_buffer(4)
bufferSize = (c.sizeof(buff))
bytesRead = c.c_ulonglong(0)

addresses_list = xrange(address,0x9000000,0x4)
log=open(r'out.txt.','wb',0)
for i in addresses_list:
    ReadProcessMemory(ph, c.c_void_p(i), buff, bufferSize, c.byref(bytesRead))
    value = unpack('I',buff)[0]
    if value == int(sys.argv[1]):
        log.write('%x\r\n' % (i, ))

You should check if ReadProcessMemory fails (returns 0)
parse sys.argv only once
read bigger chunks of memory in a row into an efficient array
best use numpy for search  (or at least array.array)

Key lines using numpy:

import numpy as np

match_val = int(sys.argv[1])
a = np.zeros(4096, 'l')     # chunk size; should be even bigger ..
r = ReadProcessMemory(ph, chunk_start_adr, a.ctypes.data,
                      a.itemsize * a.size, c.byref(bytesRead))
if r:
    indices, = np.where(a == match_val)   # fast bulk search
    log.write(''.join('%x\r\n' % i for i in indices))

It will be lightning fast.


Improving on Gradient Descent Algorithm I have tried to solve the problem 607 in Project Euler  using the Gradient Descent Algorithm. While I’m getting the answer right I’m not sure if I have used the gradient algorithm correctly or coded it efficiently in Python. I’ll appreciate any guidance or suggestions to improve.

#!/usr/bin/env python3
# -*- coding: utf-8 -*-
"""
Created on Thu Jan 11 18:48:41 2018

"""

# Gradient Descent Algorithm

import numpy as np
import time
import random

def duration(points): # Duration as the objective function
    speed = [ 10, 9, 8, 7, 6, 5,10 ]
    duration=0
    for i in range(0,len(speed)):
        distance = ((points[i+1,0]-points[i,0])**2 + (points[i+1,1]-points[i,1])**2)**0.5
        duration += distance / speed[i]
    return duration

def gradient_function(points): # Gradient: Partial differential of duration function f(x) wrt x
    speed = [ 10, 9, 8, 7, 6, 5,10 ]
    gradient = 0 #np.zeros((8,2))
    for i in range(0,len(speed)):
        gradient += ((points[i+1,0]-points[i,0])/(speed[i]*((points[i+1,0]-points[i,0])**2 + (points[i+1,1]-points[i,1])**2)**0.5))
    return gradient

# Generate randon number between -1 and +1
def myrand():
    seed = random.random() * 2 - 1
    return seed

# Compute x = x0-delta*gradient
def gradient_descent(points,delta):
    old_Duration = duration(points)
    new_Duration = old_Duration - delta * gradient_function(points)
    if myrand() > 0:
        delta = -delta
    id = int(random.random()*6)+1
    points[id,0] += delta
    new_Duration = old_Duration - (1+ delta)*delta * gradient_function(points)
    if (duration(points) >= old_Duration):
        points[id,0] -= delta
        new_Duration = old_Duration - (1-delta)*delta * gradient_function(points)
    return new_Duration

# Floating Point Range Function
def frange(start, stop, step):
    i = start
    while i >= stop:
        yield i
        i /= step
    return step

# Calculate the Minimum Days
def min_days():
    normal_terrain = ((100 / np.sqrt(2) - 50) / 2)
    marsh = 10.
    ini_xy = [ 0., normal_terrain, marsh, marsh, marsh, marsh, marsh, normal_terrain ]
    y_coords = []
    x_coords = []
    k = 0
    for i in range(0,len(ini_xy)):
        k += ini_xy[i]
        y_coords.append(k)
        x_coords.append(k)
    points = (np.stack((x_coords,y_coords), axis=-1))

    for delta in frange(1e-2, 1e-10, 10):
        for i in range(10000):
            gradient_descent(points,delta)
    min_days = gradient_descent(points,delta)
    return min_days

start = time.time()
answer = min_days()
elapsed = time.time() - start
print("\nThe Minimum Days is %s found in %s seconds" % (answer,elapsed))

For Python 3.x you do not need to specify the encoding type you chose because it chips by default with UTF-8. So you can safely remove the directive: # -*- coding: utf-8 -*-
You should use timeit over time because the first one is more accurate.
As per PEP 8, you should leave 2 blank lines between the last import statement and the start of your code.
Better if you run this module by putting the guard if __name__ == "__main__"
The inline comments you used would be better used as docstrings instead.


Refactored code for updating user password, email and other info I have to update user email and password if they are present in the params, I have other user information also which can be updated.

this is the method I have written, but it doesn't seem like a good method

def update
    if user_params[:current_email].present?
      if @current_user.has_valid_email?(user_params[:current_email])
        @current_user.update(email: user_params[:new_email])
      else
        render json: {errors: ["Current Email did not match!"]}, status: :unprocessable_entity and return
      end
    end
    if user_params[:current_password].present?
       if @current_user.has_valid_password?(user_params[:current_password])
        @current_user.update(password: user_params[:new_password])
      else
        render json: {errors: ["Current Password did not match!"]}, status: :unprocessable_entity and return
      end
    end
    if @current_user.update(sanitized_params)
      @web_user = @current_user
      render :show
    else
      render json: {errors: @current_user.errors.full_messages}, status: :unprocessable_entity
    end
  end

private

  def sanitized_params
    user_params.slice!(:current_email,:new_email,:current_password,:new_password)
  end

  def user_params
    params.permit(:current_email, :new_email, :current_password, :new_password, :password_confirmation, :reminders_frequency,
                  :coaching_style, :coaching_style_status,:suggestion_preference,:language,:region)
  end

A few obvious things would be:

1) You shouldn't include current_email and current_password in the sanitized_params since you don't want to update these only validate them.

2) Similarly for new_email and new_password, you have aleady updated these manually

3) So really you aren't updating anything else currently.

4) If your first two tests fail you render a json error but don't stop processing so the last if clause will still execute and you will get a double render error. You should probably test all your conditions before executing any code.

I would consider moving the update logic into methods on the User model (update_email, #update_password that do the validation for you.


Matrix transpose function I have a matrix transpose function that takes arrays with a possibly different number of rows and columns.

How can I improve performance and code quality?

void matrix_transpose(float *matrix, int rows, int columns)
{
    int current_row;
    int current_column;
    float temp;
    float buffer[MATRIX_TRANSPOSE_BUFFER_SIZE];
    int buffer_position = 0;

    if(rows == columns)
    {
        for(current_row = 0; current_row < rows; ++current_row)
        {
            for(current_column = current_row; current_column < columns; ++current_column)
            {
                temp = *(matrix + current_row * columns + current_column);
                *(matrix + current_row * columns + current_column) = *(matrix + current_column * columns + current_row);
                *(matrix + current_column * columns + current_row) = temp;
            }
        }
    }
    else if(rows * columns < MATRIX_TRANSPOSE_BUFFER_SIZE)
    {
        for(current_column = 0; current_column < columns; ++current_column)
        {
            for(current_row = 0; current_row < rows; ++current_row)
            {
                buffer[buffer_position++] = *(matrix + current_row * columns + current_column);
            }
        }
        buffer_position = 0;
        while(buffer_position < rows * columns)
        {
            *matrix++ = buffer[buffer_position++];
        }
    }
}

Here's an example call:

float matrix[] = {
     1,  2,  3,  4,
     5,  6,  7,  8,
     9, 10, 11, 12,
    13, 14, 15, 16,
    17, 18, 19, 20
};

matrix_transpose(matrix, 5, 4);

The call can apparently fail if the matrix supplied is larger than the internal buffer in the function. Yet there is no indication that this was the case. matrix_transpose should return an error code indicating the result of the operation.
float buffer[MATRIX_TRANSPOSE_BUFFER_SIZE]; lives on the stack of the function. The stack is usually limited to a few MB (I think default in Visual Studio is 1MB, on Linux it's typically around 8MB but depends on the setup). Not sure how big the matrices will get but this might get you into trouble. Consider using either a static buffer (not thread safe) or allocate the buffer via malloc on the heap.
Instead of *(matrix + current_row * columns + current_column) you can also write matrix[current_row * columns + current_column] which is a bit easier on the eyes (imho).
It is accepted practice that loop counters can be single letter variables. In conjunction with the fact that you operate on a 2 dimensional array you could consider renaming current_row into y and current_column into x. Shortens the code somewhat and doesn't loose much meaning. Using x and y to identify positions in a 2D structure is common practice.
When you transpose a matrix which is not square then the calling code will not be directly exposed to the fact that rows and columns are now transposed. You should pass rows and columns "by reference" (int *) and swap them before you return from the function. Otherwise the calling could needs to do that every time it calls the transpose function and the programmer is bound to forget it at one point.


A Simple Cluster Generator v0.1 [NOTE] This question can be depreciated in favor of version 0.2.

The purpose of this code is to produce a universe of points, randomly generated around predetermined centroids, provided as a vector of vector. The final product is a file of sample points, to be used for fake data analysis in another program. The objective here was brevity and speed - I feel the code could be vastly concisified, but it is working.

Detailed description of the primary algorithm parameters is in the .h.

// clustergen.h

#include <string>
#include <vector>
#include <iostream>
#include <fstream>
#include <sstream>
#include <random>
#include <chrono>

#ifndef CLUSTERGEN_H
#define CLUSTERGEN_H

// POINT GENERATION - COMMON USER DEFINED VARIABLES
double const PT_BOUND = 10;   // Defines the +/- around a centroid for point generation if UNIFORM
double const PT_SD = 5;       // Defines the stddev around a centroid for point generation if NORMAL

// PRIMARY ALGORITHM
void clustergen(unsigned int k, std::vector<std::vector<double>> &c, std::string file_out, std::string file_rpt, bool csv, bool norm);
// Produces [k] points in [file_out]; all points are separated by line breaks
// If [csv] = 0, dimensions for each point are whitespace-separated
// If [csv] = 1, dimensions for each point are comma-separated
// If [norm] = 0, points will be UNIFORMLY generated around centroids
// If [norm] = 1, points will be NORMALLY generated around centroids
// [c] is a vector of vectors; it's size is the number of centroids
// The first insertion in [c] sets dimensional precedence; dimensional mismatches are always omitted/avoided.

#endif //CLUSTERGEN_H

// clustergen.cpp

#include "clustergen.h"

void clustergen(unsigned int k, std::vector<std::vector<double>> &c, std::string file_out, std::string file_rpt, bool csv, bool norm) {

    std::ofstream fout, rout;
    fout.open(file_out);
    rout.open(file_rpt);

    rout << "CLUSTERGEN STATUS REPORT FOLLOWS...\n";

    unsigned int ct = 0;
    unsigned int cl_ct = 0;

    for (std::vector<std::vector<double>>::iterator c_iter = c.begin(); c_iter != c.end(); ++c_iter) {
        unsigned int subsec;
        std::default_random_engine gen(std::chrono::system_clock::now().time_since_epoch().count());

        (k % c.size() == 0) ? (subsec = (k + ct) / c.size()) : (subsec = 1 + ((k + ct) / c.size()));
        k -= subsec;

        for (unsigned int i = 0; i < subsec; i++) {
            std::vector<double>::iterator d_iter = (*c_iter).begin();
            while (d_iter != (*c_iter).end()) {
                double num;
                if (norm) {
                    std::normal_distribution<double> distr((*d_iter), PT_SD);
                    num = distr(gen);
                } else {
                    std::uniform_real_distribution<double> distr((*d_iter) - PT_BOUND, (*d_iter) + PT_BOUND);
                    num = distr(gen);
                }
                fout << num;
                std::vector<double>::iterator temp_d_iter = d_iter;
                std::vector<std::vector<double>>::iterator temp_c_iter = c_iter;
                if (++temp_d_iter != (*c_iter).end()) {
                    (csv == 0) ? (fout << " ") : (fout << ",");
                } else if ((i != subsec - 1) || (++temp_c_iter != c.end())) {
                    fout << "\n";
                }
                ++d_iter;
            }
            ++ct;
        }
        rout << "\n" << div << " points ";
        (norm) ? (rout << "normally") : (rout << "uniformly");
        rout << " distributed around centroid " << ++cl_ct << " ...";
    }
    rout << "\n\n" << ct << " total points assigned.";
}

// main.cpp
#include "clustergen.h"

int main() {

    std::vector<std::vector<double>> v = {{0,0}, {50,30}, {100,120}};
    clustergen(110, v, "clustergen_out.dat", "clustergen_report.dat", 1, 0);

}

Overview:

Instantiating a random engine is relatively expensive. Also creating one each loop does not increase your entropy (more likely decreases but I am not a mathamtician so don't homd me to that). BUT it is fine to create it once:

// Do this once at the beginning of the function
std::default_random_engine gen(std::chrono::system_clock::now().time_since_epoch().count());

Modern C++ techniques:

// C++ 03
for (std::vector<std::vector<double>>::iterator c_iter = c.begin(); c_iter != c.end(); ++c_iter)

In C++ 11 we can let the compiler determine the type:

// C++ 11
for (auto c_iter = c.begin(); c_iter != c.end(); ++c_iter)

In C++14 they introduced range based for()

// C++ 14
for (auto& cValue : c) //  Note:: cValue => *c_iter from the above examples.

Looking at the inner loop:

    std::vector<double>::iterator d_iter = (*c_iter).begin();
    while (d_iter != (*c_iter).end()) {
        // STUFF
        ++d_iter;
    }

We replace this while loop with a for loop.

   for (auto d_iter = (*c_iter).begin(); d_iter != (*c_iter).end(); ++d_iter)

Which of course we can then apply all the above transformations on and get the same result:

   for (auto dValue: *c_iter)

We should also note that -> is short hand for (*).

   // eg:
   (*c_iter).begin()

   // written more succinctly as:
   c_iter->begin()

Over Complex

(k % c.size() == 0) ? (subsec = (k + ct) / c.size()) : (subsec = 1 + ((k + ct) / c.size()));

First of that is hard to read. Spread it out:

(k % c.size() == 0)
    ? (subsec = (k + ct) / c.size())
    : (subsec = 1 + ((k + ct) / c.size()));

Second this relies on sideeffects of the expression. This is usually not a good idea. Let expressions by simple (with no side affects). Use the result of the whole thing to do the assignment:

subsec = (k % c.size() == 0)
    ? ((k + ct) / c.size())
    : (1 + ((k + ct) / c.size()));

// Also this:
rout << (norm) ? "normally" : "uniformly";  // from your monstrocity.

I also think I see a common sub expression in there. Extract that and assign it to a meaningful variable name to help document your code:

auto someMeaningfulName = (k + ct) / c.size();
subsec = (k % c.size() == 0)
    ? someMeaningfulName
    : 1 + someMeaningfulName;

OK. Now I can make this even more readable:

auto someMeaningfulName = (k + ct) / c.size();
subsec = someMeaningfulName + ((k % c.size() == 0) ? 0 : 1);

One line per declaration

std::ofstream fout, rout;
fout.open(file_out);
rout.open(file_rpt);

Use one line to declare each variable (and put some more meaningful names on them). Also there is no reason to explicitly call the open() method as this is handeled by the constructor:

std::ofstream fout(file_out);
std::ofstream rout(file_rpt);


Custom MySQLi statement function for binding variables I have the following functions:

function db ($query, $parameters = false, $results = false, $fetch = false, $error_reporting = true) {

    global $db;
    global $db_query;

    if(!isset($db)) {

        $db = new mysqli("localhost", "username", "password", "db");

    }

    if ($db->connect_error == NULL) {

        $db->set_charset("utf8");

        if(is_array($parameters)) {

            $db_query = $db->prepare($query);

            if($db_query) {

                foreach($parameters as $key => $value) {
                    $parameters_reference[$key] = &$parameters[$key];
                }

                $ReflectionMethod = new ReflectionMethod ('mysqli_stmt', 'bind_param');
                @$ReflectionMethod -> invokeArgs($db_query, $parameters_reference);

                $execute = $db_query->execute();

                if($execute) {

                    if($results === false && $fetch === false) {

                        $return = $db_query;

                    } else {

                        if($results === true) {
                            $return = $db_query->get_result();
                        }

                        if($results === true && $fetch === true) {
                            $return = $return->fetch_array(MYSQLI_ASSOC);
                        }
                    }

                } else {
                    $db_error = "Execution error.";
                }

            } else {
                $db_error = "Preparation error.";
            }

        } else {

            $return = $db->query($query);

            if($parameters === true) {
                $return = $return->fetch_array(MYSQLI_ASSOC);;
            }

        }

    } else {
        $db_error = "Connection error.";
    }

    if(!isset($db_error)) {

        return $return;

    } else {

        if($error_reporting === true) {

            die("Database Error: ".$db_error);

        } else {

            return false;
        }
    }
}

function db_fetch($query) {
    return $query->fetch_array(MYSQLI_ASSOC);
}

Their main purpose is to save me time when executing queries to the database. The usage is as follows:

$query = db("SELECT * FROM `table`");

When I need to execute a simple query:

$query = db("SELECT * FROM `table` WHERE `var` = ?", ['i', $var], true);

When I need to add some variables to the query. Then I use $row = db_fetch($query) to get the results. It's possible to skip the db_fetch function if i set 4th parameter of the db function to true.

How can I make the functions better? I mean, is it written well or can I change something to make it better?

An obligatory suggestion: you can greatly improve any mysqli code by switching to PDO. So your function would be three times shorter

function db ($query, $parameters = false)
{
    global $db;
    if(!isset($db))
    {
        $options = array(
            PDO::ATTR_ERRMODE            => PDO::ERRMODE_EXCEPTION,
            PDO::ATTR_DEFAULT_FETCH_MODE => PDO::FETCH_ASSOC,
            PDO::ATTR_EMULATE_PREPARES   => FALSE,
        );
        $dsn = 'mysql:host=localhost;dbname=db;charset=utf8';
        $db = new PDO($dsn, "username", "password", $options);
    }

    if (!$parameters)
    {
        return $db->query($query);
    } else {
        $stmt = $db->prepare($query);
        $stmt->execute($parameters);
        return $stmt;
    }
}

note that PDO has much better equivalent of your db_fetch() function, letting you to get the data simpler and in more formats than your awkward $results / $fetch combination:

$row = db("SELECT * FROM t WHERE id=?", [$id])->fetch();
$count = db("SELECT count(*) FROM t")->fetchColumn();
$data = db("SELECT name, id FROM t")->fetchAll(PDO::FETCH_KEY_PAIR);

compare these explicit calls where you always can tell what does your code return with your

 $data = db("SELECT name, id FROM t", true, true);

show this code to someone and ask what's in $data. Or look at this code after a couple of months and try to tell it yourself without confusing all these true ans false parameters.