javascript property extractor optimisation This is a very simple nested property extractor. I wonder if it can be optimised further?

/**
 * Extract nested property from object. If nested property cannot be reached, return value of rescue
 * @param obj Object
 * @param path Can be dot-separated  string or array
 * @param rescue (optional) default value. Defaults to null
 */
function extract(obj, path, rescue){

    if (typeof obj === "object" && path){

        var elements = typeof path === "string" ? path.split(".") : path;

        if (typeof elements.shift === "function"){

             var head = elements.shift();

             if (obj.hasOwnProperty(head)){

                return (elements.length === 0) ? obj[head] : extract(obj[head], elements, rescue);

             } // if

         } // if

    } // if

    return rescue || null;

} // extract

var noob = {k1 : {k11 : {k111 : "v1"}}, k2 : { k21 : "v2"}};

console.log(extract(noob, 'k1.k11')); // {k111 : "v1"}
console.log(extract(noob, 'k1.k11.k111')); // v1
console.log(extract(noob, ['k1', 'k11', 'k111'])); // v1
console.log(extract(noob, 'k1.k11.kx')); // null
console.log(extract(noob, 'k2')); //  k2 : { k21 : "v2"}
console.log(extract(noob, 'k2.k21.k22')); // null
console.log(extract(noob, 'k1.k11.k22', "ZUT")); // ZUT
console.log(extract(noob, '', "ZUT"));  // ZUT
console.log(extract(false, '', "ZUT"));  // ZUT

Here are some of my thoughts:

I don't see the point in providing the path as a dot separated string, but it may be useful in your application.
If you keep the dot separated string, then you should consider extracting the recursive function into a separate internal function, so that you don't have to repeat the check of the path argument in each iteration.
Why do you check for the shift method? If it doesn't exist, then your function will fail silently. The regular "method does not exist" exception would be much more useful. Instead implement the method on the Array.prototype yourself separately if it doesn't exist.
Finally I would move the path.length == 0 to the start of the function. That is where the break condition of recursive functions are usually expected.


Advent of code #2 Haskell solution I've tried to solve AoC day 2 challenge in Haskell (here - don't worry, it's not a competition so sharing a solution here is OK).

The goal is to implement a very simple VM with opcodes 1 (add), 2 (mult) and 99 (exit).

I feel like my solution is incredibly verbose. That maybe be because I rely heavily on the state monad (my background is imperative programming, so there's that). Is there anything I could improve without rewriting the whole solution?

Here's my code, thanks for all suggestions:

import Data.Sequence
import Control.Monad.State
import Data.List.Split

data Machine = Machine {
    mState :: Seq Int,
    mPos :: Int,
    isDone :: Bool
}

opReadHead :: State Machine Int
opReadHead = do
    machine <- get
    return $ index (mState machine) (mPos machine)

opReadAt :: Int -> State Machine Int
opReadAt target = do
    machine <- get
    return $ index (mState machine) target

opForward :: State Machine ()
opForward = do
    machine <- get
    put $ machine { mPos = mPos machine + 1 }

opWrite :: Int -> Int -> State Machine ()
opWrite target what = do
    machine <- get
    put $ machine { mState = update target what (mState machine) }

opReadNext :: State Machine Int
opReadNext = do
    a <- opReadHead
    opForward
    return a

opAdd :: State Machine ()
opAdd = do
    aPtr <- opReadNext
    a <- opReadAt aPtr
    bPtr <- opReadNext
    b <- opReadAt bPtr
    target <- opReadNext
    opWrite target (a + b)

opMult :: State Machine ()
opMult = do
    aPtr <- opReadNext
    a <- opReadAt aPtr
    bPtr <- opReadNext
    b <- opReadAt bPtr
    target <- opReadNext
    opWrite target (a * b)

opExit :: State Machine ()
opExit = do
    current <- get
    put $ current { isDone = True }

isMachineDone :: State Machine Bool
isMachineDone = do
    get >>= (return . isDone)

opcode :: Int -> State Machine ()
opcode 1 = opAdd
opcode 2 = opMult
opcode 99 = opExit

opExecuteNext :: State Machine ()
opExecuteNext = do
    opValue <- opReadNext
    opcode opValue

runCode :: State Machine ()
runCode = do
    done <- isMachineDone
    if done
    then return ()
    else opExecuteNext >> runCode

evalWith :: Machine -> Int -> Int -> Int
evalWith machine noun verb = do
    fst $ runState (do
        opWrite 1 noun
        opWrite 2 verb
        runCode
        opReadAt 0
        ) machine

main :: IO()
main = do
    fileData <- readFile "input"
    let memory = map read $ splitOn "," fileData
    let machine = Machine {
        mState = fromList memory,
        mPos = 0,
        isDone = False
    }
    let outputs = [(evalWith machine x y, (x, y)) | x <- [0..99], y <- [0..99]]
    print $ snd $ head $ Prelude.filter ((== 19690720) . fst) outputs
```

Use Control.Lens for code this stateful. (Control.Lens.TH must be used to define Machine.) May as well leave out type signatures this homogenous. Control.Monad.Loops often helps against explicit monadic recursion.

opReadAt target = uses mState $ (`index` target)
opReadNext = mPos <<+= 1 >>= opReadAt
opWrite target what = mState %= update target what

opBin op = do
    a <- opReadNext >>= opReadAt
    b <- opReadNext >>= opReadAt
    target <- opReadNext
    opWrite target $ op a b

opcode 1 = opBin (+)
opcode 2 = opBin (*)
opcode 99 = isDone .= True

runCode = (opReadNext >>= opCode) `untilM_` use isDone

evalWith :: Int -> Int -> Machine -> Int
evalWith noun verb = evalState $ do
    opWrite 1 noun
    opWrite 2 verb
    runCode
    opReadAt 0


Given a pivot x, and a list lst, partition the list into three parts The task:

  Given a pivot x, and a list lst, partition the list into three parts.

  The first part contains all elements in lst that are less than x The
  second part contains all elements in lst that are equal to x The third
  part contains all elements in lst that are larger than x Ordering
  within a part can be arbitrary.

  For example, given x = 10 and lst = [9, 12, 3, 5, 14, 10, 10], one
  partition may be `[9, 3, 5, 10, 10, 12, 14].

My solution:

const createPartitionOf = (lst, pivot) => {
  const lessLst = lst.filter(x => x < pivot);
  const equalLst = lst.filter(x => x === pivot);
  const largerLst = lst.filter(x => x > pivot);
  return [...lessLst, ...equalLst, ...largerLst];
};

console.log(createPartitionOf([9, 12, 3, 5, 14, 10, 10], 10));

My solution2:

const createPartitionOf2 = (lst, pivot) => {
  const lessLst = [];
  const equalLst = [];
  const largerLst = [];
  for (let i = 0; i < lst.length; i++) {
    if (lst[i] < pivot) {
      lessLst.push(lst[i]);
    } else if (lst[i] > pivot) {
      largerLst.push(lst[i]);
    } else {
      equalLst.push(lst[i]);
    }
  }
  return [...lessLst, ...equalLst, ...largerLst];
};

console.log(createPartitionOf2([9, 12, 3, 5, 14, 10, 10], 10));

In your first solution you are traversing the list 3 times, whereas in the second solution only once. Given the functional-tag, I’d say you’re after the reduce function:

const flatten = (acc, x) => acc.concat(x)

const pivotPartition = (pivot, ary) => {
  const toOrdered = (triplet, n) => {
    let [less, equal, greater] = triplet

    switch(true) {
      case n < pivot: less.push(n); break
      case n > pivot: greater.push(n); break
      default: equal.push(n)
    }

    return triplet
  }

  return ary.reduce(toOrdered, [[], [], []]).reduce(flatten)
}

If you want to sort the subarrays as well, then the whole thing is just a ary.sort((a, b) => a - b), although as it is mutating the list, it isn’t a functional solution.


C itoa implementation As an exercise, I limited my self to the man pages. I didn't look for a previous existing implementation because I wanted to see how well I could do on my own. I also tried to handle as many cases as I could think of. I am sure there are weaknesses so feel free to point them out. Where can I improve my usage of the C standard library, etc.

char *
itoa(int i)
{
  short digit_cnt;
  short index;
  void *ret;
  char digit;
  int tmp;

  errno = 0;
  feclearexcept(FE_ALL_EXCEPT);
  if (i == 0) {
    digit_cnt = 1;
  } else {
    tmp = (i == INT_MIN) ? (i + 1) : i;
    digit_cnt = floor(log10(abs(tmp))) + 1;
    if (errno || fetestexcept(FE_INVALID | FE_DIVBYZERO | FE_OVERFLOW |
      FE_UNDERFLOW)) {
      return NULL;
    }
  }

  if (i < 0) {
    ++digit_cnt;
  }
  ++digit_cnt; // '\0'

  errno = 0;
  if ((ret = malloc(digit_cnt * sizeof(char))) == NULL || errno) {
    print_error_msg(errno);
    free(ret);
    return NULL;
  }

  // This made debugging easier.
  memset(ret, '0', digit_cnt * sizeof(char));

  index = digit_cnt;
  ((char*)ret)[--index] = '\0';

  tmp = i;
  do {
    digit = (char)((int)'0' + abs(tmp % 10));
    ((char*)ret)[--index] = digit;
    tmp /= 10;
  } while (tmp != 0);

  if (i < 0) {
    ((char*)ret)[--index] = '-';
  }

  return ret;
}

Well, the code works. It's definitely not as efficient as it could be, though; you're using floating-point math to accomplish something that could be done entirely in integer operations.

You forgot to #include all the headers you need. By my count, that's

#include <fenv.h>  // feclearexcept
#include <limits.h>  // INT_MIN
#include <math.h>  // floor, log10
#include <stdlib.h>  // abs
#include <errno.h>  // errno
#include <string.h>  // memset

Plus wherever print_error_msg comes from; that's an undefined symbol in the code you posted. I'm guessing it does something similar to perror(NULL)?

If I were writing itoa, I'd start by assuming that I already had the memory allocation taken care of. Then filling the buffer is as easy as what you wrote:

size_t result_length = <some magic>;
char *result = malloc(result_length + 1);
result[result_length] = '\0';
if (number < 0) {
    result[0] = '-';
    number = -number;
}
for (int i = result_length, n = number; n != 0; n /= 10) {
    result[--i] = '0' + (n % 10);
}

(I'd special-case 0 and INT_MIN at the top of the function so that we don't have to worry about them down here.)

Then the only missing piece is the <some magic>. But that's easy — we just need to write some code to count the digits of number. And we already have some code to print the digits of number! So I'd just repeat that code again.

int result_length = (number < 0);
for (int n = number; n != 0; n /= 10) {
    result_length += 1;
}

There — the only headers we needed were <stdlib.h> (for malloc) and possibly <limits.h> (for INT_MIN).

Notice that I wrote '0' + x where you'd written (char)((int)'0' + x). C isn't Pascal or Python; there's no need to jump through hoops like chr(ord('0') + x) here. Characters in C are small integers, and you can do math on them just as you can on any other integer type.

Moreover, you should do math on character types, if it eliminates the need for a cast operation. Type-casting in C and C++ is a code smell; if you really need a cast, something is horribly wrong with your code. In this case, fortunately, you don't need one.

Another place your code uses casts is

void *ret;
...
((char*)ret)[--index] = '\0';

Kudos for using '\0' instead of 0; that's a nice way to indicate to the reader what this particular zero semantically represents. But kill the cast! In this case, you think you need the cast because ret is a void*. However, the only places you refer to ret, you cast it to char*. This is a huge honking sign that ret really wants to be char*.

char *ret;
...
ret[--index] = '\0';

Problem solved. ret had no business being a void* in the first place.

No comment on the theoretical correctness of the floating-point stuff; because, as I said, I wouldn't have used floating-point to print integers in the first place. I have tested your code on all 4 billion 32-bit integers, though, and can report that it works fine in practice.


Longest Collatz sequence in Scala I am trying to find the longest Collatz Sequence:

  Which starting number, ≤ N, produces the longest chain? If many possible such numbers are there print the maximum one.  (1 ≤ N ≤ 5×106)

I tried optimizing the code to include memoization still getting Time Limit Exceeded, for these solutions. Wanted some help to understand where the method is consuming time.

Implementation Using HashMap As Memoized Cache:

object CollatzSeq {
  private final val mapOfCollatzSequence =
    scala.collection.mutable.HashMap[BigInt, BigInt]()
      .withDefaultValue(0)

  def longestCollatzSeq(n: BigInt): BigInt = {
    def longestCollatzSeqHelper(n: BigInt, count: BigInt = 0): BigInt = {
      val mapValueOfN = mapOfCollatzSequence(n)
      n match {
        case n if n == 1 => count
        case _ if mapValueOfN != 0 =>
          count + mapValueOfN
        case n if n % 2 == 0 =>
          val updateForEven = longestCollatzSeqHelper(n/2, count + 1)
          mapOfCollatzSequence.update(n, updateForEven - count)
          updateForEven
        case n if n % 2 != 0 =>
          val c = longestCollatzSeqHelper(3 * n + 1 , count + 1)
          mapOfCollatzSequence.update(n, c - count)
          c
      }
    }
    longestCollatzSeqHelper(n)
  }
}

Implementation Using Array As Memoized Cache:

object CollatzSeq {

  // Using Array As Performance Is Better Than Map
  private final val collatzSeq = Array.fill[BigInt](5* 1000 * 1000)(0)
  def longestCollatzSeq(n: BigInt): BigInt = {
    def longestCollatzSeqHelper(n: BigInt, count: BigInt = 0): BigInt = {
      val countOfN = collatzSeq((n - 1).toInt)
      n match {
        case n if n == 1 => count
        case _ if countOfN != 0 =>
          count + countOfN
        case n if n % 2 == 0 =>
          val updateForEven = longestCollatzSeqHelper(n/2, count + 1)
          collatzSeq.update((n-1).toInt, updateForEven - count)
          updateForEven
        case n if n % 2 != 0 =>
          val updateForOdd = longestCollatzSeqHelper(3 * n + 1 , count + 1)
          collatzSeq.update((n-1).toInt, updateForOdd - count)
          updateForOdd
      }
    }
    longestCollatzSeqHelper(n)
  }
}

Performance Reference

There are a few things you could do to make what you've got a bit faster:

The numbers get quite big but BigInt is overkill (and slow). The longestCollatzSeqHelper() method will have to deal with values in the Long domain, but the problem set, and results, are well within the Int range, which will be faster.
Because of the large numbers, you're cache could grow immensely. Perhaps beyond system resources.
You're testing n as many as 4 times per iteration. That can be reduced to 2.

But this is mere window dressing. When you encounter HackerRank timeout limits, code optimization usually doesn't get you very far. 9 times out of 10 you need to go back, re-examine the problem statement, and rethink your approach to it.

When I tackled this problem I started by caching calculations, but eventually I gave that up and started focusing on the other end:

To find the longest Collatz length for numbers <= N, I don't have to count the Collatz length for every number below N. There's a threshold below which it is impossible for the C-length to surpass the lengths for numbers above that threshold.
Some numbers don't need to have their Collatz length calculated at all. Due to a simple mathematical property it is impossible for its C-length to surpass its neighbors.

With this in mind, and caching for reuse only individual results within a problem set, you should be able to pass this challenge.


Reading a dynamic record length binary file I am trying to read a dynamic record length binary file, which has data like this:

field1field2field3field4vector1vector2

Here, field3 defines the occurrence of vectors.

For example, field3 is 2, then vector1&2 would be present and if the value is 3 then vector1,2&3 would be present.

Let's say field1-4 is fixed, the length is 2652 and each vector length is 301. field3 is 3 bytes from position 2396.

I have written the below code which is working fine but giving very bad performance.

my $string;
my $rep_factor;
my $size;

open (FILE, $ARGV[0]) or die $!;
my $re = 2396;
my $rec = 0;
while (<FILE>) {
   seek(FILE,$re,0);
   read(FILE,$rep_factor,2);
   my $rep_fact = undefined2defined(convert2ascii_decimal($rep_factor,0));
   $size = ($rep_fact * 301) + 2652;
   seek(FILE,$rec,0);
   read FILE,$string,$size;

   filewrite ($ARGV[1], recordparse($string));
   $rec = $size + $rec;
   $re = $size + 2396;
}

open (FILE, $ARGV[0]) or die $!;

It won't really make a difference in this script, but the standard is to use a variable rather than a bareword.

open (my $fh, $ARGV[0]) or die $!;

This gives you the benefits of lexical scoping.

while (<FILE>) {

What this says is while there are still lines in <FILE>, do the things in the block.  It separates lines by the line ending marker, which is normally a linefeed.  This would normally be used:

while ( my $line = <$fh> ) {
    chomp $line;

Then you could break the line up into parts with unpack.

    my ( $field1and2, $vector_count, $field4, $vector_field ) = unpack( 'A2395 A3 A254 A*', $line );

Note that I may have the format wrong.  It's easier if one can test against a valid file.  You may not even need that much:

    my ( undef, $vector_field ) = unpack( 'A2652 A*', $line );

It seems like you only use the vectors portion.

Instead, you use the while loop just to keep count.  However, that wouldn't normally work unless records were terminated with linefeeds.

open (my $fh, $ARGV[0]) or die $!;
while ( my $line = <$fh> ) {
    chomp $line;

    filewrite ($ARGV[1], recordparse($line));
}

See how that's simpler?  No more seeks.  This doesn't even require unpack, as it looks like you pass the whole line to recordparse.

One more thing:

filewrite ($ARGV[1], recordparse($string));

I'm not sure what this function does, but it looks like it takes a file name and writes to it.  This suggests that it opens the file each time.  Better would be to open the file once and pass a filehandle to it.

If you were getting multiple records with the original code but there are no linefeeds in the file, then I suspect that the seek and read commands are resetting the line input operator.

Alternate version to handle reading the entire file at once (assumes no linefeeds in the data).

open (my $fh, $ARGV[0]) or die $!;
my $file = <$fh>;
while ( '' ne $file ) {
    my $rep_factor = substr $file, 2396, 3;
    my $vector_count = undefined2defined(convert2ascii_decimal($rep_factor, 0));
    my $size = ($vector_count * 301) + 2652;

    my $record = substr $file, 0, $size;
    $file = substr $file, $size;

    filewrite ($ARGV[1], recordparse($record));
}

This uses substr rather than unpack because it's easier to specify different lengths with substr.  The first version asks for $size bytes starting at the beginning.  The second version says to return everything but the first $size bytes.

I'm a little worried that there is some maximum length that the <> operator returns, but I don't know what it might be.  Since I don't have an example file, I'm not testing the code at all.


Counting DNA nucleotides in C I have written code to solve the following Rosalind problem. This is my first time writing in C and I would like a review of my code, particularly in regard to correctness and performance.

#include <stdio.h>
#include <string.h>
int main() {

    /* Input DNA of length at most 1000 nucleotides */
    char dna[1000];
    FILE *input;
    input = fopen("/Users/James/Desktop/rosalind_dna.txt", "r");
    fgets(dna, 1000, input);
    fclose(input);

    /* Count frequency of symbols 'A', 'C', 'G' and 'T' in DNA */
    int i, a, c, g, t;
    for(i=0; i<strlen(dna); i++) {
        if (dna[i] == 'A') {
            a += 1;
        }
        else if (dna[i] == 'C') {
            c += 1;
        }
        else if (dna[i] == 'G') {
            g += 1;
        }
        else if (dna[i] == 'T') {
            t += 1;
        }
    }

    /* Output frequency of DNA nucleotides */
    printf("%i %i %i %i", a, c, g, t);
    return 0;
}

Updated Code

#include <stdio.h>
#include <string.h>
#define READ_BUFFER_SIZE 1000

int main() {

    /* Input DNA of length at most 1000 nucleotides */
    char dna[READ_BUFFER_SIZE];
    FILE *input;
    input = fopen("/Users/James/Desktop/rosalind_dna.txt", "r");
    if (input == NULL) {
        perror("Error");
    }
    fgets(dna, READ_BUFFER_SIZE, input);
    fclose(input);

    /* Count frequency of symbols 'A', 'C', 'G' and 'T' in DNA */
    int aCount, cCount, gCount, tCount;
    char *current;
    for(current = dna; *current; ++current) {
        switch(*current){
            case 'A':
                ++aCount;
                break;
            case 'C':
                ++cCount;
                break;
            case 'G':
                ++gCount;
                break;
            case 'T':
                ++tCount;
                break;
            default:
                printf("Error: Invalid nucleotide\n");
                break;
        }
    }

    /* Output frequency of DNA nucleotides */
    printf("%i %i %i %i", aCount, cCount, gCount, tCount);
    return 0;
}

I am no C programmer but there are a few things that come to my eye:

Static size

You always read at most 1000 characters, what if the file size is much bigger?
Instead you would loop trough the file, reading/processing it chunkwise.

Magic Number

Even if you use the static size you should not enter it at multiple locations. Instead use a constant for it to have to apply changes only in one place:

#define READ_BUFFER_SIZE 1000
//...
char dna[READ_BUFFER_SIZE];
//...
fgets(dna, READ_BUFFER_SIZE, input);

Loop termination

It might be optimized by the compiler but unoptimized your loop end check is n times (for n = strlen(dna)) which itself is takes \$O(n)\$, making your loop \$O(n^2)\$.

Looping

You index through the loop which is more costly than walking a pointer through it like:

char *current;
// loop until the '\0' terminator is encountered
for(current = dna; *current; ++current)

However, note as stated in https://stackoverflow.com/a/2305783 that modern compiler will optimize the indexed version to the same efficient code as the pointer version.

Switching

To find the counter for the current base you are using cascading ifs, which is suboptimal. Instead use a switch statement:

switch(*current) {
default: assert("Invalid base name!");
case 'A':
//...

Incrementing

This is most likely optimized by the compiler but still: You are incrementing so use the (prefix) increment operator ++a instead of a += 1;.

Naming

In this short example the names are nearly okay but in a longer program you would want to expand the variable names e.g.:

dna -> dnaChunkBuffer (assuming you work in chunks)
input -> inputFile
i -> currentBaseIndex
a, c, g, t -> base*Count

Code-Reuse

As I said I am no C programmer but I could imagine that there is a standard algorithm which helps you count the different characters in a string. If so, I would go for that.

Error Handling

It is just a small example but what happens if there is no file at the location you entered? Your program does not account for that. You should at least test if the fopen function succeeded and given an error otherwise.


Guessing a random number The program asks for a user to set the max number of the range. That number is used to set the max range of a randomly generated number. The program keeps track of the number of guesses. I have taken a couple classes recently on different programming languages. I wrote a Hi/Low program in C a while back and decided to convert it to Python 3. I would appreciate someone looking at the code of the program and give me feedback on what I did good/bad and ways that I could improve.

def tooHigh():
    guess = int(print("Your guess is too high, please try again: "))
    return guess

def tooLow():
    guess = int(print("Your guess is too low, please try again: "))
    return guess

def guessLoop(x):
    guess=x
    counter+=1
    if guess>gennum:
        guess = tooHigh()
    elif guess<gennum:
        guess = tooLow()
    elif guess==gennum:
        print("you guessed it!")
    return guess

import random
range=int(input("Please enter the maximum number of the range you would like to guess within: "))
gennum = random.randrange(range+1)
guess = int(input("Please enter a number within the range: "))
counter =1

if guess==gennum:
    print("Congrats, you guessed right on the first try!")

while not guess==gennum:
    loop(guess)

print("It took you",counter,"guesses to guess correctly.")

Here are a few comments:

1) There are three different places in the code where you ask the user to enter the next number to guess. This is not very readable and will backfire if and when you want to extend this code. It's better to have something like this:

def getNextGuessFromUser(messageToPrint):
    print messageToPrint + '\n'
    return int(input())

Then remove the tooHigh, tooLow and initial guess calls, and replace them with calls to this function. By the way, I'm not sure how your reading of guesses works with the print call; looks like a defect to me.

2) Unnecessary usage of global variables. guess should be generated for the first time inside guessLoop, and not rely on the outer code to initialize it.

3) Double check of the right guess. First, you do it in guessLoop to print the right message, and then again in the main module. Again, all of this should be happening in guessLoop: something like this

def guessRun(rightAnswer):
   counter=1
   guess=getNextGuessFromUser("Please enter a number within the range: ")

   while guess != rightAnswer:
     if guess>rightAnswer:
        guess = getNextGuessFromUser("<Too high message>")
     elif guess<rightAnswer:
        guess = getNextGuessFromUser("<Too low message>")
     counter+=1

   print("you guessed it!")
   return counter

4) Consider using PEP-8.


Instantiating a Database connection class in PHP I am building my own MVC and I have a couple questions about making a database connection and what is the best practice. I have a model User which interacts with the database and I wonder what is the best practice to instantiate the database connection class. Currently I made it so my User class extends the Database class because I don't feel like instantiating it in a constructor for example.

class User extends Database
{

    public function data($userid)
    {
        $getRow = $this->getConnect()->getRow("SELECT * FROM users WHERE userid = ?", [$userid]);

        return $getRow;
    }

}

And my Database class:

class Database
{

    private $dbh = NULL;
    private $instance = NULL;

    public function __construct($username = 'root', $password = '', $host = '127.0.0.1', $dbname = 'testdb', $options = [])
    {
        try  {
            $this->dbh = new PDO("mysql:dbname={$dbname};host={$host};charset=utf8", $username, $password);
            $this->dbh->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
            $this->dbh->setAttribute(PDO::ATTR_DEFAULT_FETCH_MODE, PDO::FETCH_OBJ);
        } catch (PDOException $ex) {
            throw new Exception($ex->getMessage());
        }

    }

    public function getConnect()
    {
        if($this->instance == NULL)
        {
            $this->instance = new self();
        }
        return $this->instance;
    }
}

Also, my getConnect method seems kinda hard coding. Do you think there is a smarter way to do this?

And one more thing, about public/private if anything is better to be private, I'd appreciate if you let me know.

Is a "user" a "database"?

That doesn't sound right.
A database is a tool or service that provides fast access to data via queries,
and a user sounds like a representation of a real-life user,
described by a set of characteristics.
A representation of a user may use a database to store its data,
but it is not a database.

In short, User should not extend Database.
A User could be a container for user data,
and User instances could be created by a UserFactory or UserRepository which could be implemented to fetch the user data from a database or some other backend.

Getting fields from a database

Instead of SELECT * queries,
it's recommended to name the required fields explicitly.
This can reduce the amount of data transferred from the database,
and it eliminates dependence on the ordering of columns.

What is getConnect ?

The getConnect function returns the Database singleton instance,
not a "connect".
Getting a "connect" seems meaningless.
getInstance is a more common name in singleton implementations.

public or private

As a good rule of thumb,
make everything private,
and then change some of them public as necessary.

The less implementation details are visible, the better.
Implementation details that are not visible give you the flexibility to change those details without affecting the users.


How to make business logic code maintainable when working with multiple states? I am developing a UI Application for a very simple phone app, the UI is based entirely on the state of Call in a container provided to me by a third party library.

Below is a sample Call object that I am working with;

public class Call {
    private boolean isTannoyCall;
    private long callStart;
    private int callId;
    private int callState;
    private CallDirection mCallDirection = CallDirection.OUTGOING;
    private int mediaStatus;
    private boolean mediaHasVideoStream;
    private long connectStart;
        private int lastStatusCode;
        private boolean callIsMuted;
        private boolean callIsInConference;

    //removed getters and setters
}

A populated call object is then passed around my application, in various places I have to carry out business logic to check if I should be carrying out an action etc. Below is an example of an event listener method for new calls within the app.

public void update() {
    int activeCallCount = 0;
    int incomingCallCount = 0;

    for (Call call : container.getCalls()) {

        boolean isIncomingDirection = call.getCallDirection() == CallDirection.INCOMING;

        boolean isIncomingCall = isIncomingDirection
                && call.getCallState() == CallState.INCOMING && !call.isTannoyCall();

        boolean isActiveCall = call.isActive()
                && call.getCallState() != call.InvState.INCOMING && !call.isTannoyCall();

        if (isIncomingCall) {
            incomingCallCount++;
        } else if (isActiveCall) {
            activeCallCount++;
        }

        boolean isMissedCall = isIncomingDirection
                && call.getCallState() == call.InvState.DISCONNECTED
                && call.getCallStart() == 0
                && !call.isTannoyCall();

        if (isMissedCall) {
            missedCallManager.increment();
        }
    }
    incomingCallUpdate(incomingCallCount);
    activeCallUpdate(activeCallCount);
}

In areas of the application where I need to do a lot of boolean based checks, like sample 1. How can I do this whilst keeping the code maintainable?  As every time I need to add a new condition (e.g. the latest "don't update if tannoy call"), I have to modify the appropriate if statements.

It seems data/feature envy. Move the boolean conditions to the Call object:

public class Call {
    ...

    public boolean isIncomingDirection()
        return callDirection == CallDirection.INCOMING;
    }

    public boolean isIncomingCall() {
        return isIncomingDirection()
            && callState == CallState.INCOMING && !isTannoyCall;
    }

    public boolean isActiveCall() {
        return isActive()
                && callState != InvState.INCOMING && !isTannoyCall;
    }

    public boolean isMissedCall() {
        return isIncomingDirection()
                && callState == InvState.DISCONNECTED
                && callStart == 0
                && !isTannoyCall;
    }
}

See also: Clean Code by Robert C. Martin, G14: Feature Envy


Password Attacker (Google apac test problem) Original Problem

Short Summary:

  Given a \$M\$ possible characters all of which must be used at least
  once, find the number of possible combinations of length (exactly)
  \$N\$.

  The first input contains the number of cases, followed by pairs of M,
  N. Output is modulo 1000000007(\$10^9+7\$).

  Example Input:

4
1 1
3 4
5 5
15 15

  Example Output:

Case #1: 1
Case #2: 36
Case #3: 120
Case #4: 674358851

The stupid solution is to simply generate all possible repeated_permutations and then exclude the ones which do not use at least M different characters (this of course is painfully slow).

def num_pw_slow(m,n)
  m.times.to_a.repeated_permutation(n).count do |row|
    m.times.map {|i| row.include? i}.all?
  end
end

The real solution:

It's noticeable that the number of solutions which include all M characters is also: (The number of repeated permutations) \$ - \$ (the number of repeated permutations missing at least one character).

The number of repeated permutations is simply \$m^n\$. The second is a little more involved. Combinations are given by \$n!/ (n-k)!k!\$ where \$n=m\$ and \$k = m-1\$. Permutations are \$(m-1)^n\$, the tricky part is we need the set of permutations, since each of the \$(m-1)\$ possible combinations differs from another by a single character, its repeated_permutations will contain a duplicates for all permutations which exclude that character.

For example if m=3, n=4, there are \$3^4\$ repeated_permutations possible - with repeated permutations of ab, bc, ca (possible combinations of m, with length m-1) missing one required character when expanded to length n, the set operation over which needs to eliminate duplicates from repeated_permutations of a, b, c So the solution is then:

3**4 - (2**4*3 - 1**4*3)

This can be generalized for any m, n e.g.

m**n - (m-1)*num_combinations**n
#second term is the number of duplicates amongst repeated_permutations of (m-1)
+ (m-2)*num_combinations**n
#third term is the number of duplicates in the second term
- (m-3)*num_combinations**n
#fourth term is duplicates in the third term
+ (m-4)*num_combinations**n
#and so on

Which gives the following code in Ruby:

(m**n) - (1..m-1).reduce(0) { |sum, i| sum + (i % 2 == 0 ? -1 : 1)*combinations(m, m-i)*(m-i)**n }

Full Solution:

def factorial(num)
  (1..(num.zero? ? 1 : num)).reduce(:*)
end

def combinations(n, k)
  factorial(n) / (factorial(n-k)*factorial(k))
end

def num_pw(m, n)
  (m**n) - (1..m-1).reduce(0) { |sum, i| sum + (i % 2 == 0 ? -1 : 1)*combinations(m, m-i)*(m-i)**n }
end

num_cases = gets.chomp.to_i
num_cases.times do |num|
  m, n = gets.chomp.split.map(&:to_i)
  puts "Case ##{num+1}: #{num_pw(m, n) % (10**9+7)}"
end

This took me over three hours to figure out (well over the exam running time) and quite inconsistent with my times on other problems (which are usually close or slightly better than average) - so for whoever solved this in under 45 minutes or so, I'd be very much interested how. Math SE gives a concise solution with the use of Sterling's number (I'd never heard of it before and I'm not sure if this is expected knowledge or not?)

This kind of problem can be solved with dynamic programming. The key points are :

you care about the number of combinations and not the list of them
assuming you know how to compute it for some values, you can go forward and compute it for even more values.

Here is how it could go (not verified whatsoever).

Given the length n (n > 0) and the number of distinct characters m (m > 0), you know that :

nb_comb(n, 1) = 1 which corresponds to strings with a single character.
nb_comb(n, m) = 0 if 1 <= n < m which corresponds to the fact that the string is too short to use all characters
nb_comb(n, m) = m * nb_comb(n-1, m-1) + m * nb_comb(n-1, m) if 1 < m < n which corresponds to the fact that to build a string of n characters, you can consider that you have to build a string of length n-1 and to add one at the end. Basically, you can either :

consider strings containing m-1 distinct characters and add the missing character : you have nb_comb(n-1, m-1) such strings for each of the m distinct characters. This contributes to m * nb_comb(n-1, m-1) combinations.
consider string containing all m characters and add any of the already added character : you have nb_comb(n-1, m) such strings and m ways to add an already existing character at the end. This contributes to m * nb_comb(n-1, m) new combinations.

(You can check that strings have not been counted twice (either the last character is a duplicate from another or it is not a duplicate).)

Without knowing it corresponds to Stirling numbers, I managed to deduce the same equations as the one in that stackoverflow question.

Python recursive solution to check it works :

import itertools

def nb(n, m):
    #print(n, m, "?")
    assert n>0
    assert m>0
    if m == 1:
        res = 1
    elif n == 1:
        res = 0
    else:
        res = m * (nb(n-1, m-1) + nb(n-1, m))
    if False: # check results
        slow_res = sum(1 for p in itertools.product(range(m), repeat=n) if len(set(p)) == m)
        assert slow_res == res
    return res

tests = [
    (1, 1, 1),
    (2, 2, 2),
    (3, 3, 6),
    (4, 3, 36),
    (5, 5, 120),
    (15, 15, 674358851),
]
for n, m, r in tests:
    nb_ = nb(n, m) % (1000000007)
    assert nb_ == r, "(%s,%s,%s != %s)" % (n, m, r, nb_)

This still gets way too slow for bigger solutions. Having a look at the functions calls for the case 15-15, for instance, adding a call to print at the beginning of the fonction and running python my_script | sort -n | uniq -c | sort -n, you get :

   ...
   1001 (1, 11)
   1001 (1, 5)
   1287 (2, 10)
   1287 (2, 7)
   1716 (2, 8)
   1716 (2, 9)
   2002 (1, 10)
   2002 (1, 6)
   3003 (1, 7)
   3003 (1, 9)
   3432 (1, 8)

showing that the fonction gets called with the same arguments thousands of times.

A solution would be memoization but a better one is to go the other way, generating the different values.

A way to do so is :

def nb2(n, m):
    # nb(i, j) is in results[i-1][j-1]
    results = [[None for j in range(m)] for i in range(n)]
    for j in range(m):
        results[0][j] = 0
    for i in range(n):
        results[i][0] = 1
    for i in range(1, n):
        for j in range(1, m):
            results[i][j] = (j+1) * (results[i-1][j-1] + results[i-1][j])
    return results[n-1][m-1]


Manipulating URL/array to remove the last element Is there a better way to do this using chaining?

var url = "www.mycompany.com/sites/demo/t1"
var x = url.split('/');
console.log(x);
var y = x.pop();
console.log(y,x);
var z = x.join("/");
console.log(z);

I tried something like but wouldn't work since pop just returns the last value and not the rest:

 parentUrl = self.attr('Url').split("/").pop().join("/");

pop() mutates the underlying list: it removes the last item (and returns it).

It seems you're looking for slice:

"www.mycompany.com/sites/demo/t1".split('/').slice(0, -1).join('/')
// -> gives: "www.mycompany.com/sites/demo"

.slice(0, -1) gives the elements of the list from the start until the end minus one item.

However, this is a very poor solution to drop the last path element from a URL. In particular, splitting a string to an array, drop the last element and join the string again is inefficient. It would be better to use lastIndexOf to find the position of the last /, and then substr to extract the part of the string before the last /.

var s = "www.mycompany.com/sites/demo/t1";
s.substr(0, s.lastIndexOf('/'));


Cross browser JavaScript implementation of xhr I wrote a small, cross browser compliant implementation of xhr requests in client JavaScript that closely follows the jQuery $.ajax() API and standards. The goal is to keep the xhr() very small in size, yet cover most common xhr scenarios and needs.

I'd like code review for performance, general best practice, and the most browser support (IE8+, Firefox, Chrome, Safari).

function xhr(options) {
    if(!options) {
        options = {};
    }

    var url = options.url ? options.url : window.location.href;
    var method = options.method ? options.method.toUpperCase() : 'GET';
    var data = options.data ? JSON.stringify(options.data) : null;
    var contentType = (typeof options.contentType !== 'undefined') ? options.contentType : null;
    var headers = (options.headers && typeof options.headers === 'object') ? options.headers : {};
    var username = options.username ? options.username : null;
    var password = options.password ? options.password : null;
    var timeout = options.timeout ? options.timeout : 0;
    var withCredentials = (typeof options.withCredentials !== 'undefined') ? options.withCredentials : false;
    var onSuccess = (options.onSuccess && typeof options.onSuccess === 'function') ? options.onSuccess : null;
    var onError = (options.onError && typeof options.onError === 'function') ? options.onError : null;
    var onTimeout = (options.onTimeout && typeof options.onTimeout === 'function') ? options.onTimeout : null;

    var _xhr = new window.XMLHttpRequest();
    _xhr.open(method, url, true, username, password);

    _xhr.timeout = timeout;

    _xhr.setRequestHeader('X-Requested-With', 'XMLHttpRequest');

    if(contentType) {
        _xhr.setRequestHeader('Content-Type', contentType);
    } else if(contentType !== false && data) {
        _xhr.setRequestHeader('Content-Type', 'application/x-www-form-urlencoded; charset=UTF-8');
    }

    if(withCredentials) {
        _xhr.withCredentials = withCredentials;
    }

    for (var key in headers) {
        _xhr.setRequestHeader(key, headers[key]);
    }

    _xhr.onreadystatechange = function() {
        if (this.readyState === 4) {
            var responseHeaders = this.getAllResponseHeaders();

            if (this.status >= 200 && this.status < 400) {
                if(onSuccess) {
                    onSuccess(this.responseText, this.status, responseHeaders, this);
                }
            } else {
                if(onError) {
                    onError(this, this.status, responseHeaders);
                }
            }
        }
    };

    if(onTimeout) {
        _xhr.ontimeout = onTimeout;
    }

    _xhr.send(data);
    _xhr = null;
}

Usage examples:

xhr({
    url: "http://foo.bar"
});

xhr({
    url: "http://foo.bar/create",
    method: "POST",
    data: 'foo=bar'
});

xhr({
    url: "http://foo.bar/create",
    method: "POST",
    contentType: "application/json; charset=UTF-8",
    data: { foo: "bar" },
    headers: {
        "X-Key": "Value"
    },
    timeout: 5000,
    onSuccess: function(response, statusCode, responseHeaders, xhr) {
    },
    onError: function(xhr, statusCode, responseHeaders) {
    },
    onTimeout: function(event) {
    }
});

Unfortunately, there is a new API for AJAX called fetch. It even has a polyfill. It even has a polyfill for older browsers which looks to me like its already doing what you aim to do.

But anyways, onto your code.

I see you have a long list of ternaries to set values or a default to the options. Instead of that, create an object hosting your defaults, and use Object.assign to merge it and the user config into an object. This throws away the need for doing a ternary/if-else on your code by providing sensible defaults.

var defaults = {
  method: 'GET',
  url: window.location.href,
  username: undefined,
  password: undefined,
  withCredentials: undefined
};

var mergedOptions = Object.assign({}, defaults, options);

var _xhr = new window.XMLHttpRequest();

_xhr.open(mergedOptions.method, mergedOptions.url, true, mergedOptions.username, mergedOptions.password);

_xhr.withCredentials = mergedOptions.withCredentials

For calling the callbacks, consider using call with a context of null. This way, the callback has no assumptions on what the context is and code doesn't accidentally bind something to the global scope.

Alternatively, consider returning a Promise from your function. This way, it acts more like jQuery and the standard fetch.

function xhr(options){
  return new Promise(function(resolve, reject){
    // setup XHR

    if (this.status >= 200 && this.status < 400) resolve(this.responseText, this.status, responseHeaders, this);
    else reject(this, this.status, responseHeaders);

    // send XHR
  });
}

xhr({...}).then(onSuccess, onError);


SciPy sparse: optimize computation on non-zero elements of a sparse matrix (for tf-idf) I have a sparse matrix (term-document) containing integers (word counts/tf) and I am trying to compute the tf-idf, for every non-zero value in the sparse-matrix.

The formula for tf-idf I am using is:

log(1 + tf) * log(N / (1 + df))   # N is the number of coloumns of the matrix
# tf is the value at a cell of the matrix
# df is the number of non-zero elements in a row

So for a matrix csr, at an index [i,j] with a non-zero value, I want to compute:

csr[i,j] = log(1 + csr[i, j]) * log(csr.shape[1] / (1 + sum(csr[i] != 0))

Since I have a large matrix, I am using sparse matrices from scipy.sparse. Is it possible to do the tf-idf computation more efficiently?

import numpy as np
import scipy.sparse
import scipy.io

csr = scipy.sparse.csr_matrix(scipy.io.mmread('thedata'))

for iter1 in xrange(csr.shape[0]) :

    # Finding indices of non-zero data in the matrix
    tmp,non_zero_indices = csr[iter1].nonzero()
    # dont need tmp

    df = len(non_zero_indices)
    if df > 0 :
        # This line takes a long time...
        csr[iter1,non_zero_indices] = np.log(1.0+csr[iter1,non_zero_indices].todense())*np.log((csr.shape[1])/(1.0+df))

I'm making a fair few assumptions about the internal format that may not be justified, but this works on the demo data I tried:

factors = csr.shape[1] / (1 + np.diff(csr.indptr))
xs, ys = csr.nonzero()

csr.data = np.log(csr.data + 1.0) * np.log(factors[xs])

All I do is work on the internal dense data structure directly.


Router for quering MySQL results in NodeJS What should I consider in order to refactor the following code ?

It seems very close to proudly become a Pyramid of Doom:

router.get('/:id', function(req, res) {

    getId = req.params.id
    obj = {}

    db.con.query('SELECT * FROM employees where id=?', getId, function(err, results) {

        if (err) {
            console.log('error in query')
            return

        } else {
            try {
                obj = {
                    id: results[0].id,
                    name: results[0].name,
                    location: results[0].location
                }
                res.render('showuser')

            } catch (err) {
                res.status(500)
                res.render('error', { error: err, id: getId })
            }
        }
    })
});

Are we actually using the obj variable? It seems like you are not using it unless you were intending to pass to the render function. I don't think the try block is necessary. You can first check if results contains elements in it, so doing results[0].X will not return you an error since results[0] will never be undefined.

Here's an improved version:

router.get('/:id', function(req, res) {
  const getId = req.params.id;

  db.con.query('SELECT * FROM employees where id = ?', getId, function(err, results) {
    if (err || !results.length) {
      console.log('error in query');
      res.status(500).render('error', { error: err, id: getId });
      return;
    }

    var obj = {
      id: results[0].id,
      name: results[0].name,
      location: results[0].location
    };
    res.render('showuser', obj); // Assuming you're using obj to render
  });
});


Reading String data into a Tree like nested grid structure (like Sudoku Board) Context

The original idea is to create an efficient grid structure for a Sudoku like board but this can be applied to many such grid structures (like wavelet transform on images, JPEG2000 etc). This particular one is basically a 9x9 Board which is made up of 3x3 Blocks where each Block is made up of 3 x Axis a a a type. You may think Axis type like a list limited to having only 3 elements (no more no less) and no empty or identiy element.

I have defined the Axis, Block and Board types as;

data Axis a =  Axis { _0 :: a
                    , _1 :: a
                    , _2 :: a
                    } deriving (Eq, Functor)
type Block a = (Axis(Axis a))
type Board a = (Axis(Axis (Block a)))

instance Show a => Show (Axis a) where
  showsPrec _ (Axis x y z) = shows x . (' ':) . shows y . (' ':) . shows z

As you notice, polymorphic Axis type is like a ternary tree with no leaves where the nodes can either be another Axis a type or an a type value. Does anybody know a general name for such data types in Haskell literature?

Anyways, by doing so, once i have a Board then i can easily access any Block or Cell quite efficiently. To access one of the 9 Blocks i can now simply do

_0 . _1 $ myBoard -- like (row 0, col 1) from myBoard

where to gain access to any cell within the Board all i have to do is to get to the Block and then to the cell like

_2 . _1 . _0 . _1 $ myBoard --  coord of cell . coord of Block => (_2 . _1) . (_0 . _1)

Problem

The problem arose when building up my Board type from a provided String. Since i post this here, I have done it but it smells like a fish and i think there must be a better way.

So the input data comes in the form of a string of 81 numeric characters among 0..9. The string should fill up the board line by line. So if my board supposed to be a 2D list, then a chunksOf 9 . map ((read :: String-> Int) . pure) would be sufficient. However the Board type is a 4 fold nested Axis type.

To start with, I couldn't even find a way to write a Read instance for the Board type at all. My solution is to first convert the flat list into a nested list of proper structure like.

nestList :: [a] -> [[[[a]]]]
nestList = map transpose . chunksOf 3 . chunksOf 3 . chunksOf 3

and then after a 2 days war with GHC infinite type errors and whatnot, i could finally come up with this stinky part.

axify :: [[[[a]]]] -> Board a
axify ([a,b,c]) = Axis (Axis (axify' $ a !! 0) (axify' $ a !! 1) (axify' $ a !! 2))
                       (Axis (axify' $ b !! 0) (axify' $ b !! 1) (axify' $ b !! 2))
                       (Axis (axify' $ c !! 0) (axify' $ c !! 1) (axify' $ c !! 2))
                  where
                  axify' ([a,b,c]) = Axis (Axis (a !! 0) (a !! 1) (a !! 2))
                                          (Axis (b !! 0) (b !! 1) (b !! 2))
                                          (Axis (c !! 0) (c !! 1) (c !! 2))

Now this works. I can fill the Board properly from a flat string and the Show instance just gives back a stringified version of the nestedList.

Question

Could anybody please help me with a proper Read instance or at least an axify function that is idiomatic (generalized to n fold Axis a type)?

Many thanks in advance.

Consider the polymorphic function:

axis :: [a] -> Axis a
axis [x,y,z] = Axis x y z

You want to apply this at each of the four "list levels" of the output [[[[a]]]] of nestList.  So, if you have:

lst4 : [[[[Char]]]]
lst4 = nestList "295743861431865927876192543387459216612387495549216738763534189928671354154938672"

you want to write:

axis lst4 :: Axis [[[Char]]]

to replace the outermost list with an Axis, then you want to fmap axis over the Axis:

fmap axis . axis $ lst4 :: Axis (Axis [[Char]])

to replace the second-level list with an Axis, then you want to do a double-fmap (fmap axis):

fmap (fmap axis) . fmap axis . axis $ lst4 :: Axis (Axis (Axis [Char]))

to replace the third-level list, and finally the innermost list:

fmap (fmap (fmap axis)) . fmap (fmap axis) . fmap axis . axis $ lst4 :: Axis (Axis (Axis (Axis Char)))

So, you actually have:

axify :: [[[[a]]]] -> Board a
axify = fmap (fmap (fmap axis)) . fmap (fmap axis) . fmap axis . axis

Alternatively, instead of using the functor instance for Axis, you could use the functor instance for lists, starting from the inside out:

axify = axis . map axis . map (map axis) . map (map (map axis))


Adding new methods to GenericDataAccess I have this GenericDataAccess class to interact with database from my school. I want to ask for some suggestions and advice in how to to improve it and add new methods to it for future use and reference

public static class GenericDataAccess
{
    static GenericDataAccess()
    {

    }

    /// <summary>
    /// creates and prepares a new DbCommand object on a new connection
    /// </summary>
    /// <param name="command"></param>
    /// <returns></returns>
    public static DataTable ExecuteSelectCommand(DbCommand command)
    {
        // The DataTable to be returned
        DataTable table;

        try
        {
            //open connection
            command.Connection.Open();
            //execute command & save the result in DataTable
            DbDataReader reader = command.ExecuteReader();
            table = new DataTable();
            table.Load(reader);
        }
        catch (Exception ex)
        {
           Utilities.LogError(ex);
            throw;
        }
        finally
        {
            command.Connection.Close();
        }

        return table;
    }

    /// <summary>
    /// creates and prepares a new DbCommand object on a new connection
    /// </summary>
    /// <returns></returns>
    public static DbCommand CreateCommand()
    {

        //obtaine the database provider name
        string dataProviderName = CompuData_ProjectManagerConfiguration.DbProviderName;
        //Obtain database connection string
        string connectionString = CompuData_ProjectManagerConfiguration.DbconnctionString;
        //Create new Data Provider Factory
        DbProviderFactory factory = DbProviderFactories.GetFactory(dataProviderName);
        //obtain a database specific connection object
        DbConnection conn = factory.CreateConnection();
        //set the connection string
        conn.ConnectionString = connectionString;
        //create a database specific command object
        DbCommand comn = conn.CreateCommand();
        //set the command type to store proc
        comn.CommandType = CommandType.StoredProcedure;

        //return the initialized command object
        return comn;
    }

    /// <summary>
    /// execute an update, delete, or insert command and return the number of affected rows
    /// </summary>
    /// <param name="command"></param>
    public static int ExecuteNonQuery(DbCommand command)
    {
        //the number of affected rows
        int affectedRows = -1;
        // Execute the command making sure the connection gets closed in the end
        try
        {
            //open connction
            command.Connection.Open();
            affectedRows = command.ExecuteNonQuery();
        }
        catch (Exception ex)
        {
            Utilities.LogError(ex);
            throw;
        }
        finally
        {
            command.Connection.Close();
        }

        return affectedRows;
    }

    // execute a select command and return a single result as a string
    public static string ExecuteScaler(DbCommand command)
    {
        // the value to be retuned
        string value = "";

        // Execute the command making sure the connection gets closed in the end
        try
        {
            //open the connection of command
            command.Connection.Open();
            //execute the command and get the number of affected rows

            value = command.ExecuteScalar().ToString();
        }
        catch (Exception ex)
        {
           Utilities.LogError(ex);
            throw;
        }
        finally
        {
            command.Connection.Close();
        }
        return value;
    }
}

First of all, there is no need to write a generic data access class.  Use the Data Access Application Block.  It gives you everything you need to access a database.  Why reinvent the wheel.  You can even download the source code to this if you want to see how Microsoft does it.

Having said that, if you are doing this just as an exercise then I see a couple things...

I am not a big fan of static classes (except for possibly simple "function" classes, classes which provide generic functions such as a function to convert unix timestamp to datetime).  Making classes static makes it more difficult to unit test.

Along these lines of unit testing I would also recommend you implement an interface so you could can make unit testing easier through IOC / dependency injection

Your methods should only do one thing.  Adding too much functionality into each method limits the flexibility of the caller.  You should have a method that creates a connection, another that creates a command, and another that executes the command.  Example, I would not open and close my connections within the "Execute..." methods because what if the caller wants to open the connection so they can execute multiple commands in the same connection.  If you want to add methods that do multiple things then they should be separate from your generic data access class, and they should call your generic data access methods.

I do not see anything to handle transactions.  This would be something you would want to add.


Implementing `split` I implemented the split function in Haskell:

split :: (Eq a) => a -> [a] -> [[a]]
split _ [] = []
split x ys = f : split x rest
  where (f, rest) = break (== x) (dropWhile (== x) ys)

Note that I'm calling dropWhile (== x) since break's second-tuple value will include the "broken on" value.

example:

*Main> break (== 'a') "dogacactus"
("dog","acactus")

Testing

*Main> split2 '3' "123aaaBBB3"
["12","aaaBBB",""]

Generally it looks good, at least to an Haskell beginner like me.
I find the use of dropWhile to be a bit confusing. I admit I had to read your comment to understand why you used it.

I think it could also have introduced a bug. What is the expected behaviour for split ',' ",,"?
Your code returns [""] but I would expect ["","",""]. Was your behaviour intended?

This solution fixed the issue.

split :: (Eq a) => a -> [a] -> [[a]]
split _ [] = []
split separator ys = f : (split separator (dropSeparator separator rest))
  where (f, rest) = break (== separator) ys

dropSeparator :: Eq a => a ->  [a] -> [a]
dropSeparator _ [] = []
dropSeparator separator (x:xs) = if x == separator then xs else x:xs

I prefer using meaningful variable names, such as separator, even if I'm not sure if it is idiomatic Haskell. Ditto for the parentheses and the introduction of the helper function.


Parsing mathematical expressions in Reverse Polish Notation I've wanted to make a calculator for a long time now, so I wrote one of my first parsers, where the input is in reverse polish notation, \$1 \ 1 +\$ becomes \$2\$.

It supports to following operators; +, -, *, /, // floor division, ^ or ** exponent, and % or mod modulo.

While it's the best calculator I've written, the tokens are limited to only ones that take two operators. And I don't really like the idea of having a separate dictionary for ones that take more or less. This is a shame as now I can't add functions like sqrt or sin, or constants like pi or e, without having to have a different group for each.

Personally I dislike the error you get when you ^C out of the program, and so I silenced it. I also made a new error CalculatorError to not mask unknown errors, to easily exit out of the current equation if more than one function is called, make_pop, and to have a human readable error message.

TOKENS = {
    '+': lambda a, b: a + b,
    '-': lambda a, b: a - b,
    '*': lambda a, b: a * b,
    '/': lambda a, b: a / b,
    '^': lambda a, b: a ** b,
    '**': lambda a, b: a ** b,
    '%': lambda a, b: a % b,
    'mod': lambda a, b: a % b,
    '//': lambda a, b: a // b
}

class CalculatorError(Exception):
    pass

def make_pop(stack):
    def pop():
        try:
            return stack.pop()
        except IndexError:
            raise CalculatorError('Not enough arguments for function')
    return pop

def call():
    stack = []
    pop = make_pop(stack)
    buf = []

    calculation = iter(input('> ') + ' ')

    for char in calculation:
        if char not in ' \t':
            buf.append(char)
            continue

        token = ''.join(buf)
        if not token:
            continue

        if token in TOKENS:
            a = pop()
            b = pop()
            function = TOKENS[token]
            try:
                return_value = function(b, a)
            except ZeroDivisionError:
                raise CalculatorError("Can't devide by zero")
            else:
                stack.append(return_value)
        else:
            try:
                token = float(token)
            except ValueError:
                raise CalculatorError('{!r} is not a number'.format(token))
            stack.append(token)

        buf = []

    if len(stack) > 1 or buf:
        raise CalculatorError('Invalid function, not enough operators.', stack, buf)

    if stack[0] % 1:
        print(stack[0])
    else:
        print(int(stack[0]))

def main():
    while True:
        try:
            call()
        except CalculatorError as e:
            print(e)

if __name__ == '__main__':
    print('Press ^C to exit')
    try:
        main()
    except KeyboardInterrupt:
        pass
    # For some reason my Windows raises EOF on ^C...
    except EOFError:
        print()
    print('Goodbye!')

Some example output is:

Press ^C to exit
> 2 2 + 2 2 - /
Can't devide by zero
> 3 5 * 2 //
7
> 20 mod 11
Not enough arguments for function
> 20 11 mod
9
>
Goodbye!

I'm not quite sure what you're asking for, but I was bored so I typed this up anyway. Hopefully it helps. ;-)

Working from the outside in:

Invocation

if __name__ == '__main__':
    print('Press ^C to exit')
    try:
        main()
    except KeyboardInterrupt:
        pass
    # For some reason my Windows raises EOF on ^C...
    except EOFError:
        print()
    print('Goodbye!')

This is all fine, but it probably should be in the main() function, and you just write

if __name__ == '__main__':
    main()

Mostly, this is a matter of convention, but also, it's handy to have a function that can be run from another Python script that will just do everything.

Organizing the driver code

The process of evaluating an RPN expression is something that makes a lot of sense to put as its own function. That's a self-contained task you could easily want to reuse elsewhere. So I would suggest making a function evaluate_rpn(expr) which does exactly that. You can then use this function in your main loop as follows:

def main():
    print(...)
    try:
        while True:
            try:
                print(evaluate_rpn(input('> ')))
            except CalculatorError as e:
                print(e)
    except KeyboardInterrupt:
    # and so on

Evaluating

This new evaluate_rpn() function is going to do mostly the same thing as your call() function currently does, except that instead of prompting using input(), it will take a string argument, and instead of printing the result, it will return it. Let me pick on a few things in that function, though.

Let the stack just be a stack

stack = []
pop = make_pop(stack)

The only real purpose of make_pop is to make it so that you get a CalculatorError instead of an IndexError when a function or operator doesn't have enough arguments. But I don't think it makes sense to do that. See, stack is just a stack. (Well, a list that is serving as a stack.) It knows nothing of numbers or arguments or calculators and really has no business raising a CalculatorError. The code in evaluate_rpn() is the thing that knows it is a calculator. That's where you should be raising CalculatorErrors. So instead of using make_pop, just watch for IndexErrors during the evaluation, catch them, and raise a CalculatorError in the handling code. As a side effect you won't need make_pop() anymore; you can just dispense with it entirely.

try:
    a = pop()
    b = pop()
except IndexError:
    raise CalculatorError(...)

Do tokenization the easy way

Your input strings consist of tokens (numbers, operators, functions) separated by whitespace, right? There's a built-in method to split a whitespace-separated string: string.split(). You don't need to bother with a buffer, or with iterating through the string's characters.

for token in calculation.split():
    # and so on

Don't overuse variables

You don't really have to save a and b and return_value separately; you only use them once each.

if token in TOKENS:
    try:
        stack.append(TOKENS[token](stack.pop(), stack.pop())
    except ZeroDivisionError:
        raise CalculatorError(...)
    except IndexError: # from a couple paragraphs ago
        # and so on

If different parts of this expression could raise the same type of error, then that would be a good reason to break it down, so that you could tell exactly where a given error comes from. But that's not an issue here. You'll only ever get ZeroDivisionErrors for one reason.

Same goes for the float-parsing part:

else:
    try:
        stack.append(float(token))
    except ValueError:
        raise CalculatorError(...)

For complicated sequences of code, you have to make a subjective judgement about where to use variables and where not to in order to make the code as clear as possible. But in this case, I think most programmers would agree that the variables are fairly unnecessary.

Let output formatting be controlled by the formatting routines

Evaluating an RPN expression doesn't include formatting - in particular, deciding whether the result is an integer or not is a separate task from just getting the result in the first place. So at the end of evaluate_rpn, just return a float. When you print it, instead of print(result) you can do

print('{:g}'.format(result))

and it will automatically strip trailing zeros as needed. Of course, this does display large numbers in scientific notation (2.89146e+07); if that's a problem, then you might have to do your own formatting.

Use descriptive variable names

Your TOKENS list doesn't actually contain tokens; it contains operators, and could be generalized to contain functions. So I'd suggest naming it OPERATORS or some such thing.

Allowing arbitrary numbers of arguments

This is really more about adding a feature than reviewing your existing code, but I might as well point you in the right direction. All you have to do is store the number of arguments a function or operator requires, in addition to the code that actually evaluates the operator. For example,

OPERATORS = {
    '+':    (2, lambda x, y: x + y),
    'sqrt': (1, math.sqrt),
    # and so on
}

Then, when you invoke an operator, instead of hard-coding two arguments like you did or like I did above,

stack.append(OPERATORS[token](stack.pop(), stack.pop())

you can check how many operands the operator requires,

n_operands, evaluator = OPERATORS[token]

pop that many from the top of your stack,

# these two lines together are like a "multipop"
operands = reversed(stack[-n_operands:])
del stack[-n_operands:]

and evaluate the operator with those operands,

stack.append(evaluator(*operands))

I leave it to you to figure out the best way to put try/except blocks around this code.

Actually, the way I've done it here has a problem dealing with the case where n_operands is zero (this would happen if you include constants or zero-argument functions in the list of operators). To get around that, you can use the following:

n_operands, evaluator = OPERATORS[token]
stack.append(evaluator(*[stack.pop() for _ in range(n_operands)]))

Instead of chopping off the whole end of the list, as in the previous method, this one pops off the last n_operands elements one at a time.


SharePoint Online inventory I'm creating a script to turn into a commandlet we can reuse as part of our custom PowerShell module. This works, but is very slow. I don't suspect it is anything to do with my code. I have to individually load each site to retrieve the title even though this property is visible off the $site object, it is always empty/null.

Anything else I can do to improve the efficiency?

$path = "C:\Users\$env:USERNAME\Desktop\SPOExport.csv"
$csv = "Title,URL,Sharing Capability,Storage Quota (MB),Template`r`n"
$sites = Get-SPOSite -Limit all | Select Url,SharingCapability,StorageQuota,Template
foreach($site in $sites){
    try{
        $context = New-Object Microsoft.SharePoint.Client.ClientContext($site.Url)
        $context.Credentials = New-Object Microsoft.SharePoint.Client.SharePointOnlineCredentials($credential.UserName, $credential.Password)
        $web = $context.Web
        $context.Load($web)
        $context.ExecuteQuery()
        $title = $web.Title
    }
    catch {
        $title = "Error fetching title"
    }
    finally{
        $csv +=$title + "," + $site.Url + "," + $site.SharingCapability + "," + $site.StorageQuota + "," + $site.Template
    }
}
$fso = new-object -comobject scripting.filesystemobject
$file = $fso.CreateTextFile($path,$true)
$file.write($csv)
$file.close()

Note that I have not fully adapted it into a commandlet yet, which will have all the commenting and help text available, I'm more worried about the code and the time it takes for this operation presently.

Well, I cannot tell why it's so slow, but this should solve your problem:

$sites = Get-SPOSite -Limit all -Detailed

-Detailed switch will cause Get-SPOSite to load all properties. It makes iterating all sites to get title unnecessary.


How can I solve my constructor injection code architecture? The current code solves the issue I had when trying to use property injection.

Problem: Every module must use constructor injection because of a circular reference that occurs when not using constructor injection with my factories and trying to use property injection

Reason: A module requires a client and each client must have its own list of instantiated modules.

What I want: I want every module that derives from BaseModule to be able to not declare a constructor and allow BaseModule to have IIrcClient injected via a property.

But I'm not sure how to use Ninject to correctly do this without getting a circular reference. Is there a way so that for each client, a new list of modules are injected into that client (where all modules then have an injected reference of the client they belong to).

.ToFactory() is an extension method of Ninject which creates a proxy based on an interface that creates an object with a dependency.

public class BaseClient : IClient
{
    private readonly IEnumerable<IModule> modules;

    protected BaseClient(IModuleFactory moduleFactory)
    {
         // each client has its own instances of modules set
         // each module needs a reference to the client
         this.modules = moduleFactory.Create(this);
    }
}

public class BaseModule : IModule
{
    private readonly IIrcClient ircClient;

    // Factory will inject and create
    protected BaseModule(IIrcClient ircClient)
    {
        this.ircClient = ircClient;
    }
}

public class Bot
{
    // Injected factory
    public Bot(Context context, IrcClientFactory factory)
    {
        this.factory = factory;
        this.context = context;
    }

    public void Start()
    {
        List<Network> networks = context.Networks.ToList();

        foreach (var network in networks)
        {
            // create new client...but also
            // new client gets injected with module factory
            // module factory then populates modules based on client
            IIrcClient ircClient = this.ircClientFactory.Create();
            ircClient.Connect(network);
            this.clients.Add(ircClient);
        }
    }
}

public class Program
{
    public void Main()
    {
        kernel.Bind<IIrcClient>().To<DefaultIrcClient>();
        kernel.Bind<IIrcClientFactory>().ToFactory();

        kernel.Bind<IModule>().To<DiceModule>().WithPropertyValue("Name", "Dice Module");
        kernel.Bind<IModule>().To<ModuleOne>().WithPropertyValue("Name", "ModuleOne");
        kernel.Bind<IModule>().To<RubyModule>().WithPropertyValue("Name", "Ruby Module");
        kernel.Bind<IModule>().To<AdminModule>().WithPropertyValue("Name", "Admin Module");

        // Module Factory will generate IEnumerable<IModule>
        kernel.Bind<IModuleFactory>().ToFactory();

        var bot = kernel.Get<Bot>();
        bot.Start();
    }
}

A possible fix, but I would have to manaually set the property instead of letting Ninject inject the property:

public class BaseClient : IClient
{
    private readonly IEnumerable<IModule> modules;

    protected BaseClient(IModuleFactory moduleFactory)
    {
         // let the factory create new modules
         // but dont pass the instance of the client
         this.modules = moduleFactory.Create();

         foreach(var module in modules)
         {
               module.IrcClient = this; // don't want to do this, but I think it would fix it
         }
    }
}

This way, I could create tons of modules without needing to always implement the base constructor and pass in the IClient dependency:

public class ChildModule : BaseModule
{
    // client was injected via property

    // override virtual methods as required.
}

Your question isn't crystal-clear about what it is exactly that your code is supposed to be doing. We have to infer the functionality from this block of code:

public void Start()
{
    List<Network> networks = context.Networks.ToList();

    foreach (var network in networks)
    {
        // create new client...but also
        // new client gets injected with module factory
        // module factory then populates modules based on client
        IIrcClient ircClient = this.ircClientFactory.Create();
        ircClient.Connect(network);
        this.clients.Add(ircClient);
    }
}

This code involves [what appears to be] private fields that you haven't included. I'll assume the following:

private readonly Context context;
private readonly IrcClientFactory factory;

If I read this correctly, your Bot class is depending on concrete types; they should be abstractions, like IContext and IIrcClientFactory (the latter being an abstract factory).

  Naming - Private Fields

  This might be of personal preference, but I like my private fields prefixed with an underscore. Having _context and _factory removes the ambiguity in the constructor and then the this qualifier becomes redundant:

public Bot(Context context, IrcClientFactory factory)
{
    _factory = factory
    _context = context
}

The comment // injected factory is a useless one; prefer XML comments for that purpose:

 /// <summary>
 /// Creates a new instance of a Bot.
 /// </summary>
 /// <param name="context">The data context.</param>
 /// <param name="factory">A factory that creates IrcClient instances.</param>
 public Bot(Context context, IrcClientFactory factory)
 {
     _factory = factory
     _context = context
 }

As a bonus, you'll get instant IntelliSense support for your constructor and its parameters.

Back to the foreach loop; the code you have here doesn't seem to be using the fields you've initialized in your constructor:

IIrcClient ircClient = this.ircClientFactory.Create();

I would have expected:

IIrcClient ircClient = this.factory.Create();

I would rewrite it like this:

var client = _factory.Create();

  Implicit Typing

  The abstract factory shouldn't be revealing the implementation of its product to its client; the factory's Create() method should be returning an implementation for IClient, and shouldn't have to even know what possible implementations the factory can spit out. Your code is expecting an IIrcClient which is an abstraction, but from the rest of your code I infer that IIrcClient extends the IClient interface. Bottom line, the code doesn't care what type is returned, and it should be working against all implementations of IClient (assuming Connect(Network) is a member of IClient).

  Implicit typing not only makes your code shorter and easier to read, it also emphasize that you don't really care what the actual type is.

After creating a client, the loop's body passes the network to the client through its Connect method, and then the client gets added to yet another unmentioned field, assuming:

private readonly _clients = new List<IClient>();`

public void Start()
{
    var networks = context.Networks.ToList();
    foreach (var network in networks)
    {
        var client = _factory.Create();
        client.Connect(network);
        _clients.Add(client);
    }
}

If the network parameter is passed into a field of the client, and used anywhere other than in the Connect method, it would probably be best to constructor-inject the network into the client inside the factory method. If it's passed into a field of the client but isn't used anywhere other than the Connect method, it would be best to constructor-inject the network into the client inside the factory method; what I'm trying to get to, is that constructor injection should always be prioritized over other forms of injections, whenever possible. In this case, I'd probably call the IClient implementation something like ConnectedClient and inject the network through the Create method, which returns an IClient with a connection, as the ConnectedClient implementing class' name says.

Reviewing the rest of this code is very hard, I'm not seeing all the pieces of the puzzle that I'd like to see in order to be confident in anything else I'd have to say.


Avoiding code duplication for reading in similar values for each derived class I am designing a program that handles many types of grooves TKY, VSingle under GrooveType.

Each of this groove is paired with its specific parameter SimulationParam (e.g. TKY will pair with SimulationParamTKY, which inherits from SimulationParam) under the Dictionary simParams. As such, if i want to include more types of grooves, I can add it under this dictionary.

SimulationParam contains ProcessParam, which has the parameters probeStartAngle and probeStartDist. These parameters are similar(in meaning) for both TKY and VSingle.

However, in the dialog, I want the user to be able to select which particular groove they want. When that particular option is selected, the respective values for probeStartAngle and probeStartDist is read in from their respective text box. For example, if Use TKY is selected, the ProcessParam.probeStartAngle value will be read from tbTKYStartAngle.

As such, when I extend this program to even more grooves (say 10 more), under btnProcess_Click(), a lot of code will actually be repeated.

Is there a better way to write this code to reduce the repetitions? Assuming the requirement is that the textboxes for each groove cannot be merged into a single one. (i.e. meaning i cannot read from the same textbox for say ProcessParam.probeStartAngle)

public partial class GrooveDialog : Form
    {
        public enum GrooveType : int
        {
            TKY = 0,
            VSingle = 1,
        }

        Dictionary<GrooveType, SimulationParam> simParams = new Dictionary<GrooveType, SimulationParam>();

        public GrooveDialog()
        {
            simParams.Add(GrooveType.TKY, new SimulationParamTKY());
            simParams.Add(GrooveType.VSingle, new SimulationParamVSingle());
        }

        private void btnProcess_Click(object sender, EventArgs e)
        {
            if (rbUseTKY.Checked)
            {
                var simParamTKY = (SimulationParamTKY)simParams[GrooveType.TKY];

                simParamTKY.processParam.probeStartAngle = Convert.ToDouble(tbTKYAngle.Text);
                simParamTKY.processParam.probeStartDist = Convert.ToDouble(tbTKYDist.Text);
            }
            else if (rbUseVSingle.Checked)
            {
                var simParamVSingle = (SimulationParamVSingle)simParams[GrooveType.VSingle];

                simParamVSingle.processParam.probeStartAngle = Convert.ToDouble(tbVSingleAngle.Text);
                simParamVSingle.processParam.probeStartDist = Convert.ToDouble(tbVSingleDist.Text);
            }
    }

public class ProcessParam
    {
        public ProcessParam() { }
        public double probeStartAngle { get; set; }
        public double probeStartDist { get; set; }
    }

    public class SimulationParam
    {
        public ProcessParam processParam { get; set; }
    }

    public class SimulationParamTKY : SimulationParam
    {
        public double alpha, theta;

        public SimulationParamTKY()
        {
            this.processParam = new ProcessParam();
        }

        public SimulationParamTKY(ProcessParam parameters)
            : this()
        {
            this.processParam = parameters;
        }
    }

    public class SimulationParamVSingle : SimulationParam
    {
        public double deta, eta;

        public SimulationParamVSingle()
        {
            this.processParam = new ProcessParam();
        }

        public SimulationParamVSingle(ProcessParam parameters)
            : this()
        {
            this.processParam = parameters;
        }
    }

Just scatching the surface of your code, but

By default, the first enum constant has a default value of 0 , so this code

       public enum GrooveType : int
            {
                TKY = 0,
                VSingle = 1,
            }

could be written as

       public enum GrooveType : int
            {
                TKY,
                VSingle
            }

Your ProcessParam was implemented twice. When things like this occur you should change  ProcessParam into an abstract class. This is called Dependency Inversion

  Dependency Inversion: In object-oriented programming, the dependency inversion principle refers to a specific form of decoupling software modules. When following this principle, the conventional dependency relationships established from high-level, policy-setting modules to low-level, dependency modules are reversed, thus rendering high-level modules independent of the low-level module implementation details. The principle states:1

  High-level modules should not depend on low-level modules. Both should depend on abstractions.
  Abstractions should not depend on details. Details should depend on abstractions. More details of this Dependency Inversion

This line of code should have been

public class ProcessParam
    {
        public ProcessParam() { }
        public double probeStartAngle { get; set; }
        public double probeStartDist { get; set; }
    }

should have been

abstract class ProcessParam   // Abstract class
{
    public abstract void AbstractMethod();   // Abstract method
    public abstract double probeStartAngle { get; set; }
    public abstract double probeStartDist { get; set; }
}

Dependency Injection : Inject dependency rather than creating an object in the constructor. This is good

public SimulationParamVSingle(ProcessParam parameters)
    : this()
{
    this.processParam = parameters;
}

This is bad

 public SimulationParamVSingle()
        {
            this.processParam = new ProcessParam();
        }


Simple Phase Locked Loop Here is a simple Phase Locked Loop, which is a circuit used in radio communications for synchronisation between transmitter and receiver.

The loop works by calculating the (phase) difference between the input signal, and a reference oscillator, and then adjusting the reference until the phase difference is zero. In this code, the adjustment is made by approximating a digital biquad filter's output - simply by multiplying by the Q factor of the filter.

My main concern with this code, is that it would require some re-writing if I wanted to extend it.

import numpy as np
import pdb

class SimPLL(object):
    def __init__(self, lf_bandwidth):
        self.phase_out = 0.0
        self.freq_out = 0.0
        self.vco = np.exp(1j*self.phase_out)
        self.phase_difference = 0.0
        self.bw = lf_bandwidth
        self.beta = np.sqrt(lf_bandwidth)

    def update_phase_estimate(self):
        self.vco = np.exp(1j*self.phase_out)

    def update_phase_difference(self, in_sig):
        self.phase_difference = np.angle(in_sig*np.conj(self.vco))

    def step(self, in_sig):
        # Takes an instantaneous sample of a signal and updates the PLL's inner state
        self.update_phase_difference(in_sig)
        self.freq_out += self.bw * self.phase_difference
        self.phase_out += self.beta * self.phase_difference + self.freq_out
        self.update_phase_estimate()

def main():
    import matplotlib.pyplot as plt
    pll = SimPLL(0.002)
    num_samples = 500
    phi = 3.0
    frequency_offset = -0.2
    ref = []
    out = []
    diff = []
    for i in range(0, num_samples - 1):
        in_sig = np.exp(1j*phi)
        phi += frequency_offset
        pll.step(in_sig)
        ref.append(in_sig)
        out.append(pll.vco)
        diff.append(pll.phase_difference)
    #plt.plot(ref)
    plt.plot(ref)
    plt.plot(out)
    plt.plot(diff)
    plt.show()

Here is the output.

I didn't find much to say about your coding style.

Maybe in_sig is not a perfect name: signal_in would be easier to read in my opinion (putting the _in at the end is more consistent with you other variable names)

  it would require some re-writing if I wanted to extend it.

What kind of extension ?

Using iterators to get the samples

For your input functions, you could use a generator instead:

def sinusoid(initial_phi, frequency_offset):
    """Generates a sinusoidal signal"""
    phi = initial_phi
    while True:
        yield np.exp(1j*phi)
        phi += frequency_offset

When initially called, this function will return an iterator: iter_sin = sinusoid(3.0, -0.2)

And add set_signal_in and signal_out methods in your SimPLL class:

def set_signal_in(self, signal_in):
    """Set a iterator as input signal"""
    self.signal_in = signal_in

def signal_out(self):
    """Generate the output steps"""
    for sample_in in self.signal_in:
        self.step(sample_in)
        yield self.vco

(maybe signal_out could generate some tuples with the sample_in and phase_difference, or even yield self)

Usage

You can then do a pll.set_signal_in(iter_sin).

If you have a list of data, you can then do pll.set_signal_in(list_of_values).

Plotting

For plotting a limited amount of points, you may use itertools.islice


Stock checking with coffeescipt I have such cases:

stock status can be in stock, undefined (there is no such field in the object) or any other text

stock status is true if it is 'in stock' or undefined
all others are false

so this is my test object

obj =
    one:
        stock: 'in stock'
    two:
        some: 'x'
    three:
        stock: 'not in stock blah blah any other text'

my method

stockCheck = (stock, msg) ->
    console.log {msg}
    return true if !stock or stock == 'in stock'

and some simply test

console.log stockCheck(obj.one.stock)?, 'should be in stock'
console.log stockCheck(obj.two.stock)?, 'should be in stock'
console.log stockCheck(obj.three.stock)?, 'should not be in stock'

is my code well for such cases?

There are issues with your test function, don't return true, just return the result of the test you're doing or your function won't return anything if the test is false:

stockCheck = (stock, msg) ->
    console.log {msg}
    return !stock or stock is 'in stock'

Also, why {msg}? Shouldn't it be just console.log msg?

Your console.log tests are erronous too, your function is supposed to do the test not you when you call it, remove the ? from these lines:

console.log stockCheck(obj.one.stock) # true
console.log stockCheck(obj.two.stock) # true
console.log stockCheck(obj.three.stock) # false


Split function using C and dynamic memory allocation I have programmed a function to split strings, and it gives the expected output.
I am looking forward to write better code and I was told here is a good place to start.
Here's my program:
#include <stdio.h>
#include <stdlib.h>
#include <string.h>

char **split(char *str) {
    int str_length = 0;
    int i = 0;
    int separator_count = 0;
    while (str[i] != '\0') {
        if (str[i] == ' ') {
            separator_count++;
        }
        i++;
    }

    char **word_array;

    word_array = malloc((separator_count + 2) * sizeof(char *));

    int word_len = 0;
    int separator_index = 0;
    int b = 0;                                     //to iterate over "str"

    for (int a = 0; a < (separator_count + 2); a++) {
        word_len = 0;
        while (str_length < strlen(str) + 1) {
            str_length++;

            if (str[b] == ' ' || str[b] == '\0') {
                separator_index = b;
                word_array[a] = malloc(word_len * sizeof(char));
                int word_depth = 0;
                for (int c = (separator_index - word_len); c < separator_index; c++) {
                    chrctr = str[c];
                    word_array[a][word_depth] = str[c];
                    word_depth++;
                }
                word_array[a][word_len] = '\0';         //terminate found and stored word with null charachter
                word_len = 0;                           //reset word length counter to 0
                b++;                                    //skip to next charachter
                break;                                  // break to go to next index of word_array
            }
            word_len++;
            b++;
        }
    }
    word_array[separator_count + 2] = NULL;
    return word_array;
}

int main() {
    char s[] = "A complete toolkit for building search into your product.";  //test string
    char **ss = split(s);
    for (int i = 0; i < sizeof(ss); i++) {        //here is the problem, and doing sizeof(ss)/sizeof(ss[0])=1 !!
        for (int j = 0; j < strlen (ss[i]); j++) {
            printf("%c", ss[i][j]);
        }
        printf ("\n");
    }
}

Please comment with any advice or remarks on how to make this better.

Design: Code should explain cases

Double space: "abc  xyz" --> Is that to be 2 or 3 tokens?
Leading space: " abc xyz" --> Is that to be 2 or 3 tokens?
Trailing space: "abc xyz " --> Is that to be 2 or 3 tokens?

Design : Generalization
split() only considers ' '.  Maybe all white-spaces, '\t', '\n', ...?
Or pass into the function  a list of separators: split(char *str, const char *separators)
Design: const
As data referenced by str does not change, consider char **split(const char *str) for greater applicability and self documentation.
Bug: Insufficient memory
// word_array[a] = malloc(word_len * sizeof(char));
word_array[a] = malloc((word_len + 1u) * sizeof(char));
...
word_array[a][word_len] = '\0';

Bug: Writing outside allocation
word_array = malloc((separator_count + 2) * sizeof(char *));
...
word_array[separator_count + 2] = NULL; //OOPS!

// I suspect OP wanted
word_array[separator_count + 1] = NULL;

Allocate to the size of the reference object, not the type
// word_array = malloc((separator_count + 2) * sizeof(char *));
word_array = malloc(sizeof *word_array * (separator_count + 2u));

// word_array[a] = malloc(word_len * sizeof(char));
word_array[a] = malloc(sizeof (word_array[a][0] * word_len);

It is easier to code right, review and maintain.
Leading with the widest types in the multiplication prevents overflow - though that is not so important in this case.
Robust code checks allocation success
ptr = malloc(sizeof *ptr * n);
if (ptr == NULL && n > 0) {
  HandleOutOfMemory(); // Some user code
}

Clean-up allocations
main() called split(s) which allocates, but failed to call a matching free().  Not much concern with main() as code ends anyways, but good practice.
Minor: int word_len insufficient for large strings
// int word_len
size_t word_len


Closure handling in javascript / jquery ajax I'm using this pattern (for want of a better word) repeatedly in my code to call a REST API in my javascript code. Some particulars of the code below.

I have a ConfigViewer javascript class that is responsible for creating, populating and handling events for DOM element.
This class constructor inits the DOM components, and then calls an REST API to get the data to populate these DOM components.
I need to handle the response to this API from my instance of ConfigViewer

My question is related to the way I have the getAdminDataSuccessHandler() method to return a function that is called when the REST API succeeds:  Is this the cleanest way to handle the response of the API call ?

function ConfigViewer() {
  this.createUIComponents();
  this.ajaxLoadData("/adminData", this.getAdminDataSuccessHandler());
}
ConfigViewer.prototype.getAdminDataSuccessHandler = function() {
  var self = this;
  return function(data) {
     // Handle successful data retrieval
     self.populateUICoponents(data);
  }
}
/**
 * Execute the API in ajaxy fashion.
 */
ConfigViewer.prototype.ajaxLoadData = function(url, onSuccessHandler) {
    $.ajax({
       url : url,
       success : function(jsonData, textStatus, XMLHttpRequest) {
           onSuccessHandler(jsonData);
       }
    });
}

It doesn't look like your class has any data, so I must question what's the point. Any instance of the class
would be equal to each other.

Your constructor is also doing a lot of real work, constructors should just initialize the object.

You can also do what you are doing without manual closure plumbing by using jQuery's proxy:

function ConfigViewer() {
    this.createUIComponents();
    this.ajaxLoadData("/adminData").then($.proxy(this.successHandler, this));
}

ConfigViewer.prototype.successHandler = function( data ) {
     this.populateUICoponents(data);
};

ConfigViewer.prototype.ajaxLoadData = function(url) {
    return $.ajax({
       url : url
    });
};


Password Strength Detector I'm just a beginner in Python. I wrote a simple code that detects the user's password strength, but I feel that it isn't very efficient. How can I improve it?

print('''# This is Password Strength Detector.\n
-----------------------------------------------------------------''')
password = input('Enter your password here : ')
print('\n')

from re import *

lower_match = compile(r'[a-z]').findall(password)  # Finding if the password contains lowecase letters.
upper_match = compile(r'[A-Z]').findall(password)  # Finding if the password contains uppercase letters.
symbol_match = compile(r'[|\"|\'|~|!|@|#|$|%|^|&|*|(|)|_|=|+|\||,|.|/|?|:|;|[|]|{\}|<|>]').findall(
    password)  # Finding if the password contains special characters.
number_match = compile(r'[0-9]').findall(password)  # Finding if the password contains numbers.

if len(password) < 8 or len(lower_match) == 0 or len(upper_match) == 0 or len(symbol_match) == 0 or len(
        number_match) == 0:
    print('Your password is weak ! ')

elif len(password) >= 8:
    print('Your password is strong ! ')

elif len(password) >= 16:
    print('Your password is very strong ! ')

The strength of a password has nothing to do with the presence of special characters! Obligatory xkcd reference:

correct horse battery staple is a very strong password, but you would classify it as weak, simply because it doesn't contain capital letters, symbols, or numbers. Rethink the idea.

Testing

You will never indicate that a password is very strong because of this ordering:

elif len(password) >= 8:
    print('Your password is strong ! ')
elif len(password) >= 16:
    print('Your password is very strong ! ')

If len(password) is, say, 20, both branches are true. So you want to makes ure that the most restrictive one goes first:

elif len(password) >= 16:
    print('Your password is very strong ! ')
else:
    print('Your password is strong ! ')

Regex

The point of compile is if you reuse the same regex many times, you can make it more efficient. You use each one exactly once, so it's a waste of processing time. What you want to instead use is re.search(), since you don't care about all the instances you just want to know if there is such a thing:

lower_match = re.search(r'[a-z]', password)
upper_match = re.search(r'[A-Z]', password)
...

Then check against None instead of checking len(). Also your symbol check is inefficient. You have a bunch of |s, when you could simply list them all:

symbol_match = re.search(r'[\"\'~!@#$%^&\\*\(\)_=\+\|,./\?:;\[\]\{\}<>]', password)

Importing

from X import *

is highly frowned upon. Just do import re and use re.X everywhere.