Password generator class I've written this class to generate a random password and was wondering if there's anything I could improve upon here and whether or not this a safe (most random) method of generating a password.

There's only one function in here which can be used to create an alphanumerical password of a specified length. It can also be overloaded to be more specific in which characters it will use.

The code works as it's supposed to.

import UIKit

class codeGen: NSObject {

    private let base: String = "ABCDEFGHIJKLMNOPQRSTUVWXYZ"
    private let lowerChars: String = "abcdefghijklmnopqrstuvwxyz"
    private let numberChars: String = "1234567890"
    private let specialChars: String = "!@#$%^&*"

    func generate(length: Int) -> String
    {
        let letters : NSString = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789"
        let len = UInt32(letters.length)

        var result = ""

        for _ in 0 ..< length {
            let rand = arc4random_uniform(len)
            var nextChar = letters.character(at: Int(rand))
            result += NSString(characters: &nextChar, length: 1) as String
        }

        return result
    }

    func generate(length: Int, lowers: Bool, numbers: Bool, specials: Bool) -> String
    {
        var letters = base

        if (lowers == true)
        {
            letters += lowerChars
        }
        if (numbers == true)
        {
            letters += numberChars
        }
        if (specials == true)
        {
            letters += specialChars
        }

        let NSletters : NSString = letters as NSString
        let len = UInt32(NSletters.length)

        var result = ""

        for _ in 0 ..< length {
            let rand = arc4random_uniform(len)
            var nextChar = NSletters.character(at: Int(rand))
            result += NSString(characters: &nextChar, length: 1) as String
        }

        return result
    }

}

Type names in Swift use UpperCamelCase (compare API Design Guidelines), so it should
be class CodeGen.

I do not see any reason why the class must inherit from NSObject.
Actually the methods do not use any state. If the sole purpose of
the class declaration is to provide a namespace (and to avoid free
global function) then you use an enum with type methods instead:

enum CodeGen {

     static func generate(length: Int) -> String { ... }

}

so that you can call them as

let password = CodeGen.generate(length: 16)

instead of creating an instance:

let password = CodeGen().generate(length: 16)

The advantage of usage an enum over a struct is that you can not
(inadvertently)  create an instance of the type
(compare https://github.com/raywenderlich/swift-style-guide#constants,
or Swift constants: Struct or Enum).

The explicit type annotations in

private let base: String = "ABCDEFGHIJKLMNOPQRSTUVWXYZ"
// ...

are not necessary, that can be simplified to

private let base = "ABCDEFGHIJKLMNOPQRSTUVWXYZ"
// ...

You have two functions implemented independently:

func generate(length: Int) -> String

func generate(length: Int, lowers: Bool, numbers: Bool, specials: Bool) -> String

but actually the first function is a special case of the second.
Therefore the first method can simply forward to the second:

func generate(length: Int) -> String {
    return generate(length: length, lowers: true, numbers: false, specials: false)
}

Alternatively, implement only a single method with default
parameter values:

func generate(length: Int, lowers: Bool = true, numbers: Bool = false, specials: Bool = false) -> String

Testing a boolean value if (lowers == true) can always be
simplified to if (lowers), and the parentheses are not needed:

if lowers {
    letters += lowerChars
}

The type annotation in

let NSletters : NSString = letters as NSString

is again unnecessary, and variable names should start with a lowercase
letter. But the bridging to NSString can be avoided completely by creating a Swift Array of Character:

let characters = Array(letters.characters)
let numChars = UInt32(characters.count)
var result = ""
for _ in 0 ..< length {
    let rand = Int(arc4random_uniform(numChars))
    let nextChar = characters[rand]
    result.append(nextChar)
}

which can be simplified to

let characters = Array(letters.characters)
let numChars = UInt32(characters.count)
let result = String((0..<length).map { _ in
    characters[Int(arc4random_uniform(numChars))]
})

The argument names can be improved, perhaps useLowercase or
withLowercase instead of lowers etc.

Putting it all together the code would look like this:

enum CodeGen {

    private static let base = "ABCDEFGHIJKLMNOPQRSTUVWXYZ"
    private static let lowerChars = "abcdefghijklmnopqrstuvwxyz"
    private static let numberChars = "1234567890"
    private static let specialChars = "!@#$%^&*"

    static func generatePassword(length: Int, useLowercase: Bool = true,
                                 useNumbers: Bool = false,
                                 useSpecialChars: Bool = false) -> String {
        var letters = base
        if useLowercase {
            letters += lowerChars
        }
        if useNumbers {
            letters += numberChars
        }
        if useSpecialChars {
            letters += specialChars
        }

        let characters = Array(letters.characters)
        let numChars = UInt32(characters.count)
        let result = String((0..<length).map { _ in
            characters[Int(arc4random_uniform(numChars))]
        })

        return result
    }

}

The static variables can also be moved into the method unless
you need to share them with other methods of the CodeGen type.


Setting a value of an iframe in the parent This code works on Chrome 58 but it makes me feel dirty:

parent.parent.parent.document.F1.document.getElementById("txtNewDrugAction").value = "foobar"

F1 is the name of the iFrame where the text box resides.

I am setting the value of an input box from a modal popup window (via jQuery UI).  I've tried to use selectors etc, but to no avail.  I feel like there would be a hugely easier way to do this that would also be more resilient to changes in the DOM but I can't see it just yet...

I also wind up writing code like this (again, functional, but awful):

window.top.document.getElementById("F1").contentWindow.document.getElementById("txtRefresh") = "foobar"

I'm maintaining a legacy application. Please nobody think that something that uses frames is being created in 2017, especially not by me.

The simplest would be to use window.top if F1 is part of the top-most window, or possibly go top down window.top.someframe.someotherframe.F1.

An alternative might be to write a helper something like:

function getFrame(name) {
  var current = this;
  while (current) {
    if (current.document[name])
      return current.document[name].document;
    current = current.parent;
  }
  return undefined; // not found
}


Implementation of Permutation Algorithm A from Topor, 1982 This is Algorithm A from Functional Programs for Generating Permutations (Topor, 1982). The author states that the algorithm was described in TAoCP Vol 1.

We have implemented Algorithm A in F# as follows.

let rec put (a: 't) (p: List<'t>) (q: List<'t>) : List<'t> =
    if p = q then a :: q
    else p.Head :: (put a p.Tail q)

// a: an element
// p: a list of elements
// q: a sublist of p
// ps: a list of permutations
let rec insert (a: 't) (p: List<'t>) (q: List<'t>) (ps: List<List<'t>>) : List<List<'t>> =
    if q.Length = 0 then (put a p q) :: ps
    else (put a p q) :: (insert a p q.Tail ps)

let rec mapinsert (a: 't) (ps: List<List<'t>>) : List<List<'t>> =
    if ps.Length = 0 then List.empty<List<'t>>
    else insert a ps.Head ps.Head (mapinsert a ps.Tail)

// x: a list of elements
// returns a list of permutations
let rec permute1 (x: List<'t>) : List<List<'t>> =
    if x.Length = 0 then [x]
    else mapinsert x.Head (permute1 x.Tail)

Here is a usage example

permute1 [1;2;3] |> List.iter (printf "%A ")
// [1; 2; 3] [2; 1; 3] [2; 3; 1] [1; 3; 2] [3; 1; 2] [3; 2; 1]

We would like a review to make our F# more canonical. That is, we would like to adjust our code to be in keeping with common F# styles and techniques.

Alternatively, we would like a reviewer to show alternative techniques (if not better ones) for implementing Algorithm A in F#.

Echoing Henrik's answer, I'd agree that the implementation looks fairly idiomatic already.

The only material change I'd make is changing all uses of List.length to explicit pattern matching with an empty list case.  This is because List.length is an O(n) call due to the linked list implementation of F# Lists, when all you really care about is if the list is empty. As an example:

module AlgorithmA =
  let rec put a p q =
    if p = q then a :: q
    else p.Head :: (put a p.Tail q)

  // a: an element
  // p: a list of elements
  // q: a sublist of p
  // ps: a list of permutations
  let rec insert a p q ps =
    match q with
    | [] -> put a p [] :: ps
    | _::qs as q' -> put a p q' :: insert a p qs ps

  let rec mapInsert a ps =
    match ps with
    | [] -> []
    | x::xs -> insert a x x (mapInsert a xs)

  // x: a list of elements
  // returns a list of permutations
  let rec permute1 x =
    match x with
    | [] -> [[]] // return an list containing the empty list
    | x::xs -> mapInsert x (permute1 xs)


Gluing pieces of scrambled text (challenge on CodeEval) I'm trying to solve this challenge on CodeEval. Quoting:

For example, you can put pieces together and get the original text:
evil pl
vil pla
il plan
The answer is ‘evil plan’.
Your task is to print out the original text. Due to repetitions in the text, you will sometimes get identical pieces.
Your program should accept a path to a file as its first argument. Each line in the file is one test case with the pieces of shredded text separated by a pipe. Each test case starts and finishes with symbol '|'.
For example:
|deEva|lan t|to ha|evil |ankin|il-ev|o hac| to h|vil p|an to|The e|CodeE| evil|plan |hack |Eval |ack C|l ran|king.|l-evi|evil-|-evil|l pla|il pl| hack|al ra|vil-e|odeEv|he ev|n to |ck Co|eEval|nking| rank| Code|e evi|ranki|k Cod| plan|val r|
Print to stdout the original text for each test case:
The evil-evil plan to hack CodeEval ranking.
CONSTRAINTS:
For the text with the length t shredded into pieces with the length n, there are t - (n - 1) pieces of text in the input file. Each piece of text is shifted by one character. For example, the word ‘secret’ and n = 4:
secr
ecre
cret
There is only one correct answer for each test case.
The minimum number of pieces is 125, the maximum number is 975.
The minimum length of a piece of text is 8, the maximum length is 28.
The number of test cases is 20.

My algorithm produces correct output but it takes longer than maximum 10 seconds for challenge input. Can't check the input though because it is hidden.
static bool SubstringEquals(StringBuilder s1, string s2, int start1, int start2, int l)
{
    for (var i = 0; i < l; i++)
        if (s1[start1 + i] != s2[start2 + i]) return false;

    return true;
}

static int CompareFragments(StringBuilder s1, string s2, int l)
{
    if (SubstringEquals(s1, s2, 0, s2.Length - l, l)) return -1;
    if (SubstringEquals(s1, s2, s1.Length - l, 0, l)) return 1;
    return 0;
}

static string GlueFragments(List<string> a)
{
    var l = a[0].Length - 1;
    var s = new StringBuilder(a[0], a.Count + l);
    a.RemoveAt(0);

    while (a.Any())
    for (var i = a.Count - 1; i >= 0; i--)
    {
        var j = CompareFragments(s, a[i], l);
        if (j == 1) s.Append(a[i][l]);
        if (j == -1) s.Insert(0, a[i][0]);
        if (j != 0) a.RemoveAt(i);
    }

    return s.ToString();
}

static IEnumerable<string> ParseInput(string s)
{
    int l = s.IndexOf('|', 1) - 1;

    for (var i = 1; i < s.Length; i += l + 1)
        yield return s.Substring(i, l);
}

static void Main(string[] args)
{
    using (var r = File.OpenText(args[0]))
    while (!r.EndOfStream)
    {
        var a = ParseInput(r.ReadLine()).ToList();
        Console.WriteLine(GlueFragments(a));
    }

}

Any tips? Is it possible to achieve less than O(n2) complexity? Maybe there's some data structure that fits this problem perfectly I'm unaware of.

First of all I don't think the solution is correct. There could be multiple candidates to combine. Consider for example that the file contains, among other, the abc, bcd and bce strings, in this order. Your code unconditionally joins abc and bcd to produce abcd whereas the actual text has abce.

Also, it looks like you assume that at each iteration of the outer loop at least one string is removed. It is not necessarily so. If - due to a wrong candidate selection, see above - you happen to reach a dead end (unable to append or prepend any string), you are in the infinite loop. That explains TLE much better than the quadratic nature of the algorithm.

I am afraid you need some sort of backtracking. Compute the "successor" relation between fragments, and join them in a DFS with backtracking. Keep in mind that succession does not impose a strict order (e.g. aba and bab are each other successors).


Game of life C++ implementation memory access bug I'm learning C++ so I thought I'd make game of life for practice and explore pointers, 1.5D arrays and stuff like that.

I have "finished" my implementation, but I have trouble with memory access. It seems like I'm stepping over the bounds somewhere and I simply can't figure out where. Everything looks fine to me.

Problem is most likely in the function called void World::follow that is checking surrounding cells if they are alive and than sets state for center cell for the next generation/tick depending of how many neighbors it has but I simply don't see it.

Here is my code:

source.cpp

#include "World.h"

int main() {
    World world = World(40, 20, 20);
    while (world.update())
        world.render();
}

Organism.h

#pragma once

class Organism {
private:
    bool cState, nState;
public:
    Organism(bool state);
    void perish();
    void revive();
    bool status();
    void update();
};

World.h

#pragma once
#include "Organism.h"

class World {
private:
    Organism** plain;
    int population;
    short width, height;
public:
    World(short width, short height, short chance);
    ~World();
    bool update();
    void render();
    void follow(short x, short y);
};

Organism.cpp

#include "Organism.h"

Organism::Organism(bool state) {
    this->cState = state;
}

void Organism::perish() {
    this->nState = false;
}

void Organism::revive() {
    this->nState = true;
}

bool Organism::status() {
    return this->cState;
}

void Organism::update() {
    this->cState = this->nState;
}

World.cpp

#include "World.h"

#include <time.h>
#include <stdlib.h>
#include <iostream>
#include <Windows.h>

using namespace std;

World::World(short width, short height, short chance) {
    this->population = 0;
    this->width = width; this->height = height;

    this->plain = new Organism*[width*height];

    bool state;
    srand(time(nullptr));
    for (int i = 0; i < (width*height); i++) {
        state = rand() % 100 < chance;
        this->plain[i] = new Organism(state);
    }

    for (short x = 0; x < this->width; x++)
        for (short y = 0; y < this->height; y++)
            this->follow(x, y);
}

bool World::update() {
    this->population = 0;

    for (int i = 0; i < (width*height); i++) {
        this->plain[i]->update();
        if (this->plain[i]->status())
            this->population++;
    }

    for (short x = 0; x < this->width; x++)
        for (short y = 0; y < this->height; y++)
            this->follow(x,y);

    return this->population > 0;
}

void World::follow(short x, short y) {
    bool alive;
    short pals = 0;
    Organism* current = this->plain[x*this->width + y];

    short sx = (x - 1 < 0) ? x : x - 1;
    short sy = (y - 1 < 0) ? y : y - 1;
    short ex = (x + 1 == this->width - 1) ? x : x + 1;
    short ey = (y + 1 == this->height - 1) ? y : y + 1;

    for (short row = sx; row < ex; row++) {
        for (short col = sy; col < ey; col++) {
            alive = this->plain[row*this->width + col]->status();
            if (row != x && col != y && alive) pals++;
        }
    }

    if (!current->status() && pals == 3)
        current->revive();
    else if (current->status() && pals != 2 && pals != 3)
        current->perish();
}

void World::render() {
    Sleep(500);
    system("cls");
    for (short x = 0; x < this->width; x++) {
        for (short y = 0; y < this->height; y++) {
            cout << (this->plain[x*this->width + y]->status()) ? " " : "x";
        }
        cout << endl;
    }
}

World::~World() {
    for (int i = this->width * this->height; i >= 0; i--)
        delete this->plain[i];
    delete[] this->plain;
}

Stop doing manually memory management (this is not C).
Organism** plain;

There is no reason to have an array of pointers. You could just have an array of object (thus you don't need to manage the individual cells).
Organism*  plain;
....
plain = new Organism[width * height]; // Allocates and calls default constructor.
                                      // You will still need to loop over
                                      // them to reset them to an initial good
                                      // state (but you will not have) to
                                      // manage the memory for individual cells.

Because the cells are no longer pointers you will need to replace -> with . (a dot). But other than than no difference.
Part 2 of memory management
Take this a step further. You don't need to manually manage the memory for the array of plain. We have classes that do that for you.
std::vector<Organism>  plain;        // initially this holds zero elements
...

plain.reserve(width * height);       // Let the object know you are going to
                                     // add a lot of cells so it can pre-allocate space.

for (int i = 0; i < (width * height); i++) {
    state = rand() % 100 < chance;
    plain.emplace_back(state);       // add a new element to the end.
    // If you are using C++03 you may not have emplace_back()
    // So use the regular push_back instead (it does the same thing).
    //    plain.push_back(Organism(state));
}

You can access plain (a std::vector<>) with operator [] like you would an array. So no other code needs to change (you just need any new or delete calls). So you have no memory management calls and thus no headaches with leaks.
Now a nice bit is you can replace plain[x] with plain.at(x) and it checks to make sure you access to the array is in bounds (and will throw an exception if not). This may help you track down your error.
Initialize srand() one
You should only initializre the random number generator once in a program.
srand(time(nullptr));

So it is best to do this in main(). In your application you are doing this once for each game object. This may not seem like a big thing. But of your code get's re-used by another application that does not realize you are doing this. You mess up other people's code.
You should also note. This old random number generator is considered pretty bad. You should probably start looking at the new ones provided by C++11. They are much better and can be used in ways that don't interact (so you don't screw over other people's random numbers).
Move Platform specific code to its own function.
Sleep(500);
system("cls");

You should probably put this in its own function. So when people come to port your code to another platform they just need to provide an alternative implementation to a couple of specific functions and the rest of the code can stay the same.


Calculate time difference when using time only fields Most of the time I used datetime fields, but for this project it is a requirement to use time fields. The problem is calculations with 2 times not on the same date.

I.E.:

23:00 to 03:00 is 4 hours.

In vba I'm using the following code:

Me.TotalTime = IIf(Round(CCur(Nz(DateDiff("n", [startTime], [endTime]) / 60, 0)), 2) < 0, Round(CCur(Nz(DateDiff("n", [startTime], 1) / 60, 0)), 2) + Round(CCur(Nz(DateDiff("n", 0, [endTime]) / 60, 0)), 2), Round(CCur(Nz(DateDiff("n", [startTime], [endTime]) / 60, 0)), 2))

Is this the correct way to calculate and or is there an easier method?

You don't seem to require worrying about time zones or daylight savings. So if you just want a simple calculation to determine the duration between two times, then a simple function will work:

Function DurationDiff(startTime As Double, endTime As Double) As Double
    '--- assumes startTime is the start time in decimal hours (0.0 to 23.99)
    '            endTime   is the ending time in decimal hours
    '    returns duration between the times in decimal hours
    '        (accounts for time crossing midnight)
    If endTime < startTime Then
        '--- midnight crossing!
        endTime = endTime + 24
    End If
    DurationDiff = endTime - startTime
End Function

Running a quick test with this function gives you:

Sub DurationTest()
    Debug.Print DurationDiff(startTime:=9#, endTime:=17#) & " hours"
    Debug.Print DurationDiff(startTime:=23#, endTime:=3#) & " hours"
    Debug.Print DurationDiff(startTime:=11#, endTime:=15#) & " hours"
    Debug.Print DurationDiff(startTime:=15#, endTime:=3#) & " hours"
End Sub

With the output:

8 hours
4 hours
4 hours
12 hours

I think it then reduces your calculation to nothing more than

Me.TotalTime = Nz(DurationDiff([startTime], [endTime]))


Prime number sequence supporting repeated iteration I'm a long-time Java programmer, but recently I decided to give Kotlin a try.  So far, I'm loving the language features, it feels much more expressive than what I'm used to.  As practice, I wrote this function that produces a repeatable sequence of prime numbers (i.e. the sequence can be iterated multiple times, and will make use of results from past iterations to speed up future iterations).  It feels pretty solid to me, but I'm wondering how it would look to a Kotlin veteran - are there language features that I'm not taking full advantage of, or conventions that I should be following?
One question I have in particular is regarding top-level function vs. subclass; here I chose to implement a top-level function primeSequence(): Sequence<Long> that returns an object, but I could have just about as easily implemented a class PrimeSequence : Sequence<Long>.  Is there a good rule of thumb for when to use one or the other?
fun primeSequence(): Sequence<Long> = object: Sequence<Long> {
    private val knownPrimes: MutableList<Long> = arrayListOf(2L, 3L)

    private fun Long.isMultipleOf(n: Long) = (this%n == 0L)

    private fun Long.isPrime() = knownPrimes
            .takeWhile { it*it <= this }
            .none { this.isMultipleOf(it) }

    override fun iterator(): Iterator<Long> = object: Iterator<Long> {
        private var lastPrime: Long? = null

        override fun hasNext() = true

        override fun next(): Long {
            val nextPrime = when(val lastPrime = this.lastPrime) {
                null -> 2L
                2L -> 3L
                else -> {
                    if (knownPrimes.last() > lastPrime ) {
                        knownPrimes[knownPrimes.binarySearch(lastPrime) + 1]
                    } else {
                        val np = generateSequence(lastPrime+2) { it+2 }
                                .first { it.isPrime() }
                        knownPrimes.add(np)
                        np
                    }
                }
            }
            this.lastPrime = nextPrime
            return nextPrime
        }
    }
}

I don't like the idea of returning an inline object that inherits Sequence<Long> like this. Kotlin has several features to help you build stuff, including one for sequences.
Using the fantastic power of the sequence builder in Kotlin, which underneath the surface is using a suspendable function internally, we can yield numbers one by one instead of implementing an Iterator.
We can rewrite your code to use an outer object to keep the global state in, and then use a function to generate the sequence.
object Primes {
    private val knownPrimes: MutableList<Long> = mutableListOf(2L, 3L)
    private fun Long.isMultipleOf(n: Long) = (this % n == 0L)

    private fun Long.isPrime() = knownPrimes
            .takeWhile { it * it <= this }
            .none { this.isMultipleOf(it) }

    fun sequence(): Sequence<Long> {
        var lastPrime: Long? = null
        return sequence {
            while (true) {
                val nextPrime = when(val actualLastPrime = lastPrime) {
                    null -> 2L
                    2L -> 3L
                    else -> {
                        if (knownPrimes.last() > actualLastPrime) {
                            knownPrimes[knownPrimes.binarySearch(actualLastPrime) + 1]
                        } else {
                            val np = generateSequence(actualLastPrime + 2) { it+2 }
                                .first { it.isPrime() }
                            knownPrimes.add(np)
                            np
                        }
                    }
                }
                lastPrime = nextPrime
                yield(nextPrime)
            }
        }
    }
}


Convertible templated math vector I've made a templated math vector struct with a templated type and templated dimension count.

I want my vectors to be convertible so I can easily make, for example, a vec<int, 3> from a vec<float, 3>

I want to do it in a safe way, or at least I'd like to know the ways it's unsafe.

I've never used static_cast or dynamic_cast and I've never used the operator user-defined conversion.

Here is what I came up with (only the relevant parts of the struct):

template<class t, size_t dimensions> struct vec{
    typedef t coord_type;
    static constexpr size_t dimension_count = dimensions;

    std::array<coord_type, dimension_count> coords;

    template<class new_coord_type> operator vec<new_coord_type, dimension_count>() const{
        std::array<new_coord_type, dimension_count> casted_coords;
        std::transform(
            coords.begin(),
            coords.end(),
            casted_coords.begin(),
            [](coord_type coord){return static_cast<new_coord_type>(coord);}
        );
        return {casted_coords};
    }
};

It seems to work how I want it to. For example, this compiles and gives me the correct results:

vec<float, 3> v1{1.1f, 2.5f, 3.9f};
vec<int, 3> v2 = v1; // converted correctly to {1, 2, 3}

The only way I could figure out how to cast the contents of an array is to use std::transform. I'm not sure if there's a better way.

Final questions: Is this unsafe or incorrect? How can this be improved?

I had to adjust your code style a bit before I could easily read the code. In particular:

Use InitialCaps for template parameters
Add a space before {
A newline after template<...> generally improves readability

Thus:

template<class T, size_t Dimensions>
struct vec {
    using coord_type = T;
    static constexpr size_t dimension_count = Dimensions;

    std::array<T, Dimensions> coords;

    template<class U>
    operator vec<U, Dimensions>() const {
        std::array<U, Dimensions> casted_coords;
        std::transform(
            coords.begin(),
            coords.end(),
            casted_coords.begin(),
            [](T coord) {
                return static_cast<U>(coord);
            }
        );
        return {casted_coords};
    }
};

Notice that I personally prefer to use the original template parameters T, Dimensions, etc. in places where your code used the member typedefs coord_type, dimension_count etc.; I find that this improves readability but I believe reasonable people may differ on the subject.

(In particular, the C++ Standard always uses member typedefs in declarations such as reference operator*() const as opposed to T& operator*() const; but I think that's because the name reference is part of the API of the class, whereas the name T is given for exposition only. That's a Standard-ism that we don't necessarily need to emulate when writing our own code.)

Nit: Dimensions is technically the wrong name for a single dimension. Also, by capitalizing the template parameter, we've freed up the shorter name dimensions (or dimension) for the constexpr member variable. You never have to use a tediously long name like dimension_count if you manage your naming real estate effectively!

        return {casted_coords};

The extra braces here are a pessimization; they inhibit move-elision (a.k.a. Named Return Value Optimization). Remove them... oh wait, I see, casted_coords is a std::array, not a vec! Why are you constructing a separate array and then copying it into the vec? That seems pointless!

At the very least, this should be

return { std::move(casted_coords) };

but ideally you'd define casted_coords to be of type vec<U, Dimensions> and then transform the data straight into casted_coords.coords.begin().

            [](T coord) {
                return static_cast<U>(coord);
            }

This lambda takes its parameter by-copy, which is going to be expensive if T is, say, std::string. Prefer to take generic parameters by const& or (in C++14) by perfect-forward:

            [](const T& coord) {
                return static_cast<U>(coord);
            }

            [](auto&& coord) {
                return static_cast<U>(std::forward<decltype(coord)>(coord));
            }

In this case the former is better because it's less typing (and thus fewer chances to screw something up).

Your operator vec<U, Dimensions>() should be explicit, unless you have a darn good reason for it not to be. Most possible reasons I can think of do not qualify as "darn good."

In fact, I'd prefer to go further, and say that you shouldn't be using C++'s native "conversion" mechanisms for this kind of conversion at all. IMO you should write a user-defined function, along the lines of static_pointer_cast or any_cast, that does precisely and explicitly what you want.

template<class To, class From, size_t Dim>
auto static_vec_cast(const vec<From, Dim>& from) -> vec<To, Dim> {
    vec<To, Dim> to;
    std::transform(
        from.coords.begin(),
        from.coords.end(),
        to.coords.begin(),
        [](const From& x) {
            return static_cast<To>(x);
        }
    );
    return to;
}

int main() {
    vec<int, 3> a = {{1,2,3}};
    vec<float, 3> b = static_vec_cast<float>(a);
}

Season with SFINAE to taste.

However, this still has a major flaw: You're accidentally requiring that To be default-constructible! That's no good (especially if it does happen to be default-constructible but the default constructor is expensive). You can fix this issue, but it requires metaprogramming.

template<class To, class From, size_t Dim, size_t... Is>
auto static_vec_cast(const vec<From, Dim>& from, std::index_sequence<Is...>) -> vec<To, Dim> {
    vec<To, Dim> to = {{
        static_cast<To>(from.coords[Is])...
    }};
    return to;
}

template<class To, class From, size_t Dim>
auto static_vec_cast(const vec<From, Dim>& from) -> vec<To, Dim> {
    return static_vec_cast<To>(from, std::make_index_sequence<Dim>{});
}

The advantage of this approach is that it's hardly any more lines of code, but it's more correct, and it doesn't rely on running std::transform at runtime — it just generates the correct code directly inline.

Writing an efficient static_vec_cast<U>(vec<T, Dim>&&) is left as an exercise for the reader.


C++: swapping bytes of an uint64_t This question is about swapping bytes of an uint64_t value, or in other words, about converting between endianness of an integer value:
#include <algorithm>
#include <cstdint>
#include <iomanip>
#include <iostream>

uint64_t& swapBytes1(uint64_t& num) {
    using std::swap;

    union {
        uint8_t bytes[8];
        uint64_t num;
    } transformer;

    transformer.num = num;

    swap(transformer.bytes[0], transformer.bytes[7]);
    swap(transformer.bytes[1], transformer.bytes[6]);
    swap(transformer.bytes[2], transformer.bytes[5]);
    swap(transformer.bytes[3], transformer.bytes[4]);

    return num = transformer.num;
}

uint64_t& swapBytes2(uint64_t& num) {
    using std::swap;

    union {
        struct {
            uint8_t b0;
            uint8_t b1;
            uint8_t b2;
            uint8_t b3;
            uint8_t b4;
            uint8_t b5;
            uint8_t b6;
            uint8_t b7;
        } bytes;

        uint64_t num;
    } transformer;

    transformer.num = num;

    swap(transformer.bytes.b0, transformer.bytes.b7);
    swap(transformer.bytes.b1, transformer.bytes.b6);
    swap(transformer.bytes.b2, transformer.bytes.b5);
    swap(transformer.bytes.b3, transformer.bytes.b4);

    return num = transformer.num;
}

void main() {
    uint64_t num = 0x0123456789abcdef;
    std::cout << std::hex << num << "\n";
    std::cout << swapBytes1(num) << "\n";
    swapBytes2(num);
    std::cout << num << "\n";
}

Critique request
As I am not a professional C++ developer, I would like to receive any comment/answer that could improve my C++ coding habits.

Technically this is UB.
Writing a value to one member of a union, then reading from another member (that was not written to) of the union is undefined behavior.
You would be better off just casting to a char* and just reversing it.
swap(std::uint64_t& num)
{
    // I like the use of reinterpret_cast here
    // it makes sure that somebody reading the code notices
    // and takes special care to review what is happening.
    //
    char* first = reinterpret_cast<char*>(&num);
    char* last  = first + sizeof(num);

    // The advantage of reverse here:
    //
    // The standard library can have pre built optimizations
    // that will use hardware specific optimizations of this that
    // can convert it to a single instruction on some hardware.
    //
    std::reverse(first, last);
}

The question becomes are you doing this to support endianness?
If so then your code is only going to work on machines that have the same endianness as the platform you are currently working on.
Normally people want to swap from platform byte order to network byte order (or the reverse). Depending on architecture this is either a swap (as you have above) or a NO-OP or for some weird hardware something completely different.
To support this we have specific family of functions:
htonl()   // host to network long  (32 byte)
htons()   // host to network short (16 byte)

ntohl()   // network to host long  (32 byte)
ntohs()   // network to host short (16 byte)

These functions will do the appropriate conversion for your hardware.


Populating Lists with harcoded data I have a class MassiveDTO that contains lists. At some point in the project, I need to generate an object of this class, that will contain some hardcoded data.
I would like to know if there is a better way (or a design pattern) to populate those lists with known data.

With the method generateLists I populate them and I use them later on.
Keep in mind, I need this class not to be static because I reuse it on the project.

MassiveDTO class

public class MassiveDTO {

  private List<String> company = new ArrayList<>();
  private List<String> partners = new ArrayList<>();
  private List<String> biodata = new ArrayList<>();

  public MassiveDTO(){}

  public MassiveDTO(List<String> company, List<String> partners, List<String> biodata) {
    super();
    this.company = company ;
    this.partners = partners ;
    this.biodata = biodata;
  }

  /** Getters and Setters **/
  public List<String> getCompany() {
    return company;
  }

  public void setCompany(List<String> company) {
    this.company = company;
  }

  public List<String> getPartners() {
    return partners;
  }

  public void setPartners(List<String> partners) {
    this.partners = partners;
  }

  public List<String> getBiodata() {
    return biodata;
  }

  public void setBiodata(List<String> biodata) {
    this.biodata = biodata;
  }

  public void generateLists(){
    /** Company**/
    company.add("X0788");
    company.add("X0192");

    /** Partners **/
    partners.add("X0081");

    /** Biodata**/
    biodata.add("X0913");

  }

}

My first idea is to create another static class that will extend MassiveDTO in order to reuse those lists, and that class will be static because I will only need it to contain those specific lists with their data.

So you need to separate the construction of that class from the class itself? This is the Factory Pattern.

Data class

Boil down to your DTO to ... well the data.

public class MassiveDTO {
    private final List<String> company;
    private final List<String> partners;
    private final List<String> biodata;

  /**
   * @see MassiveDTOBuilder
   */
  MassiveDTO(List<String> company, List<String> partners, List<String> biodata) {
    this.company = company;
    this.partners = partners;
    this.biodata = biodata;
  }
   ... getters only
}

Factory Pattern

Then have a second class, MassiveDTOFactory, that calls that constructor with suitable fixed values.

public class MassiveDTOFactory {
  public static MassiveDTO create() {
    return new MassiveDTO(
      List.of("X0788", "X0192"), // Here we use Java 9 immutable collections
      List.of("X0081"),
      List.of("X0913")
    );
  }
}

Fluent Builder Pattern

If you wish more flexibility on the values then a Fluent Builder is for you. Something like this:

public class MassiveDTOBuilder {
  private List<String> companies = new ArrayList<>();
  private List<String> partners = new ArrayList<>();
  private List<String> biodata = new ArrayList<>();

  public static MassiveDTOBuilder massiveDto() {
    return new MassiveDTOBuilder();
  }

  private MassiveDTOBuilder() {
  }

  public MassiveDTOBuilder withCompany(String... items) {
    for (String i : items) {
      companies.add(i);
    }
    return this;
  }

  public MassiveDTOBuilder withPartner(String... items) {
    for (String i : items) {
      partners.add(i);
    }
    return this;
  }

  public MassiveDTOBuilder withBiodata(String... items) {
    for (String i : items) {
      biodata.add(i);
    }
    return this;
  }

  public MassiveDTO build() {
    return new MassiveDTO(
      Collections.unmodifiableList(companies),
      Collections.unmodifiableList(partners),
      Collections.unmodifiableList(biodata)
    );
  }
}

Which can used like:

MassiveDTO m = massiveDto()
  .withCompany("X0788", "X0192")
  .withPartner("X0081")
  .withBiodata("X0913")
  .build();


Minimalist context compressor I am working on an implementation of the minimalist compression method.
The method is based on context modeling, followed by a simple entropy coder (Golomb-Rice coder).
The context model uses a single previous byte in the uncompressed symbol stream to predict the next byte.
At the moment, my implementation works quite well.
However, I wonder if it is possible to make it faster or more concise.
Basically, any suggestions for improvement are welcome.

Prerequisites

The implementation uses the following data structure:

struct context {
    size_t freq[256];          /* char -> frequency */
    unsigned char sorted[256]; /* index -> char */
    unsigned char order[256];  /* char -> index */
} table[256];

The functions bio_open, bio_write_gr, bio_read_gr, and bio_close implement the entropy (Golomb-Rice) coding.

Moreover, the function void update_model(unsigned char delta) updates the entropy model according to the newly encoded byte delta (not the subject of this review).

Code

I use several two auxiliary functions:

static void swap_symbols(struct context *context, unsigned char c, unsigned char d)
{
    assert(context != NULL);

    unsigned char ic = context->order[c];
    unsigned char id = context->order[d];

    assert(context->sorted[ic] == c);
    assert(context->sorted[id] == d);

    context->sorted[ic] = d;
    context->sorted[id] = c;

    context->order[c] = id;
    context->order[d] = ic;
}

static void increment_frequency(struct context *context, unsigned char c)
{
    assert(context != NULL);

    unsigned char ic = context->order[c];
    size_t freq_c = ++(context->freq[c]);

    unsigned char *pd;

    for (pd = context->sorted + ic - 1; pd >= context->sorted; --pd) {
        if (freq_c <= context->freq[*pd]) {
            break;
        }
    }

    unsigned char d = *(pd + 1);

    if (c != d) {
        swap_symbols(context, c, d);
    }
}

The main entry functions are defined as follows:

void init()
{
    opt_k = 3;

    for (int p = 0; p < 256; ++p) {
        for (int i = 0; i < 256; ++i) {
            table[p].sorted[i] = i;
            table[p].freq[i] = 0;
            table[p].order[i] = i;
        }
    }
}

void *compress(void *iptr, size_t isize, void *optr)
{
    struct bio bio;
    unsigned char *end = (unsigned char *)iptr + isize;
    struct context *context = table + 0;

    bio_open(&bio, optr, BIO_MODE_WRITE);

    for (unsigned char *iptrc = iptr; iptrc < end; ++iptrc) {
        unsigned char c = *iptrc;

        /* get index */
        unsigned char d = context->order[c];

        bio_write_gr(&bio, opt_k, (uint32_t)d);

        assert(c == context->sorted[d]);

        /* update context model */
        increment_frequency(context, c);

        /* update Golomb-Rice model */
        update_model(d);

        context = table + c;
    }

    /* EOF symbol */
    bio_write_gr(&bio, opt_k, 256);

    bio_close(&bio, BIO_MODE_WRITE);

    return bio.ptr;
}

void *decompress(void *iptr, void *optr)
{
    struct bio bio;
    struct context *context = table + 0;

    bio_open(&bio, iptr, BIO_MODE_READ);

    unsigned char *optrc = optr;

    for (;; ++optrc) {
        uint32_t d = bio_read_gr(&bio, opt_k);

        if (d == 256) {
            break;
        }

        assert(d < 256);

        unsigned char c = context->sorted[d];

        *optrc = c;

        increment_frequency(context, c);

        update_model(d);

        context = table + c;
    }

    bio_close(&bio, BIO_MODE_READ);

    return optrc;
}

Symbolic Constants
This code could use some symbolic constants:
struct context {
    size_t freq[256];          /* char -> frequency */
    unsigned char sorted[256]; /* index -> char */
    unsigned char order[256];  /* char -> index */
} table[256];

void init()
{
    opt_k = 3;

    for (int p = 0; p < 256; ++p) {
        for (int i = 0; i < 256; ++i) {
            table[p].sorted[i] = i;
            table[p].freq[i] = 0;
            table[p].order[i] = i;
        }
    }
}

The use of symbolic constants would make this code easier to write and maintain. For example if there was a symbolic constant for 256 the size of the table and the loop controls could be changed with a single line edit.
Global Variables
In the function init() above, it is obvious that global variables are being used, table is on global variable and opt_k is another global variable.
Global variables are generally considered a bad programming practice, they make the code hard to write, debug and maintain. They also make it very easy to introduce bugs.
In the C programming language if the program consists of multiple files, it is possible that global variables can cause the program not to link if they are declared in multiple files.
Using assert() in C
If at some point the code is going to be compiled for release using the optimizing feature of the compiler, then the assert() statements will be optimized away. If the error checking provided by the asserts is required it will be better to replace the asserts with if statements.


Efficient Multithreaded File Processessing in Java This method in my code uses a producer-consumer design to read many files and process them for use in an NLP algorithm. The completion service collects files as they are processed, which means that docId != index. Given that I need to retrieve probabilities elsewhere where docID == index, I've implemented a priority queue to sort. However, I do this by sequentially retrieving documents from a list of futures and then putting them into the queue.

I know this implementation isn't great. I keep staring at the concurrentskiplistset thinking I should be able to implement it for better performance and not have these issues. I'm brand new to concurrency and Java, so I'm sure there's a lot to critique.

How can I make this multithreading more efficient?

Vectorizer:

public class Vectorizer {
public Vectorizer(){
}
public PriorityQueue<Document> readAll(File fileDir)
throws InterruptedException, ExecutionException, IOException{
    //read each file. When each file is vectorized, put it in a minibatch.
    //producer-consumer threading structure.
    int NUM_THREADS = Runtime.getRuntime().availableProcessors();
    BlockingQueue<LinkedList<String>> queue = new ArrayBlockingQueue<>(50);
    ExecutorService service = Executors.newFixedThreadPool(NUM_THREADS);
    CompletionService<Document> completionService =
               new ExecutorCompletionService<Document>(service);
    //TODO: is list the most efficient structure to handle future?
    List<Future<Document>> docs = new ArrayList<Future<Document>>();
    for (int i = 0; i < (NUM_THREADS - 1); i++) {
        docs.add(completionService.submit(new DocumentConsumer(queue)));
    }
    // Wait for ReadFile to complete
    service.submit(new ReadFile(queue, fileDir)).get();
    service.shutdownNow();  // interrupt CPUTasks
    // Wait for DocumentConsumer to complete
    service.awaitTermination(365, TimeUnit.DAYS);

    //do things with processed docs.
    //should I be doing this, though?
    PriorityQueue<Document> Documents = new PriorityQueue<Document>();
    for(Future<Document> d : docs){
        try{
        Document doc = d.get();
        Documents.add(doc);
        System.out.println(Integer.toString(doc.Cj));
        } catch(ExecutionException e) {
            e.getCause();e.printStackTrace();
        }
    }
    return Documents;
}

Document object (without compare and overload):

public class Document implements Comparator<Document>{
int docId, Cj;
HashMap<String,Integer> termTable;
public Document(int id,int Cj, HashMap<String,Integer> termMap){
    this.docId = id;
    this.Cj = Cj;

    this.termTable = termMap;
}

docId is set during file reading. I read in a file and append the count to the first line, which is then set during processing.

The rest of the code is available here.

I didn't look at the specific performance issue you described. However, my general review:

Readability

for (int i = 0; i < (NUM_THREADS - 1); i++) {
    docs.add(completionService.submit(new DocumentConsumer(queue)));
}

Starting at 0 and ending at somevalue - 1. Why don't you start at 1? You don't even do anything with the index, so it's not like it matters.

        e.getCause();e.printStackTrace();

Don't put multiple statements on the same line. It turns your code into a wall of code. You think understanding a wall of text is hard? Try a wall of code...

Naming

Use camelCase for methods and variables. Use PascalCase for types. Use ALL_CAPS for constants. This means that...

int NUM_THREADS = Runtime.getRuntime().availableProcessors();

should be final and

PriorityQueue<Document> Documents = new PriorityQueue<Document>();

should be renamed to documents.

public Document(int id,int Cj, HashMap<String,Integer> termMap){

Should rename Cj to cj. ... And even better would be to rename it to something that explains what it is. Right now I have no idea what a cj is.


Determining whether a number is a palindrome Can someone improve this for me? It seems ugly.

The method should test whether an incoming number is a palindrome and return a Boolean result.

private static bool IsPalindrome(int value)
{
    var inverseValue = string.Concat(value.ToString().Reverse());
    return inverseValue == value.ToString();
}

Not much there to be ugly to be honest.  I guess all I might consider would be the reverse of MrSmith42 and adding functionality to avoid the duplication of ToString().

private static bool IsPalindrome(int value)
{
    var valueStr = value.ToString();
    return valueStr == string.Concat(valueStr.Reverse());
}


Java Doubly Linked List I've been working on implementing a doubly linked list from scratch in Java, and if anyone has time, could you critique it?

class Node {
    Node prev;
    Node next;
    int data;

    public Node(int d) {
        data = d;
        prev = null;
        next = null;
    }
}

class LinkedList {
    Node head;

    public LinkedList() {
        head = null;
    }

    public void insert(int d) {
        if (head == null) {
            head = new Node(d);
            return;
        }

        if (head.data > d) {
            Node holder = head;
            Node newNode = new Node(d);
            head = newNode;
            head.next = holder;
            holder.prev = newNode;
            return;
        }

        Node tmpNode = head;

        while (tmpNode.next != null && tmpNode.next.data < d) {
            tmpNode = tmpNode.next;
        }

        Node prevTmp = tmpNode;
        Node insertedNode = new Node(d);

        if (tmpNode.next != null) {
            Node nextTmp = tmpNode.next;
            insertedNode.next = nextTmp;
            nextTmp.prev = insertedNode;
        }
        prevTmp.next = insertedNode;
        insertedNode.prev = prevTmp;
    }

    public void delete(int d) {
        if (head == null) {
            System.out.println("The list is empty.");
            return;
        }

        if (head.data == d) {
            head = head.next;
            if (head != null) {
                head.prev = null;
            }
            return;
        }

        Node tmpNode = head;

        while (tmpNode != null && tmpNode.data != d) {
            tmpNode = tmpNode.next;
        }

        if (tmpNode == null) {
            System.out.println("That node does not exist in the list");
            return;
        }

        if (tmpNode.data == d) {
            tmpNode.prev.next = tmpNode.next;
            if (tmpNode.next != null) {
                tmpNode.next.prev = tmpNode.prev;
            }
        }
    }

    public void print() {
        Node tmpNode = head;

        while (tmpNode != null) {
            System.out.print(tmpNode.data + " -> ");
            tmpNode = tmpNode.next;
        }

        System.out.print("null");
    }

Can use Generics for Node to provide something for more then just ints.

class Node<T> {
    Node prev;
    Node next;
    T data;

    public Node(T data) {
        this.data = data;
        prev = null;
        next = null;
    }
}

then same for your linked list implementation.

class LinkedList<T> {
    Node<T> head;

    ...

    public void insert(T d) {
        if (head == null) {
            head = new Node<T>(d);
            return;
        }

    ...

Can make T extends Comparable and use comparable interface to compare if they are less/greater than


Thread-safe retirement account class Are these classes properly guarded for thread safety?  Do you see any issues with any of the classes?

@ThreadSafe
public class RetirementAccount {

     public static final int TYPE1 = 0;
     public static final int TYPE2 = 1;

    @GuardedBy("this")
    private final int accountType;
    @GuardedBy("buySell")
    public final List<BuySell> buySell;

    public RetirementAccount(int accountType) {
       this.accountType = accountType;
       this.buySell = Collections.synchronizedList(new ArrayList<BuySell>());
    }

    public void addToAccount(double amount) {
       if (amount <= 0) {
            throw new IllegalArgumentException();
       } else {
           synchronized (buySell) {
               buySell.add(new BuySell(amount));
            }
         }

      public double interestGained() {

           synchronized (this) {
              switch (accountType) {
                …..
        }
      }

BuySell:

@Immutable
public class BySell {
    public final double depositAmount;

    public BuySell(double depositAmount) {
        this.depositAmount = depositAmount;

    }

  }

In your example code you have the following synchronization-related statements:

this.buySell = Collections.synchronizedList(new ArrayList<BuySell>());
synchronized (buySell) {
synchronized (this) {

As a consequence, you are synchronizing on three different things in one class.

This is very confusing, and is buggy.

You are onlu showing us part of your class, but, there is no point in making buySell a synchronizedCollection if you are going to also gate access to it using synchronized(buySell).... it is redundant, and introduces 2 levels of synchronization.

Presumably when you calculate interest you also access the buySell class, and, as a consequence you have two levels of synchronization there too.

A deadlock is a situation where two threads each have a lock, and they each attempt to get another lock where that other lock is being held by the other thread. If you have multiple lock points in your class it becomes much easier to code things where you introduce deadlock possibilities. I cannot see any in your code at the moment, but your code is incomplete, and these are bugs that can creep in as the class is maintained.

The safest way to do things is to have just a single object which you use to cate access to your class. It is often tempting to use the actual instance itself as the gate, make every method synchronized, and, if you did, your class would become:

public class RetirementAccount {

    public static final int TYPE1 = 0;
    public static final int TYPE2 = 1;

    private final int accountType;
    public final List<BuySell> buySell;

    public RetirementAccount(int accountType) {
       this.accountType = accountType;
       this.buySell = new ArrayList<BuySell>();
    }

   public synchronized void addToAccount(double amount) {
       if (amount <= 0) {
            throw new IllegalArgumentException();
       } else {
            buySell.add(new BuySell(amount));
       }
   }

   public synchronized double interestGained() {

        switch (accountType) {
                …..
   }

This would be a thread-safe class, and it has just one synchronization lock point (the class instance itself). There is no internal possibility of any deadlock, and it would be much better than what you have.

There is a better solution though....

The problem with the above solution is that you expose the locking system in the instance and make it public. Anyone can do:

synchronized(myaccount) {
    ....
}

and they essentially become a prt of your locking system, and that can result in lock-conditions that are unexpected when you created your class.

To make the class completely thread-safe, and have just a single lock point, and still keep isolated from the out-side, I recommend using a dedicated lock instance. It typically looks like this....

public class RetirementAccount {

    public static final int TYPE1 = 0;
    public static final int TYPE2 = 1;

    private final int accountType;
    public final List<BuySell> buySell;
    private final Object synclock = new Object();

    public RetirementAccount(int accountType) {
       this.accountType = accountType;
       this.buySell = new ArrayList<BuySell>();
    }

   public void addToAccount(double amount) {
       synchronized (synclock) {
            if (amount <= 0) {
                throw new IllegalArgumentException();
           } else {
                buySell.add(new BuySell(amount));
           }
       }
   }

   public double interestGained() {

       synchronized (synclock) {

            switch (accountType) {
                …..
       }
   }

Synchronization and other locking mechanisms are complicated concepts, and they are hard to write, and hard to read. There are rules which make it easier:

discipline - you have to create a system, and you have to follow it diligently
dependence - use prefabricated classes where you can (java.util.concurrent.* is your friend) .... but understand what those classes do. Just because one instance is safe, it does not mean your system is safe. They do not solve all problems.
discipline - you have to create a system, and you have to follow it diigently
use as few locks as possible - the fewer locks you have to mentally track, the better.
discipline - you have to create a system, and you have to follow it diigently
avoid places where you have double-locking - you synchonize against one class, then synchronize against another. These double-locking situations are a precursor to deadlocks (they add risk).
discipline - you have to create a system, and you have to follow it diigently
avoid synchronizing classes with synchronized methods because that leads to conditions where outside influences can affect the internal locking of your instance.
discipline - you have to create a system, and you have to follow it diigently

Did I mention that successful thread-safe implementations require discipline? You have to be diligent about implementing the synchronization correctly at each place it is needed.

Finally, there are ways you can force thread-safe operation with light versions of locking. My general advice to people is "don't try to be clever unless you really are that clever". It takes a special kind of brain to understand all the nuances of Java's memory moela nd the way the threading affects it. If you have it clear in your mind, then sure, play some tricks.... but, even those who have i clear, mess up when they try to be clever.


Laravel function to add a supplier, and associated addresses and contacts I have a function addSupplier() that adds a supplier. Each supplier has multiple addresses and each address has multiple contacts.

public function addSupplier ($request) {
        // ADDING THE SUPPLIER BASIC INFO
        $supplier_fillable_values = array_merge(
            $request -> except('address_contact_inputs'),
            ['created_by' => $this->getAuthUserId()]
        );
        $new_supplier = Supplier::create($supplier_fillable_values);

        // ADDING THE SUPPLIER ADDRESSES AND THE ADDRESS CONTACTS
        foreach ($request -> address_contact_inputs as $address) {
            // ADD ADDRESS
            $new_supplier_address = $new_supplier -> addresses() -> create([
                'address'       => $address['address'],
                'created_by'    => $this -> getAuthUserId()
            ]);

            // ADD ADDRESS CONTACTS
            foreach ($address['contacts'] as $contact)
                $new_supplier_address -> contacts() -> create([
                    'name'          => $contact['name'],
                    'phone_number'  => $contact['phone_number'],
                    'created_by'    => $this -> getAuthUserId()
                ]);
        }

        return $request -> address_contact_inputs;
    }

Everything works fine but question is: Is this the best practice or I should use events and create an event for:

Storing the supplier basic info
Storing the supplier addresses
Storing contact for each supplier's address

You could use events for triggering these kinds of things, but there is a danger that your code starts to become harder to follow. Martin Fowler gives a good top-level talk on it here.

There are two issues jumping out at me:

Firstly, your method accepts a request object so there is no indication as to what the requirements of that method are. You could change the method to accept specific arguments such as $created_by, $address_details, etc. This would make it easier to re-use the code elsewhere if needed.

I would make the controller then responsible for preparing that set of data (e.g. handling the preparation of the address details array, getting the 'created_by' etc). And make your addSupplier method work with a set of arguments that could be run from anywhere in your app.

public function addSupplier ($primary_contact_details, $contact_addresses, $created_by = NULL) {
    // ADDING THE SUPPLIER BASIC INFO
    $supplier_fillable_values = array_merge(
        $primary_contact_details,
        ['created_by' => $created_by]
    );
    $new_supplier = Supplier::create($supplier_fillable_values);

    // ADDING THE SUPPLIER ADDRESSES AND THE ADDRESS CONTACTS
    foreach ($contact_addresses as $address) {
        // ADD ADDRESS
        $new_supplier_address = $new_supplier -> addresses() -> create([
            'address'       => $address['address'],
            'created_by'    => $this -> getAuthUserId()
        ]);

        // ADD ADDRESS CONTACTS
        foreach ($address['contacts'] as $contact)
            $new_supplier_address -> contacts() -> create([
                'name'          => $contact['name'],
                'phone_number'  => $contact['phone_number'],
                'created_by'    => $this -> getAuthUserId()
            ]);
    }

    return $request -> address_contact_inputs;
}

Secondly, it's good practise to avoid too many levels of indention, so you could  refactor to use one level of indentation and splitting out the foreach loops to their own public/private methods on the same class.


Mark a radio element as selected on page load and when clicked I have the following javascript that calls a function on page load and on click of an element on my page. Is this the cleanest way to do this?

StyleSelectedPlan();

$(document).on('click', '.select-plan', function () {
    StyleSelectedPlan();
});

function StyleSelectedPlan() {
    $('.radio-wrapper').removeClass('selected');
    $('.select-plan:checked').closest('.radio-wrapper').addClass('selected');
}

As @elclanrs says, function () { StyleSelectedPlan(); } is redundant — you can just write StyleSelectedPath.

Since you're using jQuery, you should use $(document).ready(handler), or better yet, $(handler), to call the handler function when the page loads.

$(StyleSelectedPlan).on('click', '.select-plan', StyleSelectedPlan);

function StyleSelectedPlan() {
    $('.radio-wrapper').removeClass('selected');
    $('.select-plan:checked').closest('.radio-wrapper').addClass('selected');
}
.radio-wrapper.selected { color: green; }

<input name="name" type="radio" class="select-plan" id="one">one

<input name="name" type="radio" class="select-plan" id="two">two

<input name="name" type="radio" class="select-plan" id="three" checked>three (preselected)


Wrapping certain elements in an array with an array with PHP I have a function that's doing this correctly, but I feel as though this can be done (possibly) with minimal code, with recursion, but I'm just wracking my brain. Basically I have JSON that gets decoded to an array. What I want to do is turn this:

{
  "parentView": {
    "childViews": [
      {
        "type": "container-fluid",
        "childViews": [
          {
            "type": "row",
            "childViews": [
              {
                "type": "slider"
              },
              {
                "type": "slider"
              }
            ]
          }
        ]
      }
    ]
  }
}

Into this:

{
  "parentView": {
    "childViews": [
      {
        "type": "controller",
        "childViews": [
          {
            "type": "container-fluid",
            "childViews": [
              {
                "type": "controller",
                "childViews": [
                  {
                    "type": "row",
                    "childViews": [
                      {
                        "type": "controller",
                        "childViews": [
                          {
                            "type": "slider"
                          }
                        ]
                      },
                      {
                        "type": "controller",
                        "childViews": [
                          {
                            "type": "slider"
                          }
                        ]
                      }
                    ]
                  }
                ]
              }
            ]
          }
        ]
      }
    ]
  }
}

Here is the code I am using to make it work how I need. I am just looking for a cleaner method:

function createControls($parentView)
{
    $newArr = $parentView;

    foreach ($parentView as $k => $v) {
        $newArr[$k] = [
            'type' => 'control',
            'childViews' => $v
        ];

        if ($v['childViews']) {
            foreach ($v['childViews'] as $kk => $vv) {
                $newArr[$k]['childViews']['childViews'] = [
                    'type' => 'control',
                    'childViews' => $vv
                ];

                if ($vv['childViews']) {
                    foreach ($vv['childViews'] as $_k => $_v) {
                        $newArr[$k]['childViews']['childViews']['childViews']['childViews'][$_k] = [
                            'type' => 'control',
                            'childViews' => $_v
                        ];
                    }
                }
            }
        }
    }

    return $newArr;
}

$newArr = createControls($arr['parentView']['childViews']);
$arr['parentView']['childViews'] = $newArr;

You can try something like this, note that it is untested:

<?php
function createControls($parentView) {
    $newArr = $parentView;
    foreach ($parentView as $k => $v) {
        $newArr[$k] = ['type' => 'control'];
        if (isset($v['childViews'])) {
            $v = createControls($v['childViews']);
        }
        $newArr[$k]['childViews'] = $v;
    }
    return $newArr;
}
?>

This function uses reccursion to do the same as your code above. It takes one array of Key Value pairs and reassigns the values to wrap the old values into an array with the following format:

array ( 'type' => 'control', 'childViews' => $oldValue );

Basically, instead of nesting a new loop each time the depth of your input array changes like in your function above this function will just pass the old childValues back into itself and assign the result as the new childValues array.


Find number of combinations of strings This was an interview question I was asked. My answer is below, but I wonder if there is a more efficient way. Possible a combinatorics type function, or is an exhaustive generation + count the only way?

Rules: Given a character, the next character can be any one of How many N character combinations can you make? For example

a -> i,e
e -> i,o
i -> a,e,o
u -> a,e,i,o
o -> i,u,o

For N = 1 possible string combinations are a , e, i ,o ,u
for N = 2 possible string combinations are [ai, ae], [ei ,eo] ,[ia , ie , io],[ua,ue,ui,uo],[oi,ou,oo]
and so on.
I solved it by constructing a tree and then counting the leaves

struct Node {
    Node(char v) : v_(v) {}
    void                     add_child(shared_ptr<Node> n) { children.emplace_back(n); }
    char                     v_ = 0;
    vector<shared_ptr<Node>> children;
};

void add_child(shared_ptr<Node> root, int n, int depth, const map<char, vector<char>> &combo) {
    if (depth == n) return;
    auto ch = root->v_;
    auto it = combo.find(ch);
    for (auto ch : it->second) {
        //printf("Adding to %c to %c\n", ch, root->v_);
        root->add_child(make_shared<Node>(ch));
    }
    ++depth;
    for (auto &child : root->children) { add_child(child, n, depth, combo); }
}

int count_leaf(shared_ptr<Node> root) {
    if (root->children.size() == 0) return 0;
    int sum = 0;
    for (auto &child : root->children) sum += count_leaf(child);
    return sum > 0 ? sum : root->children.size();
}
void password_combination() {
    // Find combos until this depth
    int n = 3;

    map<char, vector<char>> combo;
    combo.insert(make_pair('a', vector<char>{'i', 'e'}));
    combo.insert(make_pair('e', vector<char>{'i', 'o'}));
    combo.insert(make_pair('i', vector<char>{'a', 'e', 'o'}));
    combo.insert(make_pair('u', vector<char>{'a', 'e', 'i', 'o'}));
    combo.insert(make_pair('o', vector<char>{'i', 'u', 'o'}));

    // Create a N depth tree and count leaves
    // Count all children until n = 3;
    auto root  = make_shared<Node>(0);
    int  depth = 0;
    for (auto &c : combo) { root->add_child(make_shared<Node>(c.first)); }
    for (auto child : root->children) { add_child(child, n, depth, combo); }
    printf("%d\n", count_leaf(root));
}

Is there a better way. Critique on the code/bugs, and if there's a c++14 idiomatic way of doing it, even better.

TL;DR: dynamic programming.

If the alphabet has \$k\$ symbols (5 in the example), in the worst case there are \$k^N\$ possible strings and generating them is \$\Omega(k^N)\$. But it's possible to count them in \$O(k^2 N)\$ time and \$O(k^2 + N)\$ space using a simple dynamic programming approach.

The input gives the successor function: \$a\$ must be followed by one of \$\{i,e\}\$. The first step is to reverse that to get the predecessor function: \$a\$ must be preceded by one of \$\{i,u\}\$.

Then the number of strings of length \$l\$ which end in \$a\$ is \$1\$ if \$l=1\$, or the sum of the number of strings of length \$l-1\$ which end in \$i\$ or \$u\$ otherwise.

Actually, if \$N\$ is large enough relative to \$k\$ you can do even better with a more complicated approach. Treat it as a Markov chain and find a matrix power of the \$0-1\$ matrix which indicates the successor function. That takes \$O(\lg N)\$ matrix multiplications of a \$k\times k\$ matrix.


Reading CSV files pertaining to financial recorts I have a number of text files (they are reports from a financial system) stored in a directory. Each report has a particular piece of text to search for (eg Report Total) and then a range of characters to extract for the value (e.g. position 78 for 10 characters).
There are a number of these reports - but no more than 200 so not a huge amount of processing or work required.

The premise of my solution is to maintain a simple CSV file with the report name, text to search for, start value for extract, length of characters to extract. Then, it is a simple case of updating this text file to add or remove the reports as necessary.

The script I have developed is below, and it works, but I would be interested in some critique as to how it may be made "better".

What needs changing/improving and why?

#!/usr/bin/perl -w
use strict;
use strict;
use Data::Dumper;
use Cwd;
use Text::CSV;

my $SrcDir="/data03/adudata/live/work/";
my $RepDir="/data03/adudata/live/archive/";
my $ReportList='pl_rec.list';
my $csv = Text::CSV->new();
my @FileToRead;
my @TextToFind;
my @RowsOffset;
my @DataStart;
my @DataEnd;
my %ReportData;

chdir $SrcDir;
open my $fh, '<', $ReportList or die "Could not open file";
while (my $row = $csv->getline( $fh )) {
  push (@FileToRead, $row->[0]);
  push (@TextToFind, $row->[1]);
  push (@RowsOffset, $row->[2]);
  push (@DataStart, $row->[3]);
  push (@DataEnd, $row->[4]);
}

close ($fh);

for my $i(0 .. $#FileToRead) {
  open $fh, "<", $RepDir.$FileToRead[$i]. ".prn" or
    die "Could not open $FileToRead[$i]";

  while (<$fh>) {

    if (grep /$TextToFind[$i]/, $_) {
      my $extractValue = substr($_,$DataStart[$i],$DataEnd[$i]);
      $extractValue =~s/(^\s+)|(s+$)//g;
      $extractValue =~ s/,//g;
      $ReportData{$TextToFind[$i]} = $extractValue+0;
    }
  }
close $fh;
}

while(my($code,$value)=each(%ReportData)){
  print "$code,$value\n";
}

The point of filehandles is that it allows you to open more than one file simultaneously. So you could be reading the file list and the files you're searching through simultaneously.

Also, this:

if (grep /$TextToFind[$i]/, $_)

Is a a really convoluted way to write:

if (/$TextToFind[$i]/)

Overall, I'd do something like this:

#!/usr/bin/env perl

use strict;
use warnings;
use Text::CSV;

my $SrcDir = "/data03/adudata/live/work/";
my $RepDir = "/data03/adudata/live/archive/";
my $ReportList = 'pl_rec.list';

open my $list_fh, '<', $SrcDir.$ReportList
  or die "Could not open $ReportList: $!";

my %ReportData;
my $csv = Text::CSV->new();

while (my $row = $csv->getline( $list_fh ))
{
  my ($filename, $text, $rows_offset, $data_start, $data_end) = @$row;

  open my $fh, '<', $RepDir.$filename
    or die "Could not open $filename: $!";

  while (<$fh>)
  {
    next unless /\Q$text_to_find/;

    my $extractValue = substr($_, $data_start, $data_end);
    $extractValue =~ s/(^\s+)|(s+$)//g;
    $extractValue =~ s/,//g;

    $ReportData{$text} = $extractValue + 0;
  }
}

for (sort keys %ReportData)
{
  my $value = $ReportData{$_};
  print "$_,$value\n";
}


How to set an interval to std::map This code belongs to a contest. I tried to apply the range of data to a std::map. The contest system however warns me that the implementation of the assign function is not valid. I don't know which part of the code is written incorrectly, hence the system doesn't reveal any details except that the implementation is erroneous.

Edit: I also added the test class that I used for testing the interval_map class. I tried to test every possible aspect of what could possibly go wrong. The test runs without any problem.

        void assign(unsigned char first, unsigned char last, unsigned char value)
        {
            test();

            interval_map::assign(first, last, value);
                for (int i = first; i < last; ++i)
                    vector_cont[i] = value;

            test();
        }
```

Quite frankly it seems that you did not do what was asked from you.

The text above assign gives you the requirements

    // Assign value val to interval [keyBegin, keyEnd).
    // Overwrite previous values in this interval.
    // Conforming to the C++ Standard Library conventions, the interval
    // includes keyBegin, but excludes keyEnd.
    // If !( keyBegin < keyEnd ), this designates an empty interval,
    // and assign must do nothing.

So you have to find all elements in the map that are in the range [keyBegin, keyEnd) and assign val to them. Then you should insert keyBegin and keyEnd with the value val into the map.

Actually you are quite close to what was asked with the calls to lower_bound

    void assign(K const& keyBegin, K const& keyEnd, V const& val) {
        // keyBegin < keyEnd indicates an empty interval, so the function simply returns
        if (!(keyBegin < keyEnd))
            return;

        ....
    }

This is already a problem as the comment is incorrect. It should read keyEnd < keyBegin

        auto begin_lower_iter = m_map.lower_bound(keyBegin);
        auto end_upper_iter = m_map.upper_bound(keyEnd);

The first call is obviously correct. However, the second call is the wrong one. upper_bound returns the first element that is greater than the key. Now consider if keyEnd is in the map. As the interval is half opened you would have to decrement twice in that case and once if it is not in the map.

Rather than that you can simply use lower_bound here too

        auto begin_iter = m_map.lower_bound(keyBegin);
        auto end_iter = m_map.lower_bound(keyEnd);

Now we have to consider the bounds. First you should check, whether keyBegin is already in the map. Otherwise you should insert it so that the left boundary is inclusive. Note that in that case we can use begin_iter as a hint so that the complexity is an amortized constant

        if (begin_iter->first != keyBegin) {
            begin_iter = m_map.insert(begin_iter, val);
            // The element at begin_iter already has the correct value so increment
            ++begin_iter;
        }

It is slightly different for the upper boundary, as that is exclusive. As far as i understand it, that means that you should insert an element with the value the interval had previously so that it is the inclusive left border of the new interval.

If end_iter is at keyEnd it already has the correct value and we do not have to do anything. However if it is not in the map, then we have to insert the value of the previous element at position keyEnd

        if (end_iter->first != keyEnd) {
            V const& oldVal = std::prev(end_iter, 1)->second;
            end_iter = m_map.insert(end_iter, oldVal);
        }

Now to a completely different topic. As you might have seen insert returns an updated iterator. While there is no iterator invalidation in std::map,i would not risk it. So the insertion should happen right after you get the iterator.

        auto begin_iter = m_map.lower_bound(keyBegin);
        if (begin_iter->first != keyBegin) {
            begin_iter = m_map.insert(begin_iter, val);
            // The element at begin_iter already has the correct value so
            ++begin_iter;
        }

        auto end_iter = m_map.lower_bound(keyEnd);
        if (end_iter->first != keyEnd) {
            V const& oldVal = std::prev(end_iter, 1)->second;
            end_iter = m_map.insert(end_iter, oldVal);
        }

Also this is still not correct. Lets assume begin_iter was not less that the first element in the map smaller than keyEnd. In that case we would have overwritten the original value of the -larger- interval. Therefore, the insertion of the upper bound needs to happen first.

Now that we handeled the borders, we should update the elements int the range

        while(begin_iter != end_iter) {
            begin_iter->second = val;
            ++begin_iter;
        }

Finally, there is still an elephant in the room. While the whole file seems to be not fully consistent, the arguments to the function are camelCase. Yet you named your variables using snake_case. For me that is already a minus as you do not adopt to the existing codebase and inconsistent naming is a constant source of confusion and a huge drain on mental capacity.

So lets put it together

    void assign(K const& keyBegin, K const& keyEnd, V const& val) {
        // keyBegin < keyEnd indicates an empty interval, so the function simply returns
        if (!(keyBegin < keyEnd)) {
            return;
        }

        // Search for the upper bound and if necessary include a border element
        // with the value of the old interval
        auto endIter = m_map.lower_bound(keyEnd);
        if (endIter->first != keyEnd) {
            // As the intervals are half open we need to take the value of the left element and insert it at keyEnd
            V const& oldVal = std::prev(endIter, 1)->second;
            endIter = m_map.insert(endIter, oldVal);
        }

        // Search for the lower bound and if necessary include the element
        auto beginIter = m_map.lower_bound(keyBegin);
        if (beginIter->first != keyBegin) {
            beginIter = m_map.insert(beginIter, val);
            // The element at begin_iter already has the correct value so
            ++beginIter;
        }

        // Update the values within the range [beginIter, endIter)
        while(beginIter != endIter) {
            beginIter->second = val;
            ++beginIter;
        }
    }


Using Ramda in point-free way to transform data into new format Recently I asked on SO about using point free methods to rearrange some data

The idea was to turn data in this format:

const data = [
  {
    timeline_map: {
      "2017-05-06": 770,
      "2017-05-07": 760,
      "2017-05-08": 1250,
    }
  }, {
    timeline_map: {
      "2017-05-06": 590,
      "2017-05-07": 210,
      "2017-05-08": 300,
    }
  }, {
    timeline_map: {
      "2017-05-06": 890,
      "2017-05-07": 2200,
      "2017-05-08": 1032,
    }
  }
]

Into this:

const hope = [
  ["2017-05-06", 770, 590, 890],
  ["2017-05-07", 760, 210, 2200],
  ["2017-05-08", 1250, 300, 1032],
]

The answers I got seemed kind of verbose in my opinion however (though I've no idea if they are better or worse than mine I haven't checked performance yet) so I spent more time studying Ramda and have come up with a solution of my own I like a little better. However I've only been doing this a little over a week so I'm sure it can be improved.

My code:

const datesValuesReducer = (accum, curr) => {
  if (accum.hasOwnProperty(curr[0])) {
    accum[curr[0]] = accum[curr[0]].concat(curr[1])
  } else {
    accum[curr[0]] = [curr[0], curr[1]]
  }
  return accum
}

const res = R.pipe(
  R.pluck('timeline_map'),
  R.map(R.toPairs),
  R.flatten,
  R.splitEvery(2),
  R.reduce(datesValuesReducer, {}),
  R.values
)

console.log(res(data))

Three points of possible concern:

1) Omitting R.flatten and R.splitEvery(2) get pretty close to what Is being output directly from R.map(R.toPairs) so maybe there is a way to deal with that data more directly and those methods can be omitted.

2) datesValuesReducer is pretty complex, maybe it could be simplified. I'm not sure if having the accum and curr means this solution isn't entirely "point free". Thoughts?

3) Also, I use pipe in favor over compose; I just find it reads more naturally. Maybe someone has some opinions about that.

JSBIN

I like your pipeline approach in general!

  1) Omitting R.flatten and R.splitEvery(2) get pretty close to what Is being output directly from R.map(R.toPairs) so maybe there is a way to deal with that data more directly and those methods can be omitted.

Yup, you want something like R.chain here.

R.chain(R.toPairs)

is equivalent here to

R.pipe(
  R.map(R.toPairs),
  R.flatten,
  R.splitEvery(2)
)

effectively performing a non-recursive flatten on the output of the chained function. Clojure calls this mapcat.

  2) datesValuesReducer is pretty complex, maybe it could be simplified. I'm not sure if having the accum and curr means this solution isn't entirely "point free". Thoughts?

Yeah, named arguments are essentially the "points" you're trying to avoid in the pointfree style.

I see two directions here. One is to shorten the function but add more points via destructuring:

R.reduce((acc, [k, v]) => R.assoc(k, (acc[k] || [k]).concat(v), acc),
         {})

The other is to fall further into the Ramda world and do something like this:

const f = R.pipe(
  R.pluck('timeline_map'),
  R.chain(R.toPairs),
  R.reduceBy(R.flip(R.useWith(R.append, [R.nth(1)])), [], R.head),
  R.toPairs,
  R.map(R.unnest)
)

(The last two functions could probably be turned into a reduce as well if you're determined.)

This is fully pointfree, but seems kinda deliberately obtuse, even if the reduceBy reducer were to be pulled out into a named function. If you want an opinion I'd say stick the entire thing in a function that takes in the base keyword (eg. 'timeline_map'), and then use whatever you find most readable inside that function, without striving for total pointfree purity.

If you want more opinions, arguably there's a sweet spot of, say, 60-70% pointfree where you gain the most from the style. Beyond that, things can become increasingly contorted, and you end up with, well, R.reduceBy(...R.useWith...


Updating dimensions when focus is lost from any of 9 columns How can I write this more pretty?

private void _detailSpread_OnCellFocusChange(object sender, CellFocusChangeEventArgs e)
{
    if (e.SourceCol == _detailSpread.GetColNumber("article"))
        UpdateDimensionData(_detailSpread.GetColID(e.SourceCol), _detailSpread.Rows[e.SourceRow]);
    else if (e.SourceCol == _detailSpread.GetColNumber("account"))
        UpdateDimensionData(_detailSpread.GetColID(e.SourceCol), _detailSpread.Rows[e.SourceRow]);
    else if (e.SourceCol == _detailSpread.GetColNumber("dim_1"))
        UpdateDimensionData(_detailSpread.GetColID(e.SourceCol), _detailSpread.Rows[e.SourceRow]);
    else if (e.SourceCol == _detailSpread.GetColNumber("dim_2"))
        UpdateDimensionData(_detailSpread.GetColID(e.SourceCol), _detailSpread.Rows[e.SourceRow]);
    else if (e.SourceCol == _detailSpread.GetColNumber("dim_3"))
        UpdateDimensionData(_detailSpread.GetColID(e.SourceCol), _detailSpread.Rows[e.SourceRow]);
    else if (e.SourceCol == _detailSpread.GetColNumber("dim_4"))
        UpdateDimensionData(_detailSpread.GetColID(e.SourceCol), _detailSpread.Rows[e.SourceRow]);
    else if (e.SourceCol == _detailSpread.GetColNumber("dim_5"))
        UpdateDimensionData(_detailSpread.GetColID(e.SourceCol), _detailSpread.Rows[e.SourceRow]);
    else if (e.SourceCol == _detailSpread.GetColNumber("dim_6"))
        UpdateDimensionData(_detailSpread.GetColID(e.SourceCol), _detailSpread.Rows[e.SourceRow]);
    else if (e.SourceCol == _detailSpread.GetColNumber("dim_7"))
        UpdateDimensionData(_detailSpread.GetColID(e.SourceCol), _detailSpread.Rows[e.SourceRow]);

    _detailSpread.Validate();
}

I don't do C# so pardon the syntax, but you should be able to reduce the amount of code by using a for loop over a string array with your column names.

Again I don't do C# but in Java you could do something like this:

for(String col : new String[]{"article", "account", "dim_1", ...}) {
    if (e.SourceCol == _detailSpread.GetColNumber(col)){
        UpdateDimensionData(_detailSpread.GetColID(e.SourceCol),
                            _detailSpread.Rows[e.SourceRow]);
        break;
    }
}

I would consider extracting the string array to a constant.

Or if your table is static you could just use a HashSet (or whatever it's called in C#):

// Somewhere in a constructor or static initializer
HashSet<Integer> focusChangeColNumbers = new ...;
colNumbers.put(_detailSpread.GetColNumber("article");
colNumbers.put(_detailSpread.GetColNumber("account");
...

// Then do:

private void _detailSpread_OnCellFocusChange(object sender, CellFocusChangeEventArgs e)
{
    if(focusChangeColNumbers.contains(e.SourceCol)){
        UpdateDimensionData(_detailSpread.GetColID(e.SourceCol),
                            _detailSpread.Rows[e.SourceRow]);
    _detailSpread.Validate();
}

Note that this is going to be \$O(1)\$ in the number of columns where the other options are \$O(n)\$ in the number of columns.

Another option (if the API permits, I have no idea) is to simply turn it around:

String[] focusColNumbers = new String[]{"article", "account", "dim_1", ...};
if(focusColNumbers.contains(_detailSpread.GetColName(e.SourceCol))){
    ...
}


Object - Text Validator EDIT:

Function is returnted to JClass Text.  Prototypes hold non instance members (statics).  No privacy.  Privacy requires self executing methods.

var Text = function( form_name )
{
    this.text_array = document.forms[form_name].elements;
};

Text.prototype.patterns =
{
    prefix_url: /^http:\/\//,
    url:        /^.{1,2048}$/,
    tweet:      /^.{1,40}$/,
    title:      /^.{1,32}$/,
    name:       /^.{1,64}$/,
    email:      /^.{1,64}@.{1,255}$/,
    pass:       /^.{6,20}$/
};

Text.prototype.pattern = function( type )
{
    return this.patterns[type].exec( this.text_array[type].value );
};

Text.prototype.patternAdd = function( type )
{
    return this.patterns[type].exec( this.text_array.url.value );
};

Text.prototype.same = function()
{
    return ( (this.text_array.email.value) === (this.text_array.email1.value) );
};

Text.prototype.emptyUser = function()
{
    var element;
    for ( element in this.text_array )
    {
        if( this.text_array[element].value === '' )
        {
            return 0;
        }
    }
    return 1;
};

/**
 *      Text End
 */

Few additions

1) First get some unit tests in

2) I would do something like this http://jsfiddle.net/Brw4W/1/


Trying to create login I'm new to PHP and web development in general. Is this a good coding style for me as a beginner?  I don't care about password hashing.  I use MD5 which is, as far as I know, not a good encryption method, and I'm trying to create just the login for now.

connection.php:

<?php

try {
    $pdo = new PDO("mysql:host=localhost;dbname=akar","akar","raparen");
} catch (PDOException $e) {
    echo $e->getMessage();
}
?>

index.php:

<?php
session_start();
include "connection.php";

if(isset($_SESSION['logged_in']) == true){
    //Redirect the user to members
    header("Location: member.php");
    exit();

} else {
    ?>

    <?php
}
    if(isset($_POST['username']) && isset($_POST['password'])) {
        $username = $_POST['username'];
        $password = md5($_POST['password']);

        if(empty($username) or empty($password)) {
            //Display Errors
            $error = "All fields are required!";

        } else {
            //Verify the user
            $sql = "SELECT * FROM `users` WHERE user_name = ? AND user_password = ?";
            $query = $pdo->prepare($sql);
            $query->bindValue(1,$username);
            $query->bindValue(2,$password);
            $query->execute();

            if ($query->rowCount() > 0) {
                //User details right
                $_SESSION['logged_in'] = true;
                header("Location: member.php");
            } else {
                //User details wrong
                $error = "Wrong username or password";
            }
        }
    }

?>

<?php if (isset($error))  { ?>

    <?php echo $error; ?>

<?php } ?>

member.php:

<?php
session_start();

if($_SESSION['logged_in']) {
    echo "You're logged in";
} else {
    header("Location: index.php");
}

?>

Logout

logout.php

<?php
    session_start();

    session_destroy();
    header("Location: index.php");
?>

Here is my form:

 <form action="index.php" method="POST" autocomplete="off">
    <input type="text" name="username" placeholder="Username" />
    <input type="password" name="password" placeholder="Password" />
    <input type="submit" name="submit" value="Login" />
 </form>

Here are few things I observed:

You repeated session_start() twice in your member.php - you can only do that once in order to start the session
You are not sanitizing your input for bad values - its best practice to do so to avoid SQL injection - however as a beginner its a good idea that you started with PDO and not legacy mysql_query
After a redirect you should exit(); because your script will continue to process until end of the file. Why let a script continue when you're done.
I know you are trying to section your code but if PHP code continues don't close it un-necessarily. (referring to index.php and to the if error part).
You should use require or require_once. include causes an error if include is invoked (by mistake) more than one time.
Last observation would be in your member.php it should be a blacklist if and not if/else. You should basically tell the user that you shouldn't be here if you are not logged in..if not just proceed to the rest of the script (ask question if you didn't understand this part)


Creating an array of random numbers with no duplicates I only need three random numbers within the range of 1-54 (deck of cards) and none of the three numbers can be a duplicate. I cannot use an ArrayList; I must use a standard array. Is the method I chose to check for duplicates good enough?

public class Test {

public static int genRandom() {
    // generate random int value between 1 and 54
    int random = 1 + (int) (Math.random() * ((54 - 1) + 1));
    return random;
}

public static boolean containsDuplicates(int[] arr) {
    // check to see if positions have matching values
    if (arr[0] == arr[1] || arr[0] == arr[2] || arr[1] == arr[2]) {
        System.out.println("Duplicates Exist");
        // if matching value is found, randomize the array again
        shuffle(arr);
        return true;
    }
    System.out.println("No duplicates");
    return false;
}

public static void shuffle(int[] arr) {
    for (int i = 0; i < arr.length; i++) {
        arr[i] = genRandom();
        System.out.println(arr[i]);
    }
    containsDuplicates(arr);
}

public static void main(String[] args) {

    int[] cards = new int[3];
    shuffle(cards);
}
}

No, not a good way at all.

shuffle and containsDuplicate recursively call each other without guarantee to ever stop doing that. That is the program has a certain probability to die because of stack overflow.

A similar non-recursive version

    public static boolean containsDuplicates(int[] arr) {
        return arr[0] == arr[1] || arr[0] == arr[2] || arr[1] == arr[2];
    }

    public static void shuffle(int[] arr) {
        for (int i = 0; i < arr.length; i++) {
            arr[i] = genRandom();
        }
    }

    public static void main(String[] args) {
        do {
            shuffle();
        } while(containsDuplicates());
    }

doesn't suffer stack overflow, but still may run for indefinitely long time.

As Jerry Coffin mentioned in the comment, a Fisher-Yates algorithm is a way to go.


Making the Levenshtein distance code cleaner I was writing an implementation of Levenshtein distance in Python and I found my code really ugly.
Have you any idea how to make it more elegant?

import unittest
import enum

def seq_dist(first, second):
    Direction = enum.Enum('Direction', 'gap_first gap_second match finish')

    def find_dist():
        dist_table = [(len(second) + 1) * [0] for i in range(len(first) + 1)]
        directions = [(len(second) + 1) * [None] for i in range(len(first) + 1)]
        for i in range(len(first) + 1):
            for j in range(len(second) + 1):
                if (i, j) == (0, 0):
                    dist_table[i][j] = 0
                    directions[i][j] = Direction.finish
                elif i == 0:
                    dist_table[i][j] = j
                    directions[i][j] = Direction.gap_first
                elif j == 0:
                    dist_table[i][j] = i
                    directions[i][j] = Direction.gap_second
                else:
                    gap_first = dist_table[i - 1][j] + 1
                    gap_second = dist_table[i][j - 1] + 1
                    match = dist_table[i - 1][j - 1] + (first[i-1] != second[j-1])
                    if gap_first <= min(gap_first, match):
                        dist_table[i][j] = gap_first
                        directions[i][j] = Direction.gap_second
                    elif gap_second <= min(gap_second, match):
                        dist_table[i][j] = gap_second
                        directions[i][j] = Direction.gap_first
                    else:
                        dist_table[i][j] = match
                        directions[i][j] = Direction.match
        return dist_table, directions

    def backtrack(directions):
        i, j = (len(first), len(second))
        seq = []
        while directions[i][j] != Direction.finish:
            direction = directions[i][j]
            if direction == Direction.gap_first:
                seq.append((None, second[j-1]))
                j -= 1
            elif direction == Direction.gap_second:
                seq.append((first[i-1], None))
                i -= 1
            elif direction == Direction.match:
                seq.append((first[i-1], second[j-1]))
                i -= 1
                j -= 1
        seq.reverse()
        return seq

    dist_table, directions = find_dist()

    dist = dist_table[-1][-1]
    diff = backtrack(directions)

    return (dist, diff)

class MatchSeqsTest(unittest.TestCase):
    def test_seq_dist(self):
        self.assertMatchDist(0, '', '')
        self.assertMatchDist(2, 'something cool', '')
        self.assertMatchDist(0, 'first', 'first')
        self.assertMatchDist(1, 'first', 'first second')
        self.assertMatchDist(1, 'fourth second', 'first second')
        self.assertMatchDist(1, 'first second third', 'second third')

    def assertMatchDist(self, dist, first, second):
        self.assertMatchDistOneWay(dist, first, second)
        self.assertMatchDistOneWay(dist, second, first)

    def assertMatchDistOneWay(self, dist, first, second):
        actualDist, actualDiff = seq_dist(first.split(), second.split())
        self.assertEqual(dist, actualDist)
        self.assertEqual(dist, self.alignment_dist(actualDiff))
        self.assertEqual(self.extract_first(actualDiff), first.split())

    def extract_first(self, diff):
        return [f for f, s in diff if f is not None]

    def alignment_dist(self, diff):
        return sum(1 for f, s in diff if f != s)

if __name__ == '__main__':
    unittest.main()

It would seem that you try to do more than is sufficient for computing the Levenshtein edit distance. You could try this:

def levenshtein_distance(string1, string2):
    n = len(string1)
    m = len(string2)
    d = [[0 for x in range(n + 1)] for y in range(m + 1)]

    for i in range(1, m + 1):
        d[i][0] = i

    for j in range(1, n + 1):
        d[0][j] = j

    for j in range(1, n + 1):
        for i in range(1, m + 1):
            if string1[j - 1] is string2[i - 1]:
                delta = 0
            else:
                delta = 1

            d[i][j] = min(d[i - 1][j] + 1,
                          d[i][j - 1] + 1,
                          d[i - 1][j - 1] + delta)

    return d[m][n]

def main():
    print(levenshtein_distance("code", "rodde"))

if __name__ == "__main__":
    main()

Hope that helps.

Edit

The following extension will also give you the edit sequence, and the alignment of the two input words:

def levenshtein_distance(s, z):
    s = "\u0000" + s
    z = "\u0000" + z

    n = len(s)
    m = len(z)
    d = [[0 for x in range(n + 1)] for y in range(m + 1)]
    parent_map = dict()
    parent_map[(0, 0)] = None

    for i in range(1, m + 1):
        d[i][0] = i

    for j in range(1, n + 1):
        d[0][j] = j

    for j in range(1, n + 1):
        for i in range(1, m + 1):
            if s[j - 1] is z[i - 1]:
                delta = 0
            else:
                delta = 1

            tentative_dist = d[i - 1][j] + 1
            operation = 1 # Insert

            if tentative_dist > d[i][j - 1] + 1:
                tentative_dist = d[i][j - 1] + 1
                operation = 2 # Delete

            if tentative_dist > d[i - 1][j - 1] + delta:
                tentative_dist = d[i - 1][j - 1] + delta
                operation = 3

            if operation is 1:   # Insert
                parent_map[(i, j)] = (i - 1, j)
            elif operation is 2: # Delete
                parent_map[(i, j)] = (i, j - 1)
            else:                # Substitute
                parent_map[(i, j)] = (i - 1, j - 1)

            d[i][j] = tentative_dist

    edit_string_top = ""
    edit_string_bottom = ""
    edit_sequence = ""

    current = (m, n)

    while True:
        predecessor = parent_map[current]

        if not predecessor:
            break

        if current[0] != predecessor[0] and current[1] != predecessor[1]:
            schar = s[predecessor[1]]
            zchar = z[predecessor[0]]

            edit_string_top += schar
            edit_string_bottom += zchar

            if schar is zchar:
                edit_sequence += "N"
            else:
                edit_sequence += "S"
        elif current[0] != predecessor[0]:
            edit_string_top += "-"
            edit_string_bottom += z[predecessor[0]]
            edit_sequence += "I"
        else:
            edit_string_top += s[predecessor[1]]
            edit_string_bottom += "-"
            edit_sequence += "D"

        current = predecessor

    edit_string_top = edit_string_top[:-1][::-1]
    edit_string_bottom = edit_string_bottom[:-1][::-1]
    edit_sequence = edit_sequence[:-1][::-1]

    return d[m][n], edit_sequence, edit_string_top, edit_string_bottom

def main():
    dist, edit_sequence, edit_string_top, edit_string_bottom = levenshtein_distance("handball", "aballad")
    print("Distance: ", dist)
    print("Edit sequence: ", edit_sequence)
    print(edit_string_top)
    print(edit_string_bottom)

if __name__ == "__main__":
    main()