Memory leak in stack I'm implementing a stack by C++ and here I think it has memory leakage problem: in Stack::peek() and Stack::pop(), where I created heap space and returned pointer to function caller.

Users who call these two functions may perform deletion on returned pointer, which will recycle heap space once done. I am wondering what is a better approach to my current leakage prone approach.

Header file:

// this is the header file of stack data structure.

#ifndef MY_STACK_H
#define MY_STACK_H

class Node
{
private:
    int number;
    Node * next;

public:
    Node(){number = 0; next = NULL;}

    Node(int initialNumber, Node * initialNext = NULL)
    {
        number = initialNumber;
        next = initialNext;
    }

    // copy constructor
    Node(Node & copyFromNode)
    {
        this->number    =   copyFromNode.getNumber();
        this->next      =   copyFromNode.getNext();
    }

    // setters & getters
    int getNumber() {return number;}
    Node * getNext()    {return next;}
    void setNumber(int newNumber)   {number = newNumber;}
    void setNext(Node * newNext)    {next = newNext;}
};

class LinkedList
{
private:
    Node * head;

public:
    LinkedList(){head = NULL;}

    // very similar to Stack::push()
    void addFirst(Node *newNode)
    {
        newNode->setNext(head);
        head = newNode;
    }

    // somehow similar to Stack::pop()
    void deleteFirst()
    {
        if (head == NULL)
            return;
        else
        {
            Node * temp = head;
            head = head->getNext();
            delete temp;
        }
    }

    bool isEmpty()      {return (head == NULL);}
    Node * getHead()    {return head;}

    // there is no setter to head
    // since head should be maintained by addFirst() & deleteFirst() only.
};

class Stack
{
private:
    Node * top;
    LinkedList ll;
public:
    Stack(){top = NULL; }

    // push new node to the stack
    // new node will become the new top of stack
    void push(Node * newNode);

    // it checks if the stack is empty or not.
    bool isEmpty();

    // it copies the top node on stack,
    // without deleting it.
    Node * peek();

    // it copies the top node on stack,
    // and deletes the node from stack.
    Node * pop();
};

#endif

.cpp file:

// implementation for the stack

#include <iostream>
#include "stack.h"

using namespace std;

void Stack::push(Node * newNode)
{
    ll.addFirst(newNode);
    top = ll.getHead();
}

bool Stack::isEmpty()
{
    if(top == NULL)
        return true;
    else
        return false;
}

Node * Stack::peek()
{
    // make a copy, erase next link and return
    // copy is made by calling copy construtor
    Node * copy = new Node(*top);   // ?? asks for reference but pointer is given

    return copy;
}

Node * Stack::pop()
{
    if (isEmpty())
        return NULL;
    else
    {
        Node * copy = new Node(*top);
        ll.deleteFirst();
        top = ll.getHead();
        return copy;
    }
}

int main()
{
    Stack stack;

    Node * nd1 = new Node(100);
    Node * nd2 = new Node(200);

    // test push()
    stack.push(nd1);
    stack.push(nd2);

    // test isEmpty()
    cout<<"Test: isEmpty(): "<< stack.isEmpty()<<endl;

    // test peek()
    cout<<"Test: peek(): "<<stack.peek()->getNumber()<<endl;

    // test pop()
    cout<<"Test: pop()"<<stack.pop()->getNumber()<<endl;
    cout<<"Test: pop()"<<stack.pop()->getNumber()<<endl;

    cout<<"Test: isEmpty(): "<<stack.isEmpty()<<endl;

    return 0;
}

Don't use NULL unless you have to

Some IDEs will refuse to compile your code unless you include <cstdlib> so that NULL is available. If you compile using a C++ compiler using latest standards, you could use nullptr instead, since it is a language construct in those versions and does not require including anything.

Minor improvements

//bool Stack::isEmpty()
//{
//    if(top == NULL)
//        return true;
//    else
//        return false;
//}

bool Stack::isEmpty()
{
    return top == nullptr;
}

and

//Node * Stack::pop()
//{
//
//    if (isEmpty())
//        return NULL;
//    else
//    {
//        Node * copy = new Node(*top);
//        ll.deleteFirst();
//        top = ll.getHead();
//        return copy;
//    }
//}

Node * Stack::pop()
{
    if (isEmpty())
    {
        return nullptr;
    }

    Node * copy = new Node(*top);
    ll.deleteFirst();
    top = ll.getHead();
    return copy;
}

Alternative implementation

It's not difficult to devise a generic stack:

my_stack.h

#ifndef MY_STACK_H
#define MY_STACK_H

#include <iostream>
#include <stdexcept>
#include <vector>

template<typename T>
class MyStack {

    std::vector<T> storage;
    void check_not_empty()
    {
        if (storage.empty())
        {
            throw std::runtime_error{"Peeking from an empty stack."};
        }
    }

public:
    void push(const T& element)
    {
        storage.push_back(element);
    }

    void pop()
    {
        check_not_empty();
        storage.pop_back();
    }

    const T& peek() const
    {
        check_not_empty();
        return storage[storage.size() - 1];
    }

    size_t size()
    {
        return storage.size();
    }

    bool is_empty()
    {
        return storage.empty();
    }

    friend std::ostream& operator<<(std::ostream& os, const MyStack<T>& stack)
    {
        os << "[";
        std::string separator = "";

        for (size_t i = 0; i != stack.storage.size(); ++i)
        {
            os << separator << stack.storage[i];
            separator = ", ";
        }

        return os << "]";
    }
};

#endif

main.cpp

#include "my_stack.h"
#include <iostream>

using namespace std;

int main()
{
    MyStack<int> stack;

    cout << stack << endl;

    for (int i = 1; i <= 5; ++i)
    {
        stack.push(i);
        cout << stack << endl;
    }

    while (!stack.is_empty())
    {
        stack.pop();
        cout << stack << endl;
    }
}


Get all node descendants in a tree I have a CSV holding a "flat" table of a tree's edges (NOT binary, but a node cannot have two parents), ~1M edges:

node_id parent_id
1       0
2       1
3       1
4       2
...

The nodes are sorted in a way that a parent_id must always come before any of its children, so a parent_id will always be lower than node_id.

I wish, for each node_id, to get the set of all ancestor nodes (including itself, propagated until root which is node 0 here), and a set of all descendant nodes (including itself, propagated until leaves), and speed is crucial.

Currently what I do at high level:

Read the CSV in pandas, call it nodes_df
Iterate once through nodes_df to get node_ancestors, a {node_id: set(ancestors)} dict adding for each node's ancestors itself and its parent's ancestors (which I know I have seen all by then)
Iterate through nodes_df again in reverse order to get node_descendants, a {node_id: set(ancestors)} dict adding for each node's descendants itself and its child's descendants (which I know I have seen all by then)

import pandas as pd
from collections import defaultdict

# phase 1
nodes_df = pd.read_csv('input.csv')

# phase 2
node_ancestors = defaultdict(set)
node_ancestors[0] = set([0])
for id, ndata in nodes_df1.iterrows():
    node_ancestors[ndata['node_id']].add(ndata['node_id'])
    node_ancestors[ndata['node_id']].update(node_ancestors[ndata['parent_id']])

# phase 3
node_descendants = defaultdict(set)
node_descendants[0] = set([0])
for id, ndata in nodes_df1[::-1].iterrows():
    node_descendants[ndata['node_id']].add(ndata['node_id'])
    node_descendants[ndata['parent_id']].\
      update(node_descendants[ndata['node_id']])

So, this takes dozens of seconds on my laptop, which is ages for my application. How do I improve?

Plausible directions:

Can I use pandas better? Can I get node_ancestors and/or node_descendants by some clever join which is out of my league?
Can I use a python graph library like Networkx or igraph (which in my experience is faster on large graphs)? E.g. in both libraries I have a get_all_shortest_paths methods, which returns something like a {node_id: dist} dictionary, from which I could select the keys, but... I need this for every node, so again a long long loop
Parallelizing - no idea how to do this

id

you shadow the builtin id with this name as variable

itertuples

a way to improve performance is using itertuples to iterate over the DataFrame: for _, node, parent in df.itertuples():

iterations

You can do this in 1 iteration over the input with a nested loop over the ancestors:

node_ancestors = defaultdict(set)
node_ancestors[0] = set([0])
node_descendants = defaultdict(set)
node_descendants[0] = set([0])
for _, node, parent in df.itertuples():
    node_ancestors[node].add(node)
    node_ancestors[node].update(node_ancestors[parent])

    for ancestor in node_ancestors[node]:
        node_descendants[ancestor].add(node)

Depending on how nested the tree is, this will be faster or slower than iterating over the whole input twice. You'll need to test it on your dataset.

global vs local

another speedup might be achieved by doing this in a function instead of the global namespace (explanation)

def parse_tree(df):
    node_ancestors = defaultdict(set)
    node_ancestors[0] = set([0])
    node_descendants = defaultdict(set)
    node_descendants[0] = set([0])
    for _, node, parent in df.itertuples():
        node_ancestors[node].add(node)
        node_ancestors[node].update(node_ancestors[parent])

        for ancestor in node_ancestors[node]:
            node_descendants[ancestor].add(node)

    return node_ancestors, node_descendants


ConsoleWorker update console timer with how long application has been open earlier I coded a quick console worker that runs a timer every 1 second and updates the consoles timer with how long the app has been open, how can I improve this?

using Faze.Other.App;
using log4net;
using System;
using System.Collections.Generic;
using System.Diagnostics;
using System.Linq;
using System.Text;
using System.Threading;
using System.Threading.Tasks;

namespace Faze.Other.Util.Console
{
    class ConsoleWorker : IDisposable
    {
        /// <summary>
        /// Holds an instance of Timer.
        /// </summary>
        private Timer consoleWorkerTimer;

        /// <summary>
        /// Interval of the ConsoleWorker timer.
        /// </summary>
        private int consoleWorkerInterval = 1000;

        /// <summary>
        /// Holds an instance of ILog.
        /// </summary>
        private ILog classLogger;
        /// <summary>
        /// Initializes a new instance of ConsoleWorker.
        /// </summary>
        public ConsoleWorker()
        {
            if (StaticSettings.includeSecondsInUptimeString && consoleWorkerInterval < 1000)
                consoleWorkerInterval = 1000;

            classLogger = LogManager.GetLogger(typeof(ConsoleWorker));
            consoleWorkerTimer = new Timer(new TimerCallback(timerElapsed), null, TimeSpan.FromMilliseconds(consoleWorkerInterval), TimeSpan.FromMilliseconds(consoleWorkerInterval));
        }

        /// <summary>
        /// Starts the worker (timer) process.
        /// </summary>
        public void startWorker()
        {
            // I'll leave this for now..
        }

        /// <summary>
        /// Handles the timers callback once elapsed.
        /// </summary>
        /// <param name="timerObject">ConsoleWorker timer's object.</param>
        private void timerElapsed(object timerObject)
        {
            if (Faze.getServer().getServerInformation().hasStarted())
            {
                if (StaticSettings.advancedLoggingWhenDebugging && Debugger.IsAttached)
                    classLogger.Info("ConsoleWorker has elapsed -> " + DateTime.Now.ToLongTimeString());

                TimeSpan serverUptime = DateTime.Now - Faze.getServer().getServerInformation().getStartedTime();

                string consoleTitleString = string.Empty;

                if (StaticSettings.includeSecondsInUptimeString)
                {
                    consoleTitleString = Faze.getServer().getServerInformation().getProjectName() + " / " + serverUptime.Days + " day" + (serverUptime.Days != 1 ? "s" : "")
                    + ", " + serverUptime.Hours + " hour" + (serverUptime.Hours != 1 ? "s" : "") + ", " +
                    serverUptime.Minutes + " minute" + (serverUptime.Minutes != 1 ? "s" : "") + " and " + serverUptime.Seconds + " second" + (serverUptime.Seconds != 1 ? "s" : "");
                }
                else
                {
                    consoleTitleString = Faze.getServer().getServerInformation().getProjectName() + " / " + serverUptime.Days + " day" + (serverUptime.Days != 1 ? "s" : "")
                    + ", " + serverUptime.Hours + " hour" + (serverUptime.Hours != 1 ? "s" : "") + " and " +
                    serverUptime.Minutes + " minute" + (serverUptime.Minutes != 1 ? "s" : "");
                }

                System.Console.Title = consoleTitleString;
            }
        }

        public void Dispose()
        {
            consoleWorkerTimer.Dispose();
        }
    }
}

I think the first thing that you should do is create one more variable for Faze.getServer().getServerInformation().getProjectName() don't type that twice, and please don't make me read it twice.

I would also use String Interpolation (if you can) it makes this look a little cleaner.

You should pull out the stuff that is the same from the if/else statement and then use a single if statement if you want to add seconds onto the console title string, and then you can create the variable in one declaration minimizing the lines of code.

Here is what I have:

if (Faze.getServer().getServerInformation().hasStarted())
{
    if (StaticSettings.advancedLoggingWhenDebugging && Debugger.IsAttached)
        classLogger.Info("ConsoleWorker has elapsed -> " + DateTime.Now.ToLongTimeString());

    TimeSpan serverUptime = DateTime.Now - Faze.getServer().getServerInformation().getStartedTime();

    var projectName = Faze.getServer().getServerInformation().getProjectName();

    var consoleTitleString = $"{projectName} / {serverUptime.Days} " +
                         $"day{(serverUptime.Days != 1 ? "s" : "")}, {serverUptime.Hours} " +
                         $"hour{(serverUptime.Hours != 1 ? "s" : "")}, {serverUptime.Minutes} " +
                         $"minute{(serverUptime.Minutes != 1 ? "s" : "")}";

    if (StaticSettings.includeSecondsInUptimeString)
    {
        consoleTitleString += $" and {serverUptime.Seconds} second{(serverUptime.Seconds != 1 ? "s" : "")}";
    }

    System.Console.Title = consoleTitleString;
}

Another change that I might attempt would be to minimize some typing by creating an object from Faze.getServer().getServerInformation() so that I don't have to type that 3 times in my code,  it might look something like this.

var serverInfo = Faze.getServer().getServerInformation();
if (serverInfo.hasStarted())
{
    if (StaticSettings.advancedLoggingWhenDebugging && Debugger.IsAttached)
        classLogger.Info("ConsoleWorker has elapsed -> " + DateTime.Now.ToLongTimeString());

    TimeSpan serverUptime = DateTime.Now - serverInfo.getStartedTime();

    var projectName = serverInfo.getProjectName();

    var consoleTitleString = $"{projectName} / {serverUptime.Days} " +
                         $"day{(serverUptime.Days != 1 ? "s" : "")}, {serverUptime.Hours} " +
                         $"hour{(serverUptime.Hours != 1 ? "s" : "")}, {serverUptime.Minutes} " +
                         $"minute{(serverUptime.Minutes != 1 ? "s" : "")}";

    if (StaticSettings.includeSecondsInUptimeString)
    {
        consoleTitleString += $" and {serverUptime.Seconds} second{(serverUptime.Seconds != 1 ? "s" : "")}";
    }

    System.Console.Title = consoleTitleString;
}

I don't see anything that would change the private consoleWorkerInterval to anything other than a value of 1000 so I really don't see a need for this

if (StaticSettings.includeSecondsInUptimeString && consoleWorkerInterval < 1000)
            consoleWorkerInterval = 1000;

Resharper also noted that you don't need the Redundant explicit delegate creation on this line of code

consoleWorkerTimer = new Timer(new TimerCallback(timerElapsed), null, TimeSpan.FromMilliseconds(consoleWorkerInterval), TimeSpan.FromMilliseconds(consoleWorkerInterval));

you don't need to create a new TimerCallback because you are creating this Timer and giving it a method to call when the time passes, so this line of code can be shorter as well

consoleWorkerTimer = new Timer(timerElapsed, null, TimeSpan.FromMilliseconds(consoleWorkerInterval), TimeSpan.FromMilliseconds(consoleWorkerInterval));


BFS and DFS in Scala I would love it if someone could give me some suggestions for these 2 graph search functions. I am new to scala and would love to get some insight into making these more idiomatic.

  type Vertex=Int
  type Graph=Map[Vertex,List[Vertex]]
  val g: Graph=Map(1 -> List(2,4), 2-> List(1,3), 3-> List(2,4), 4-> List(1,3))
 //example graph meant to represent
 //  1---2
 //  |   |
 //  4---3

//I want this to return results in the different layers that it finds them (hence the list of list of vertex)
def BFS(start: Vertex, g: Graph): List[List[Vertex]]={
  val visited=List(start)
  val result=List(List(start))

  def BFS0(elems: List[Vertex],result: List[List[Vertex]], visited: List[Vertex]): List[List[Vertex]]={
    val newNeighbors=elems.flatMap(g(_)).filterNot(visited.contains).distinct
    if(newNeighbors.isEmpty) result
    else BFS0(newNeighbors, newNeighbors :: result, visited ++ newNeighbors)
    }

  BFS0(List(start),result,visited).reverse
   }

//I would really appreciate some input on DFS, I have the feeling there is a way to do this sans var.

 def DFS(start: Vertex, g: Graph): List[Vertex]={
    var visited=List(start)
    var result=List(start)

  def DFS0(start: Vertex): Unit={
     for(n<-g(start); if !visited.contains(n)){
       visited=n :: visited
       result=n :: result
       DFS0(n)
  }}
   DFS0(start)
   result.reverse
}

//some examples
scala> BFS(1,g)
res84: List[List[Vertex]] = List(List(1), List(2, 4), List(3))
scala> BFS(2,g)
res85: List[List[Vertex]] = List(List(2), List(1, 3), List(4))
scala> DFS(1,g)
res86: List[Vertex] = List(1, 2, 3, 4)
scala> DFS(3,g)
res87: List[Vertex] = List(3, 2, 1, 4)

OK, so I'm going to start with your DFS method.  You're right - you should be able to do it without those vars in the outer function.  You should be able to work out why - after all, you have vals in the outer layer of your BFS method.  Why?  Because your BFS uses a recursive helper function, so the vals are only used once (and could be discarded).

So your DFS function should really use recursion, but I suspect you may have rejected recursion because you couldn't see how visited would be properly preserved as a recursive function popped back and forth.  The answer is foldLeft.

def DFS(start: Vertex, g: Graph): List[Vertex] = {

  def DFS0(v: Vertex, visited: List[Vertex]): List[Vertex] = {
    if (visited.contains(v))
      visited
    else {
      val neighbours:List[Vertex] = g(v) filterNot visited.contains
      neighbours.foldLeft(v :: visited)((b,a) => DFS0(a,b))
    }
  }
  DFS0(start,List()).reverse
}

I don't have space here to explain foldLeft, if you've never encountered it - maybe Matt Malone's blog post will help.  You can rewrite almost anything with foldLeft, although it isn't always a good idea.  Definitely the right thing to do here, though.  Notice that I completely dropped your result var since visited is the result, the way you are doing this.

My version of your DFS method is entirely functional, which is how Scala really wants to be used.  Note also the lack of braces and brackets in

val neighbours:List[Vertex] = g(v) filterNot visited.contains

It can be written

val neighbours:List[Vertex] = g(v).filterNot(visited.contains)

but the Scala style is to omit the brackets and braces except where essential.

Your BFS method is similarly over-populated.  I've slimmed it down a little without altering the basic way it works:

def BFS(start: Vertex, g: Graph): List[List[Vertex]] = {

  def BFS0(elems: List[Vertex],visited: List[List[Vertex]]): List[List[Vertex]] = {
    val newNeighbors = elems.flatMap(g(_)).filterNot(visited.flatten.contains).distinct
    if (newNeighbors.isEmpty)
      visited
    else
      BFS0(newNeighbors, newNeighbors :: visited)
  }

  BFS0(List(start),List(List(start))).reverse
}

It still gives the same results.

The other big point to make is that while Scala is a functional language it is also an Object Oriented language.  Those DFS and BFS methods should belong to a graph object, preferably at least derived from a generic class.  Something like this:

class Graph[T] {
  type Vertex = T
  type GraphMap = Map[Vertex,List[Vertex]]
  var g:GraphMap = Map()

  def BFS(start: Vertex): List[List[Vertex]] = {

    def BFS0(elems: List[Vertex],visited: List[List[Vertex]]): List[List[Vertex]] = {
      val newNeighbors = elems.flatMap(g(_)).filterNot(visited.flatten.contains).distinct
      if (newNeighbors.isEmpty)
        visited
      else
        BFS0(newNeighbors, newNeighbors :: visited)
    }

    BFS0(List(start),List(List(start))).reverse
  }

  def DFS(start: Vertex): List[Vertex] = {

    def DFS0(v: Vertex, visited: List[Vertex]): List[Vertex] = {
      if (visited.contains(v))
        visited
      else {
        val neighbours:List[Vertex] = g(v) filterNot visited.contains
        neighbours.foldLeft(v :: visited)((b,a) => DFS0(a,b))
      }
    }
    DFS0(start,List()).reverse
  }
}

And then you could do this:

scala> var intGraph = new Graph[Int]
scala> intGraph.g = Map(1 -> List(2,4), 2-> List(1,3), 3-> List(2,4), 4-> List(1,3))
scala> intGraph.BFS(1)
res2: List[List[Int]] = List(List(1), List(2, 4), List(3))
scala> intGraph.BFS(2)
res3: List[List[Int]] = List(List(2), List(1, 3), List(4))
scala> intGraph.DFS(3)
res4: List[Int] = List(3, 2, 1, 4)

or this:

scala> var sGraph = new Graph[String]
scala> sGraph.g = Map("Apple" -> List ("Banana","Pear","Grape"), "Banana" -> List("Apple","Plum"), "Pear" -> List("Apple","Plum"), "Grape" -> List("Apple","Plum"), "Plum" -> List ("Banana","Pear","Grape"))
scala> sGraph.BFS("Apple")
res6: List[List[java.lang.String]] = List(List(Apple), List(Banana, Pear, Grape), List(Plum))


Extendable format provider I needed a better string formatting options like ToUpper and ToLower but I also wanted to be able to use the default formattings so I created a custom formatter. It's main purpose is to support a template engine where the user should be able to use different formattings like upper or lower case. I still however couldn't figure out how to be make the date time upper case.

public class ExtendableFormatter : IFormatProvider, ICustomFormatter
{
    public IDictionary<Type, IFormatProvider> FormatProviders { get; set; }

    public ExtendableFormatter()
    {
        FormatProviders = new Dictionary<Type, IFormatProvider>
        {
            { typeof(Byte), CultureInfo.InvariantCulture.NumberFormat },
            { typeof(SByte), CultureInfo.InvariantCulture.NumberFormat },
            { typeof(Int16), CultureInfo.InvariantCulture.NumberFormat },
            { typeof(Int32), CultureInfo.InvariantCulture.NumberFormat },
            { typeof(Int64), CultureInfo.InvariantCulture.NumberFormat },
            { typeof(UInt16), CultureInfo.InvariantCulture.NumberFormat },
            { typeof(UInt32), CultureInfo.InvariantCulture.NumberFormat },
            { typeof(UInt64), CultureInfo.InvariantCulture.NumberFormat },
            { typeof(Single), CultureInfo.InvariantCulture.NumberFormat },
            { typeof(Double), CultureInfo.InvariantCulture.NumberFormat },
            { typeof(Decimal), CultureInfo.InvariantCulture.NumberFormat },
            { typeof(DateTime), CultureInfo.InvariantCulture.DateTimeFormat },
            { typeof(String), new StringCaseFormatter() }
        };
    }

    public object GetFormat(Type formatType)
    {
        return formatType == typeof(ICustomFormatter) ? this : null;
    }

    public string Format(string format, object arg, IFormatProvider formatProvider)
    {
        var fp = (IFormatProvider)null;
        if (FormatProviders.TryGetValue(arg.GetType(), out fp))
        {
            formatProvider = fp;
        }

        format = string.IsNullOrEmpty(format) ? string.Empty : ":" + format;
        var result = string.Format(formatProvider, "{0" + format + "}", arg);
        return result;
    }

}

public class StringCaseFormatter : IFormatProvider, ICustomFormatter
{
    public object GetFormat(Type formatType)
    {
        return formatType == typeof(ICustomFormatter) ? this : null;
    }

    public string Format(string format, object arg, IFormatProvider formatProvider)
    {
        switch (format)
        {
            case "U": return arg.ToString().ToUpper();
            case "L": return arg.ToString().ToLower();
            default: return arg.ToString();
        }
    }
}

Example:

string.Format(
    new ExtendableFormatter(),
    "foo {0:U} baz {1,-5:f1} {2:ddMMMyy}",
    "qux", 1.234f, DateTime.Now).Dump();

Result:

  foo QUX baz 1.2   05Jun16

public IDictionary<Type, IFormatProvider> FormatProviders { get; set; }

This is bad. Having a public setter for the dictionary will let a user of that code pass in null resulting in the Format() method throwing a NullReferenceException.

You don't check the passed in parameters of the Format() method if they are valid. By omiting the validation of the arguments you are exposing implementation details for the case they aren't valid. E.g if the passed object arg is null then the call to arg.GetType() will throw and exposes that you are requesting the type of that object.

You only want to show the user that the passed in arguments are invalid, but you shouldn't let the code do it for you.


Validation using preg_match functions I need some improvements on my code. I did some validation by preg_match and functions.  I need some special characters restrictions every field.

<?php
$error='';
if(isset($_POST['submit']))
{
if(nameField($_POST['name'])=='')
{
    $error="Invalid Name";
}

if(numberField($_POST['phone'])=='')
{
    $error.="<br>Invalid Number";
}
if(nameField($_POST['address'])=='')
{
    $error.="Invalid Address";
}
if($error)
{
    echo $error;
}
else
{
    echo "Form submitted successfully";
}
}

function nameField($val)
{
if(preg_match("/([%\$#\*\'\"\()]+)/", $val)) //Not allowed some of the special characters
{
return false;
}
else
{
return true;
}
}
function numberField($val)
{
if(preg_match("/^[0-9]*$/", $val))
{
return true;
}
else
{
return false;
}
}
?>

<html>
<body>
<form method="post">
<input type="text" name="name"/>
<input type="text" name="phone">
<input type="text" name="address">
<input type="text" name="city">
<input type="submit" name="submit" value="Submit"/>
</form>
</body>
</html>

I'll try to keep things brief, for a change:

nameField seems reasonable enough. Although I do have some suggestions (see below).
The numberField function is another matter. If you want to check for a valid phone number, there are a couple of things to consider:

User tend to enter a phone number in a human-readable format: 213-1234-4454 or 123/1234.4567
International phone numbers can start with Either a double 0, or a plus sign. both of which are valid: (0012) 123/1234-5678 is a possible value you'd have to process, just as +12 123/1234-5678

Bottom line: First trim the non-numeric chars, then check if the input is valid, I'd say:

function sanitizeAndValidate($phoneNr)
{
    //replace all non-numeric chars, after replacing a single + char with 00
    $phone = preg_replace('/[^\d]/', '', str_replace('+', '00', $phone, 1));
    $len = strlen($phone);
    //perhaps changing these constants to argumenst might be preferable
    if (is_numeric($phone) && $len >= MIN_PHONE_LEN && $len <= MAX_PHONE_LEN)
        throw new \InvalidArgumentException(
            sprintf(
                '%s is an invalid phone number (%s after sanitation)',
                $phoneNr,
                $phone
            )
        );
    return $phone;

}

Pas the raw phone number to this function, if the phone number is valid, you'll get a santized version back. If the phone number is invalid, catch the exception and present the user with an error message.

The nameField function is OK, but it's a bit redundant. If you only use it in one place, I'd avoid the overhead of a function-call, and simply inline the check, by replacing if (nameField($_POST['name'])) with this:

if (preg_match('/([%_\$#*\'"\()]+)/', $_POST['name']))

You might have noticed that I don't escape all of the chars, simply because there's no reason to escape them all. I've also added _ to the restricted chars. Simply because everybody knows that % is a MySQL wildcard, but not everybody seems to be aware that _ is, too.

Regular expressions can impact preformance, which is, I suspect, why you're asking for ways to improve your code. An alternative to preg_match here could be to have an array of prohibited chars, and use str_replace:

$notAllowed = array('%', '_', '$', '#', ...);//and so on
if (str_replace($notAllowed, '', $_POST['name']) !== $_POST['name'])
{//values are not the same, a forbidden char was replaced
    //name contains prohibited char
}

I'm not so sure if that would be faster then a preg_match, though, and besides: premature optimization is the root of all evil.

Last comment:
Please, try to adhere to the coding standards as much as possible. You seem to have some formatting issues


"Skewed" average in Lisp I set myself the task to calculate the average of a list, but with two conditions:

negative numbers are ignored
numbers greater than 100 are counted as if they were 100

So the "skewed" average of the list '(1 -3 42 297 14) should be (1 + 42 + 100 + 14) / 4 (157/4).

I wrote 2 functions that do that. Please review and comment.

(defun skewed-average1 (list)
    "calculate average by summing and dividing"
    (let ((sum 0) (n 0))
        (dolist (x list)
            (if (>= x 0)
                (progn
                    (if (> x 100) (setf x 100))
                    (setf sum (+ x sum))
                    (setf n (+ 1 n)))))
         (/ sum n)))

(defun skewed-average2 (list)
    "calculate average by building 'fixed' list"
    (let (newlist)
        (dolist (x list)
            (if (>= x 0)
                (progn
                    (if (> x 100) (setf x 100))
                    (setf newlist (cons x newlist)))))
        (/ (apply #'+ newlist) (length newlist))))

(let ((numbers '(1 -3 42 297 14)))
    (print (skewed-average1 numbers))
    (print (skewed-average2 numbers)))

Also, how should errors be treated? Imagine passing an empty list to the functions; or a list with all negative numbers.

(defun skewed-average1 (list)
  "calculate average by summing and dividing"
  (let ((sum 0) (n 0))
    (dolist (x list)
      (if (>= x 0)
          (progn
            (if (> x 100) (setf x 100))
            (setf sum (+ x sum))
            (setf n (+ 1 n)))))
    (/ sum n)))

IF ... PROGN  is WHEN.

>= 0 is plusp.

(setf sum (+ ... is INCF.

(defun skewed-average1 (list &aux (sum 0) (n 0))
  "calculate average by summing and dividing"
  (dolist (x list (when (plusp n)
                     (/ sum n)))
    (when (plusp x)
      (incf sum (min x 100))
      (incf n))))

Next function:

(setf newlist (cons x newlist)) is (push x newlist).

Don't use APPLY, use REDUCE. APPLY has a list length limit.

Alternative implementations:

(defun skewed-average3 (list)
  (loop for x in list
        when (plusp x)
        sum (min x 100) into sum1
        and count t into count1
        finally (return (when (plusp count1)
                          (/ sum1 count1)))))

(defun skewed-average4 (list)
  (let ((new-list (remove-if #'minusp
                             (substitute-if 100
                                            (lambda (item) (> item 100))
                                            list))))
    (when new-list
      (/ (reduce #'+ new-list)
         (length new-list)))))


Web Scraper in Python So, this is my first web scraper (or part of it at least) and looking for things that I may have done wrong, or things that could be improved so I can learn from my mistakes.

I made a few short function that can take a user and search tpb to get the maximum number of pages of content that that user has by requesting the url, parsing the html for the links div, and then crawling through them to see if each page is valid or not (since some linked pages actually have no content).

I know that I haven't covered all cases such as invalid urls, non-existent users etc, just trying to find what I've done wrong so far in actually parsing existent content before I go further.

There's two main things I'm concerned about:

First, I have filter/map/lambda combos all over the place here. These are generally slow from what I remember about their efficiency, and they seemed to be a fairly easy and concise way to get the filtering I needed (although not the prettiest). So, is this acceptable and/or is there a better way with bs4 or another alternative?

Second, since beautifulsoup is recursive in finding nested tags anyways does calling my get_max_pages function recursively really matter here?

import requests, urllib, re
from bs4 import BeautifulSoup

BASE_USER = "https://thepiratebay.se/user/"

def request_html(url):
    hdr = {'User-Agent': 'Mozilla/5.0'}
    request = urllib.request.Request(url, headers=hdr)
    html = urllib.request.urlopen(request)
    return html

def get_url(*args):
    for arg in args:
        url = BASE_USER + "%s/" % arg
    return url

def check_page_content(url):
    bs = BeautifulSoup(request_html(url), "html.parser")
    rows = bs.findAll("tr")
    rows = list(filter(None, map(lambda row: row if row.findChildren('div', {'class': 'detName'}) else None, rows)))
    return True if rows else False

def get_max_pages(user, url=None, placeholder=0, links = list(), valid=True):
    if url is None:
        url = get_url(user, str(placeholder), 3)
    if valid:
        td = BeautifulSoup(request_html(url), "html.parser").find('td', {'colspan': 9, 'style':'text-align:center;'})
        pg_nums = td.findAll('a', {'href': re.compile("/user/%s/\d{,3}/\d{,2}" % user)})
        pages = list(filter(None, map(lambda a: int(a.text) - 1 if a.text else None, pg_nums)))
        if links: #Unnecessary to filter this at start as placeholder = 0
            pages = list(filter(None, map(lambda x: x if int(x) > placeholder else None, pages)))
        if pages:
            i = 0
            while valid and i < len(pages):
                element = pages[i]
                valid_page = check_page_content(get_url(user, element, 3))
                if valid_page:
                    links.append(element)
                else:
                    valid = False
                i += 1
            return get_max_pages(user, get_url(user, len(links), 3), len(links), links, valid)
        else:
            return links
    else:
        return links

Also, if anyone wouldn't mind would it be viable to split this into threads for say the content validation? I don't see how that would really improve anything all that much with python's GIL, but curious nonetheless.

To find ~35 pages of valid content it takes about 1.5 mins.

Your get_url function is confusing. It looks like you keep assigning new values to url and ignoring all the previous values. This is what happens when I run it:

>>> get_url("Hello", "World")
'https://thepiratebay.se/user/World/'

Surely this is either a bug or redundant behaviour, since the only argument that matters is the last one? It seems like instead you should be using str.join, which will concatenate all the arguments together with a string separator, so for example:

def get_url(*args):
    return BASE_USER + "/".join(args)

>>> get_url("Hello", "World")
'https://thepiratebay.se/user/Hello/World'
>>> get_url("ban", "an", "a")
'https://thepiratebay.se/user/ban/an/a'

Though as @Mathias Ettinger noted, you should use map(str, args) to ensure that all the arguments are converted to strings as join will raise errors if any of the arguments aren't strings.

check_page_content is hard to understand at first, a lot happens in one line so you should try spread it out to multiple lines. It would be easier to build rows with a list comprehension that filters in advance, rather than gathering up a lot of rows just to later remove them:

def check_page_content(url):
    bs = BeautifulSoup(request_html(url), "html.parser")
    rows = [row for row in bs.findAll("tr")
            if row.findChildren('div', {'class': 'detName'})]

But then I see that you're just returning the boolean value of the list! That means you can break as soon as you found that there's any row, no need to store the list at all. Luckily there's the any function that will do this for you. It supports short circuiting, which means that it will return as soon as it finds a condition that evaluates as True:

def check_page_content(url):
    bs = BeautifulSoup(request_html(url), "html.parser")
    return any(row.findChildren('div', {'class': 'detName'})
               for row in bs.findAll("tr"))

any will apply truthiness to all the values in bs.findAll so if any of them have truthy results then your function will immediately return. Even if you have to check every single row, this is faster than building a full list, mapping and filtering it.

In get_max_pages you have the default value links=list(). I'm not sure if you know about the mutable default and thought this would avoid it but it wont. links will be created once as an empty list. Every time you call the function the same list exists so the same list will be appended to, which is not what you need. Here's a simple example:

>>> def a(b=list()):
    b.append("another")
    return b

>>> a()
['another']
>>> a()
['another', 'another']
>>> a()
['another', 'another', 'another']

Instead you need to use links=None and then use if links is None: links = [] so that a new list is created within each function call.

You have most of your code nested inside if valid, but if you flipped it around you could save nesting by doing this:

def get_max_pages(user, url=None, placeholder=0, links = list(), valid=True):
    if url is None:
        url = get_url(user, str(placeholder), 3)
    if not valid:
        return links

    td = BeautifulSoup(request_html(url), "html.parser").find('td', {'colspan': 9, 'style':'text-align:center;'})

Less nesting generally makes it easier to read code like this.

You have another map filter, I'd recommend doing a similar list comprehension, though it's a little harder to follow:

    regex = re.compile("/user/%s/\d{,3}/\d{,2}" % user)
    pages = [int(a.text) - 1
             for a in td.findAll('a', {'href': regex}) if a.text]

What's happening here is that I'm iterating through td.findAll, and if a.text is True then I'm storing int(a.text) - 1 in pages. This does what your next line does without needing multiple calls.

However, you then may need to further filter pages based on a condition. You could still do this in a comprehension though, just run a list comprehension over pages itself:

if links:
    pages = [x for x in pages if x > placeholder]

Note I removed the int call because you already store them as integers to it's not necessary.

Lastly, your while loop here is strange. It would be easier to just do for element in pages and then break from the loop instead of setting valid = False. This gives you a much simpler loop with less lines:

        for element in pages:
            valid_page = check_page_content(get_url(user, element, 3))
            if valid_page:
                links.append(element)
            else:
                break


Searching a CSV with PHP Given a string, I've done the below to search a CSV for a keyword and display the row if it's found

<?php
$ch = fopen("test.csv", "r");
$header_row = fgetcsv($ch);

/* This will loop through all the rows until it reaches the end */
while(($row = fgetcsv($ch)) !== FALSE) {
    $cols = array($row[0], $row[1], $row[2], $row[3]);
    if (in_array("text", $cols)) {
        echo '<div>'.$row[0].' | '.$row[1].' | '.$row[2].' | '.$row[3].' </div>';
    }
?>

I'm worried it's not best optimised for my needs, especially as the search is not regex'd and its assuming a X number of columns

Useless transformation

Since fgetcsv() returns an indexed array, the statement

$cols = array($row[0], $row[1], $row[2], $row[3]);

is totally useless: it results in $cols which is exactly the same as $row.

Using any number of columns

BTW, the above already answers part of your concern, regarding:

  its assuming a X number of columns

Directly working with $rows you keep all original columns.

The last part is that this statement:

echo '<div>'.$row[0].' | '.$row[1].' | '.$row[2].' | '.$row[3].' </div>';

can be replaced by:

echo '<div>' . implode(' | ', $row) . ' </div>';

Then, again, you'll get the exact content, whatever is the number of columns.

So your whole code can be reduced like this:

$ch = fopen("test.csv", "r");
$header_row = fgetcsv($ch);

/* This will loop through all the rows until it reaches the end */
while($row = fgetcsv($ch)) {
    if (in_array("text", $row)) {
        echo '<div>' . implode(' | ', $row) . ' </div>';
    }
}

NOTE: if (in_array("text", $cols)) is enough and you don't need to look for FALSE, since fgetcsv() doesn't return falsy value other than FALSE itself.

Any not empty content is obviously not FALSE, and an empty line returns [0 => null], as stated by the manual:

  A blank line in a CSV file will be returned as an array comprising a single null field, and will not be treated as an error.

Using regexes

There is no need to use regexes: you're looking for a unique precise value, so in your case it'll be overkill!

It might not be the same if, for example, you wanted to find "text" even if it's only part of one (or more) column content.

Then you should do something like this:

if (preg_match('/text/', implode('', $row)))

Here we had to turn $row content into a string before applying the pattern.
But this can be improved in the context of your snippet: do implode() only once, so it will be alredy done for the echo, while the additional | doesn't matter for the preg_match.

This way, it'll result in:

while($row = fgetcsv($ch)) {
    if (preg_match('/text/', $row = implode(' | ', $row))) {
        echo '<div>' . $row . ' </div>';
    }
}


Remove outliers from a point cloud This function accepts a cloud of points, and returns those points that are within delta distance of the average (mean) position.

def points_average(points,delta):
    """ this function will check, for every point in points
    what are the points that are near the point (below a distance delta)
    it will then average every such points, creating a new list of points.
    This will continue until we arrive at a fixed amount of points """

    L = math.inf
    n = 0

    while L > len(points):

        L = len(points)

        points2 = []

        for i in xrange(len(points)):

            pt = points[i]
            d = (pt[0]-points[:,0])**2.+(pt[1]-points[:,1])**2.
            pts = points[d<delta**2.]

            x = np.average(pts[:,0])
            y = np.average(pts[:,1])

            points2 += [[x,y]]

        points2 = np.array(points2)
        points = np.unique(points2,axis=0)
        print len(points)

    return points

The function can be tested with:

import numpy as np

delta = 1
points = np.random.uniform(0,100,(500,2))
new_pts = simplify_by_avg_weighted(points,delta)

Please review the code for any improvements.  In particular, I would like to replace the for loop with a numpy vectorized version - any advice towards that would be welcome.

Please add a docstring.

Simplify_by_avg_weighted might be named points_near_centroid (or _center if you prefer).

Please initialize with L = math.inf, to clarify your intent.

Rather than a variable length list for [x,y,1], please store it as a 3-tuple (x, y, 1), or more naturally as a pair ((x, y), 1), where 1 is a count rather than a spatial coordinate.

    for i in xrange(len(points)):

This would more naturally be expressed as for pt in points:.

Now we come to the heart of the matter. It's a little hard to evaluate the code in the loop, as it seems to differ from your English language description, and it's hard to declare a "violates spec" bug when there's no docstring offering a spec.

When you assign x, y, they might better have names like x_centroid. And since z is not a spatial coordinate, it would more appropriately be named count. It's fine that you don't take square root, but d should have a name like dsq for distance squared.

To "return those points that are within delta of the mean position", does the interpreter even need to loop? It should suffice to compute centroid of the whole cloud (numpy does the looping), then select on distance (more numpy looping).


Showing/hiding deletion buttons in a grid view I want to perform some operations on the gridview (asp.net 4.0). So, to achieve this i have written a jquery function which i am calling on pageload but, because this function takes some time for execution my grid performance degrades (IE 8). Can i optimize it someway?

     $.each($("#divTab2GridInquiries").find("tr").not(":first"), function () {
            var tr = $(this);
            var val = tr.find("input[id*='hdnLineStatus']").val();
            var btnDelete = tr.find("div[id='divBtnDelete']");
            var btnTobeDeleted = tr.find("div[id='divBtnTobeDeleted']");
            if (val == "N") {
                btnDelete.hide();
                btnTobeDeleted.hide();
            }
            if (val == "S") {
                tr.css("background-color", "#99FF99");
                tr.find("input").css("background-color", "#99FF99");
                btnDelete.show();
                btnTobeDeleted.hide();
            }

            if (val == "D") {
                tr.css("background-color", "#FFFF99");
                tr.find("input").css("background-color", "#FFFF99");
                btnDelete.show();
                btnTobeDeleted.hide();
            }

            //From user rights
            if ($("input[id*='hdnTab2ShowDelete']").val() != "Y") {
                btnTobeDeleted.hide();
                //btnDelete.show();
            }
        });

Use a single selector instead of "find" and "not." Leave the find, this is faster in every browser except opera (see comments).

For the 2nd part of the selector, you can use the "sibling" combinator ~ to grab everthing except its first operand, and the :first-child pseudoclass selector to get the first child, giving you the same set of elements without using several jQuery methods. This is faster than using not(':first') in all browsers, and faster than a single selector (e.g. not using find either) in all browsers except Opera (which maintains its native-selector edge). See this test.

Note: #someTable tr will also return tr elements from a nested table. You really want to target the direct row descendants of the table. But don't forget about tbody, which is a required element. So this probably should be "#divTab2GridInquiries > tbody > tr:first-child ~ tr". But that is a mouthful... and it's really slow. If you have no nested tables it will work fine as coded below.

$.each($("#divTab2GridInquiries").find("tr:first-child ~ tr"), function () {
    var tr = $(this);

Not sure what  you're doing here - the selector is using a wildcard match, but val only operates against the first element in a selection set. Can you target this element more specifically? In any event, instead of wildcard matching the id, add a class and select on that. Classes are much faster than substring matching attributes.

        //var val = tr.find("input[id*='hdnLineStatus']").val();
        var val = tr.find(".hdnLineStatus").val();

IDs are supposed to be unique. I'm not sure why you would have to target it this way. But using an attribute selector like this will definitely be slower than a regular ID or class selector. If these ids are really unique then just use #divBtnDelete. I suspect that they aren't and you're creating invalid html. Get rid of the ID an add a class.

        // var btnDelete = tr.find("div[id='divBtnDelete']");
        var btnDelete = tr.find(".divBtnDelete");

        //var btnTobeDeleted = tr.find("div[id='divBtnTobeDeleted']");
        var btnTobeDeleted = tr.find(".divBtnTobeDeleted");

This set of ifs should be a switch, but that's probably not slowing you down nearly as much as the selectors.

        if (val == "N") {
            btnDelete.hide();
            btnTobeDeleted.hide();
        }
        if (val == "S") {
            tr.css("background-color", "#99FF99");h
            tr.find("input").css("background-color", "#99FF99");
            btnDelete.show();
            btnTobeDeleted.hide();
        }

        if (val == "D") {
            tr.css("background-color", "#FFFF99");
            tr.find("input").css("background-color", "#FFFF99");
            btnDelete.show();
            btnTobeDeleted.hide();
        }

Use a class again.

        //From user rights
        //if ($("input[id*='hdnTab2ShowDelete']").val() != "Y") {
        if ($(".hdnTab2ShowDelete").val() != "Y") {

            btnTobeDeleted.hide();
            //btnDelete.show();
        }
    });


Python script to clean out old files from a repository I wrote the script below as my first python program that will be running in a production environment. Essentially this program looks at a source location and moves all the files to a destination location if the file is older than 14 days old and does not exists in the destination. I had a user on stack overflow suggest some variable name changes and I am in the process of doing that. I would love if someone could review this code and let me know if anyone sees any potential bugs or errors that I might run into.

import os
import shutil
import time
import errno
import time
import sys
import logging
import logging.config

source = r'C:\Users\Desktop\BetaSource'
dest = r'C:\Users\Desktop\BetaDest'
#Gets the current time from the time module
now = time.time()
#Timer of when to purge files
cutoff = now - (14 * 86400)
source_list = []
all_sources = []
all_dest_dirty = []
logging.basicConfig(level = logging.INFO, filename = time.strftime("main-%Y-%m-%d.log"))

def main():
    dest_files()
    purge_files()
#I used the dess_files function to get all of the destination files
def dest_files():
    for dest_root, dest_subdirs, dest_files in os.walk(dest):
        for f in dest_files:
            global All_dest_dirty
            all_dest_dirty.append(f)

def purge_files():
    logging.info('invoke purge_files method')
    #I removed all duplicates from dest because cleaning up duplicates in dest is out of the scope
    all_dest_clean = list(dict.fromkeys(all_dest_dirty))
    #os.walk used to get all files in the source location
    for source_root, source_subdirs, source_files in os.walk(source):
        #looped through every file in source_files
        for f in source_files:
            #appending all_sources to get the application name from the file path
            all_sources.append(os.path.abspath(f).split('\\')[-1])
            #looping through each element of all_source
            for i in all_sources:
                #logical check to see if file in the source folder exists in the destination folder
                if i not in all_dest_clean:
                    #src is used to get the path of the source file this will be needed to move the file in shutil.move
                    src =  os.path.abspath(os.path.join(source_root, i))
                    #the two variables used below are to get the creation time of the files
                    t = os.stat(src)
                    c = t.st_ctime
                    #logical check to see if the file is older than the cutoff
                    if c<cutoff:
                        logging.info(f'File has been succesfully moved: {i}')
                        print(f'File has been succesfully moved: {i}')
                        shutil.move(src,dest)
                        #removing the allready checked source files for the list this is also used in other spots within the loop
                        all_sources.remove(i)
                    else:
                        logging.info(f'File is not older than 14 days: {i}')
                        print(f'File is not older than 14 days: {i}')
                        all_sources.remove(i)
                else:
                    all_sources.remove(i)
                    logging.info(f'File: {i} allready exists in the destination')
                    print(f'File: {i} allready exists in the destination')
if __name__ == '__main__':
    main()

This is just a brain dump of some things about your code that I hope you find useful.

Automated tools

Automated tools can help you maintain your code more easily.

The first ones I install whenever I start a new Python project, are:

black — The uncompromising Python code formatter
isort — A Python utility to sort imports
flake8 — A tool to enforce PEP 8 compliance (among other things)

I ran them on your code with the following options:

pipenv run black clean_old_files.py
pipenv run flake8 --max-line-length 88 clean_old_files.py
pipenv run isort clean_old_files.py -m=3 -tc -l=88

(An aside, I'm using Pipenv to manage my virtual environments. They're unmissable once you start getting into larger projects with external dependencies, and other contributors.)

Noisy comments

You have several comments that explain what the code does, rather than why.

For instance this:

# Gets the current time from the time module
now = time.time()
# Timer of when to purge files
cutoff = now - (14 * 86400)

which could be rewritten as:

# Timer of when to purge files
cutoff = time.time() - (14 * 86400)

which removes one "what" comment, and inlines a temporary variable.

My advice would be to go over all the comments in your script, and see which are "what" comments, and which are "why" comments.

Code structure

A minor thing to start off, I'd move the main function to the bottom of the script, right before if __name__ == "__main__", which, conceptually makes most sense to me.

You are mutating global variables from functions, which makes it harder to follow your program structure as-is.

On a related note, you also have global All_dest_dirty, which is a typo.

Globals

Instead of, for instance:

all_dest_dirty = []

def dest_files():
    for dest_root, dest_subdirs, dest_files in os.walk(dest):
        for f in dest_files:
            global all_dest_dirty
            all_dest_dirty.append(f)

prefer:

def dest_files():
    dirty_files = []
    for dest_root, dest_subdirs, dest_files in os.walk(dest):
        for f in dest_files:
            dirty_files.append(f)

    return dirty_files

all_dest_dirty = dest_files()

Closing remarks

Correct me if I'm wrong, but from

all_dest_clean = list(dict.fromkeys(all_dest_dirty))

I think that you're trying to remove duplicates from a list?

You can write that as

all_dest_clean = set(all_dest_dirty)

which uses the set data type, which prevents any duplicates. Plus, it's less typing, and also clearer. :-)

I hope you found my mini review helpful. You're doing great for a beginner! Keep it up. Also, let me know if anything is unclear.


Reduce a string using grammar-like rules I've got a program in Java which simplifies input string using grammar-like rules. Length of string is up to 200 and number of rules is up to 300. In big cases it works too slow, so I need some advice on time optimization of the code.

Example input: 4 4; a b a b; a b -> b; a b -> c; b a -> a; c c -> b

Program show all the simple chars that can be received from the input string a b a b using the rules. For this example it is b c. For string of length 155 and number of rules 92 the algorithm should work 0.05 seconds faster.

import java.util.Map;
import java.util.HashMap;
import java.util.Set;
import java.util.HashSet;
import java.util.List;
import java.util.ArrayList;
import java.util.Collections;
import java.io.BufferedReader;
import java.io.InputStreamReader;
import java.io.IOException;

public class Main {

    static int n, p;
    static String input;
    String[] textStr;
    static BufferedReader buff;
    static Map<String, Set<String>> cache = new HashMap();
    static Map<String, ArrayList<Character>> rules = new HashMap();

    public static void main(String[] args) {
        buff  = new BufferedReader(new InputStreamReader(System.in));
        try{
            String tem = buff.readLine();
            String textStr[] = tem.split("\\s+");
            n = Integer.parseInt(textStr[0]);
            p = Integer.parseInt(textStr[1]);
            tem = buff.readLine();
            input = tem.replaceAll("\\s+","");
            for(int i = 0; i < p; i++){
                tem = buff.readLine();
                textStr = tem.split("\\s+");
                Character cl = textStr[0].charAt(0);
                Character fa = textStr[1].charAt(0);
                Character re = textStr[2].charAt(0);
                if(!rules.containsKey(cl+" "+fa)){
                    rules.put(cl + " " + fa, new ArrayList());
                }
                rules.get(cl+" "+fa).add(re);
            }
            Set<String> temp = simplify(input);
            List<String> te = new ArrayList<>(temp);
            Collections.sort(te);
            for(int i = 0; i < temp.size(); i++){
                System.out.print(te.get(i)+" ");
            }
        }catch(IOException e){
            System.out.println("Error");
        }
        finally {
            try {
                buff.close();
            } catch (IOException e) {
                System.err.println("Error");
            }
        }
    }

    public static Set<String> simplify(final String in) {
        if(cache.containsKey(in)){
            return(cache.get(in));
        }
        Set<String> ret = new HashSet<>();
        if (in.length() == 1) {
            ret.add(in);
            return ret;
        }
        for (int i = 1; i < in.length(); i++) {
            String head = in.substring(0, i);
            String tail = in.substring(i);
            for (String c2 : simplify(tail)) {
                for (String c1 : simplify(head)) {
                    ArrayList<Character> rep = rules.get(c1+" "+ c2);
                    if (rep != null) {
                        for (Character c : rep) {
                            ret.add(c.toString());
                        }
                    }
                }
            }
        }
        cache.put(in, ret);
        return ret;
    }

}

n is unused.

You write to it here:

        n = Integer.parseInt(textStr[0]);

But you don't do anything with it.


Getting filepath of a specific asset I wanted to make my assets semi-dynamically so I made this utility class for it:

Assets PHP class:

<?php

    class Assets {

        private static $paths = array(
            'CSS' => '/assets/css/',
            'JS' => '/assets/js/',
            'FONTS' => '/assets/fonts/',
            'IMAGES' => '/assets/images/'
        );

        public static function get($file) {
            $fileType = pathinfo($file, PATHINFO_EXTENSION);

            switch (strtolower($fileType)) {
                case "css":
                    return self::$paths['CSS'] . $file;
                case "js":
                    return self::$paths['JS'] . $file;
                case "otf":
                case "eot":
                case "ttf":
                case "woff":
                case "woff2":
                case "svg":
                    return self::$paths['FONTS'] . $file;
                case "png":
                case "gif":
                case "jpg":
                    return self::$paths['IMAGES'] . $file;
                default:
                    throw new InvalidArgumentException("Filetype is not supported.");
            }
        }
    }
?>

Usage:

<link href="<?php echo Assets::get('bootstrap.css') ?>" rel="stylesheet">
<script type="application/javascript" src="<?php echo Assets::get('jquery.js') ?>"></script>

Is this efficient and how can I make it better, more dynamically and further.
Also I want to note I made this also to shorten my asset urls.

I think this class is largely fine as a display/view generation helper class, but worry about its broader utility.  Let me ask, if the PHP script in your usage example already owns the logic for the specific filename that it wants to include, shouldn't it also know the specific for the path to the file at this point already?  This HTML-rendering logic already knows know the most specific information about the asset that is to be included (the asset filename) but defers the more general information about the asset (its directory location) to the class.  That seems like an odd division of responsibility.  Additionally, your approach seems limited to only using asset files which are hosted on the same domain as the page being rendered, as I don't see how your solution supports full URL references.

Since you are going down the path of trying to dynamically inject what are basically config-driven values into this HTML template, I would suggest you truly decouple the configuration of included assets from the HTML template, perhaps striving more towards a solution like:

<html>
<head>
    <!-- Other HTML HEAD contents -->
<?php
// You could obviously use templating library as well,
// but raw PHP "spaghetti code" shown here for demonstration
foreach(Assets::getCssIncludes() as $url) {
?>
    <link rel="stylesheet" href="<?php echo $url; ?>">
<?php
}

foreach(Assets::getJsHeadIncludes() as $url) {
?>
    <script type="application/javascript" src="<?php echo $url; ?>"></script>
<?php
}
?>
</head>
<body>
    <!-- BODY content -->
<?php
foreach(Assets::getJsEndOfBodyIncludes() as $url) {
?>
    <script type="application/javascript" src="<?php echo $url; ?>"></script>
<?php
}
?>
</body>
</html>

With this sort of approach you can totally decouple your template from needing to have any hard-coded references to it's asset dependencies.  You could derive these from configuration and expose via "Assets" (or similar class).

This is advantageous as you begin to want to deploy different configurations of a page into different environments (i.e. dev, staging, production).  You could have production configured to inject the current stable production assets, while pre-prod environments can injects other assets versions or new assets that are not suitable or not yet deployed to production.


Stopping Google from tracking Search results I've created a brief UserScript (GitHub repository) for TamperMonkey to stop Google from hijacking Search results URLs for tracking purposes when you click them:

// ==UserScript==
// @include     http*://www.google.*/*
// @run-at      document-start
// ==/UserScript==
(function () {
    'use strict';
    function handleNodes(nodes) {
        for (var i = 0, j = nodes.length; i < j; i++) {
            handleNode(nodes[i]);
        }
    }
    function handleNode(node) {
        if (node.tagName === 'A' && node.onmousedown) {
            node.onmousedown = null;
        }
        handleNodes(node.childNodes);
    }
    document.onclick = null;
    var observer = new MutationObserver(function (mutations, observer) {
        for (var i = 0, j = mutations.length; i < j; i++) {
            var mutation = mutations[i];
            for (var k = 0, l = mutation.addedNodes.length; k < l; k++) {
                handleNode(mutation.addedNodes[k]);
            }
        }
    });
    observer.observe(document, { childList: true, subtree: true });
} ());

The script seems to work for both Google "Classic" and Google Instant doing its job.

Since I'm not a UserScript or DOM guru I'd like some feedback on it. Did I make any mistake or inefficiency? Is there something that can be improved?

to stop Google from hijacking Search results URLs for tracking purposes when you click them

Wrong, there's a lot of ways websites profile you. One other method is your device fingerprint, the combination of hardware, software and network information that normally gets sent to the server. A server can then compute a hash and link it to you.

No matter how you block the click-jacked search results, if you end up on a page that happens to have some widget that reports back to their server (with the same hardware, software and network info), consider yourself tracked. These "widgets" come in a variety of forms, like ads, like buttons, tweet buttons, follow buttons etc.

As for your code...

node.onmousedown = null

You are only removing the inline/property handler. You're not preventing handlers that are assigned via addEventListener and delegated (handlers assigned to ancestors to listen to a descendant).

function foo(message){
  alert('No stopping ' + message);
}

var link = document.querySelector('a');

// Assigned as a property
link.onmousedown = function(){
  foo('property');
};

// Assigned as listener
link.addEventListener('mousedown', function(){
  foo('handler');
});

// Body property
document.body.onmousedown = function(){
  foo('delegated property');
};

// Delegated handler
document.body.addEventListener('mousedown', function(){
  foo('delegated handler');
});

// Your script only does this:
link.onmousedown = null;
Test

Now there's several steps needed to prevent it.

First, we cannot prevent addEventListener handles since removeEventListener requires the reference of the handle function which we normally don't have access to. The best we could do is to clone the node, which doesn't copy over handlers assigned via addEventListener.

Then we override the inline onmousedown by defining your own function. Then we prevent delegated handlers by calling stopPropagation.

Lastly, we replace the original element with the clone.

function foo(message){
  alert('No stopping ' + message);
}

var link = document.querySelector('a');

// Assigned as a property
link.onmousedown = function(){
  foo('property');
};

// Assigned as listener
link.addEventListener('mousedown', function(){
  foo('handler');
});

// Body property
document.body.onmousedown = function(){
  foo('delegated property');
};

// Delegated handler
document.body.addEventListener('mousedown', function(){
  foo('delegated handler');
});

var clone = link.cloneNode(true);

clone.onmousedown = function(event){
  event.stopPropagation();
}

link.parentNode.replaceChild(clone, link);
Test


Convert a mediawiki to dokuwiki script I have quickly wrote a Perl script to convert a mediawiki to a dokuwiki. I know there's an existing script to make that, but I would have a simpler usage code and some use of the API from mediawiki. It was also a good occassion to use pQuery module. I know I can improve some things and I'm going to make a generic command and use Moo and MooX::Options for the next version.  I may also write a parser to convert the syntax, but I did make a first version quickly and an easy script some can easily maintain for no Perl programmers.

I would like to know what you think about the code:

#!/usr/bin/env perl

use strict;
use warnings;

use IO::File;

use HTTP::Request::Common qw(POST);
use LWP::UserAgent;

use MediaWiki::API;
use pQuery;
use Text::Unidecode;

sub generate_file_name {
    my $title  = shift;

    $title =~ s/\s/_/g;
    $title =~s/'/_/g;

    return unidecode($title) . '.txt';
}

sub set_content_file {
    my ( $file, $content ) = @_;

    my $fh = IO::File->new($file, "w");
    if ( defined($fh) ) {
        print $fh $content;

        undef $fh;
    }

    return;
}

sub mediawiki_login {
    my $mediawiki = MediaWiki::API->new({
        api_url => 'http://vim-fr.org/api.php'
    });

    $mediawiki->login({
        lgname     => 'login',
        lgpassword => 'mypassword'
    }) || die $mediawiki->{error}->{code} . ': ' . $mediawiki->{error}->{details};

    return $mediawiki;
}

my $mediawiki    = mediawiki_login();
my $all_articles = $mediawiki->list({
    action => 'query',
    list   => 'allpages'
});
my $user_agent = LWP::UserAgent->new();

foreach my $page (@{$all_articles}) {
    my $article = $mediawiki->get_page({
        title => $page->{title}
    });

    my $request   = POST 'http://johbuc6.coconia.net/mediawiki2dokuwiki.php', [ mediawiki => $article->{'*'} ];
    my $response  =  $user_agent->request($request);
    my $file      = generate_file_name($page->{title});

    print 'Export de la page ' . $page->{title} . " dans le fichier $file", "\n";

    pQuery($response->content)
        ->find('textarea[name=dokuwiki]')
        ->each(sub {
            my $count = shift;
            set_content_file($file, pQuery($_)->html());
        });
}

It was a long time ago when I did not use Perl my favorite language AND I'm going to use more. I hope it was not so bad.

This looks fairly good, which is why I'll be fairly harsh.

generate_file_name

Even with single arguments, I'd suggest using a line like my ($title) = @_; rather than shift: Using @_ is the dominant idiom, and it will be less confusing for non-Perl programmers.

The two substitutions can be performed in one go: s/[\s']/_/g – but why are you removing unacceptable characters before the unidecode step, which might introduce them. I find it interesting that you do not prevent characters like ", \, /. It might be more sensible to specifically allow a certain set of characters rather than disallowing some other set (where you are bound to forget something important).

But why are you even using Text::Unidecode? Not only is this module extremely limited in what it can do, but most filesystems also have some degree of support for Unicode filenames.  If we ignore this last point, I would write:

sub generate_file_name {
    my ($title) = @_;
    my $ascii_title = unidecode($title);    # transliterate Unicode
    $ascii_title =~  s/[^a-zA-z0-9.-]+/_/g;  # remove unwanted characters
    return $ascii_title;
}

set_content_file

The first problem is the word order in this function's name, it should be set_file_contents. But this still isn't a terribly good name, I'd rather say write_file. By the way, a function with this name and functionality is already provided by File::Slurp, but we might as well write it ourselves.

Using the object oriented IO::File instead of the builtin open is OK. I don't like it, but it probably makes it easier for non-Perl programmers to understand.

But there is one problem: While a file may only contain bytes, the input string may contain Unicode: You have to encode the string either explicitly, or through a PerlIO encoding layer.

The undef $fh is highly unusual. The $fh is a lexical variable – once the variable goes out of scope, the contained data's refcount is decremented and the object destroyed. Destroying a file handle will close it. undef $fh resets the scalar, which will effectively do the same thing. So it is a very confusing way of doing nothing, or saying $fh->close. Leave it.

Instead of ignoring any errors when opening a file handle fails, you should probably terminate an error or at least output a warning message.

Performing the generate_file_name transformation should happen inside this code, the details of storage are irrelevant for the other code.

We should not overwrite a file if our normalization produces the same name twice. Instead: detect this error, and at the very least produce an error message.

sub write_file {
    my ($file, $contents) = @_;
    # technically, we have a concurrency bug here between the `-e` and the `open`, but I'd ignore it.
    die qq(The file "$file" already exists. Will not overwrite) if -e $file;
    open my $fh, "<:utf8", $file or die qq(Can't open "$file": $!);
    print { $fh } $contents;
    return;
}

...

use Try::Tiny;

...

try {
    write_file($title, pQuery($_)->html);
}
catch {
    warn $_;
};

mediawiki_login

Does this function add any abstraction to the code? I think not, considering also its proximity to its only point of use.

main code

For some reason, you are using HTTP::Request::Common instead of the available LWP::UserAgent methods (which admittedly are just a wrapper).

my $response = $user_agent->post('http://johbuc6.coconia.net/mediawiki2dokuwiki.php', [ mediawiki => $article->{'*'} ]);

You should also at least check for success:

if (not $response->is_success) {
    warn qq(Request for article "$page->{title}" failed: ), $response->status_line;
    next;
}


Consuming an ASP.NET Web API call in an MVC controller In a new project I am creating for my work, I am creating a fairly large ASP.NET web API. The API will be in a separate Visual Studio solution that also contains all of my business logic, database interactions, and Model classes.

In the test application I am creating (which is ASP.NET MVC4), I want to be able to hit an API URL I defined from the control and cast the return JSON to a Model class. The reason behind this is that I want to take advantage of strongly typing my views to a Model. This is all still in a proof of concept stage, so I have not done any performance testing on it, but I am curious if what I am doing is a good practice, or if I am crazy for even going down this route.

Here is the code on the client controller:

public class HomeController : Controller
{
    protected string dashboardUrlBase = "http://localhost/webapi/api/StudentDashboard/";

    public ActionResult Index() //This view is strongly typed against User
    {
        //testing against Joe Bob
        string adSAMName = "jBob";
        WebClient client = new WebClient();
        string url = dashboardUrlBase + "GetUserRecord?userName=" + adSAMName;
        //'User' is a Model class that I have defined.
        User result = JsonConvert.DeserializeObject<User>(client.DownloadString(url));
        return View(result);
    }
. . .
}

If I choose to go this route, another thing to note is I am loading several partial views in this page (as I will also do in subsequent pages). The partial views are loaded via an $.ajax call that hits this controller and does basically the same thing as the code above:

Instantiate a new WebClient
Define the URL to hit
Deserialize the result and cast it to a Model Class

So it is possible (and likely) I could be performing the same actions 4-5 times for a single page.

Is there a better method to do this that will:

Let me keep strongly typed views
Do my work on the server rather than on the client (this is just a preference since I can write C# faster than I can write JavaScript)

Testing?

Then create a simple unit test and test your API controller or your business layer classes directly.
If you want to create an integration test or need a cleaner solution in production code read further.

Wrapping the API calls

This is just a disposable wrapper around the WebClient which can be easily reused.

public abstract class WebClientWrapperBase : IDisposable
{
    private readonly string _baseUrl;
    private Lazy<WebClient> _lazyClient;

    protected WebClientWrapperBase(string baseUrl)
    {
        _baseUrl = baseUrl.Trim('/');
        _lazyClient = new Lazy<WebClient>(() => new WebClient());
    }

    protected WebClient Client()
    {
        if (_lazyClient == null)
        {
            throw new ObjectDisposedException("WebClient has been disposed");
        }

        return _lazyClient.Value;
    }

    protected T Execute<T>(string urlSegment)
    {
        return JsonConvert.DeserializeObject<T>(Client().DownloadString(_baseUrl + '/' + urlSegment.TrimStart('/')));
    }

    ~WebClientWrapperBase()
    {
        Dispose(false);
    }

    public void Dispose()
    {
        Dispose(false);
        GC.SuppressFinalize(this);
    }

    protected virtual void Dispose(bool disposing)
    {
        if (_lazyClient != null)
        {
            if (disposing)
            {
                if (_lazyClient.IsValueCreated)
                {
                    _lazyClient.Value.Dispose();
                    _lazyClient = null;
                }
            }

            // There are no unmanaged resources to release, but
            // if we add them, they need to be released here.
        }
    }
}

Creating a "strongly typed proxy":

class StudentDashboardClient : WebClientWrapperBase
{
    public StudentDashboardClient()
        : base("http://localhost/webapi/api/StudentDashboard/")
    {
        //just for compatibility
    }

    public StudentDashboardClient(string baseUrl)
        : base(baseUrl)
    {
    }

    public User GetUserRecord(string userName)
    {
        return Execute<User>("GetUserRecord?userName=" + userName);
    }
}

And then passing to your controllers where it's needed:

public class HomeController : Controller
{
    private readonly StudentDashboardClient _studentDashboardClient;

    public HomeController(StudentDashboardClient studentDashboardClient)
    {
        _studentDashboardClient = studentDashboardClient;
    }

    public ActionResult Index()
    {
        return View(_studentDashboardClient.GetUserRecord("jBob"));
    }

    protected override void Dispose(bool disposing)
    {
        if (disposing)
        {
            _studentDashboardClient.Dispose();
        }
        base.Dispose(disposing);
    }
}

Note that the controller now have a parameterless constructor so you will need a solution to instantiate controllers this way for example a DI framework with MVC support like Ninject.

What you gain? Cleaner code.


Creating a real unit test in Karma for Angular code This is a piece of test code made for the Angular application.

The router object is a mock provided by the RouterTestingModule dependency. I wonder if such a test can be considered a unit test (because it actually tests only one element and the fact that it calls some method - without checking its result), or should it be called an integration test (due to the fact that it still call external dependence)?

it('should trigger the navigation to `/home`', async(() => {
  const link = fixture.debugElement.nativeElement.querySelector('.home-link');

  link.click();

  expect(router.navigateByUrl).toHaveBeenCalled();
}))

Since

  router object is a mock, and
  the fact that it calls some method - without checking its result

You have a unit test, more specifically a whitebox test, as opposed to a blackbox test that tests the output of some method.

For it to become an integration test, you would have use a router instead of a mock.


Vectorized type conversion for autocorrelation I have a simple task as part of a larger autocorrelation DSP system - to convert from signed 16-bit integer audio samples to floating-point. This part is quite self-contained:
#include <cblas.h>

typedef int16_t sample_t;

void consume(const sample_t *samples, int N, int rate)
{
    float *input = calloc(N, sizeof(float)),
         *output = calloc(N, sizeof(float));
    assert(input);
    assert(output);

    for (int i = 0; i < N; i++)
        input[i] = (float)samples[i];

    // BLAS processing of 'input'

    for (int i = 0; i < N; i++)
    {
        output[i] = cblas_sdot(  // Standard CBLAS call
            N-i,     // N
            input,   // X
            1,       // incX
            input+i, // Y
            1        // incY
        );
    }
}

Due to external constraints - the hardware only supports S16_LE samples - ALSA cannot do this conversion for me, nor can cblas accept integer types; so I'm stuck doing this myself. I've not found anything in gsl or cblas that runs vectorized type conversion.
I'm also not in love with having to loop over a level-1 dot product, but I've not been able to find a good way in BLAS to represent a single level-2 (matrix-vector) multiplication where the matrix is just a heavily-aliased single vector.
Is there a better way to do this?

If this code goes into production then the asserts for the success of calloc() will be optimized out of the code if the code is optimized. Prefer if statements with calls to fprintf(stderr,.
The parameter rate is never used, this can lead to maintenance errors in the future.
For maintenance reasons the ability of someone else to understand the code I would suggest using &input[i] rather than input+ in the function call.


Time Limit Exceeded for ETF - Euler Totient Function at Spoj In number theory, the totient φ of a positive integer n is defined to be the number of positive integers less than or equal to n that are coprime to n.

Given an integer n (1 ≤ n ≤ 106), compute the value of the totient φ.

For the code written below, this is giving TLE. How i can optimize this code or should i prefer some other language for this problem.
Link of the problem at spoj

test = int(input())
for t in range(test):
    n = int(input())
    result = n
    i = 2
    while i*i <= n :
        if n %i == 0:
            while n%i == 0 :
                n /= i
            result -=result/i
        i +=1
    if n > 1:
        result -= result/n
    print(int(result))

The way I would approach this problem is to find the prime factorization of n, and then use Euler's formula which says:

$$\varphi(n) = n \prod_{p|n}\left(1 - \frac{1}{p}\right)$$

The easiest way to do this is to find the prime factorization of n (which you are basically already doing), but store each distinct prime factor in a list. Once you are done, compute this formula for the primes, and you should have your answer fairly quickly.


Submitting form using dynamic variable with query I have a form that I submit with these variables: category_time, date_month, date_year, and status_time.

Here is an example of different form possibilities:

CATEGORY_TIME     1
DATE_MONTH    Month:
DATE_YEAR     Year:
FIELDNAMES    DATE_MONTH,DATE_YEAR,SUBMIT
STATUS_TIME   Select:
SUBMIT    submit
----------------
CATEGORY_TIME     Choose:
DATE_MONTH    Month:
DATE_YEAR     Year:
FIELDNAMES    DATE_MONTH,DATE_YEAR,SUBMIT
STATUS_TIME   Pending
SUBMIT    submit
----------------
CATEGORY_TIME     Choose:
DATE_MONTH    Month:
DATE_YEAR     2016
FIELDNAMES    DATE_MONTH,DATE_YEAR,SUBMIT
STATUS_TIME   Select:
SUBMIT    submit

After I submit the form depending on the variables it outputs a query. For example, with the first possibility only category_time chose, it will output the second if statement.

From what you can see, there's a lot of ifs statements that will be needed. Is there an easier way to make this work without so many ifs statement and just depending on the variables that are chosen, even if I have to use JavaScript to make it work better?

<cfif  structKeyExists(form, "category_time") ||  structKeyExists(form, "date_month")  || structKeyExists(form, "date_year") || structKeyExists(form, "status_time") >
<cfquery ....>
SELECT  *
    FROM work_timeline
    <cfif  (category_time eq 'ALL' || category_time eq 'Choose:') &&  date_month eq 'Month:' && date_year eq 'Year:' && status_time eq 'Select:' >
        order by year(date_time) desc, month(date_time) desc
    </cfif>

    <cfif  category_time neq 'ALL' && category_time neq 'Choose:' &&  date_month eq 'Month:' && date_year eq 'Year:' && status_time eq 'Select:' >
    where category_time = #category_time#
    </cfif>
    <cfif structKeyExists(form, "category_time") && date_month neq 'Month:' && date_year eq 'Year:' && category_time neq 'Choose:'
        and category_time neq 'ALL' && status eq 'Select:'>
    where category_time = #category_time# and month(date_time)=#date_month#
    </cfif>
    more if's statments .....
</cfquery>
</cfif>

A few things you could do to simplify the code:

Rather than examining every form field to determine if the form was submitted, a single check of say - the submit button - should suffice for most purposes.
Avoid using strings like "ALL" or "Choose:" for the form field values. Those are fine for display purposes, but the field values should be something more generic, like an empty string. That makes for simpler, more uniform, validation code.

For example, if the default is an empty string, a simple LEN() check is all you need to determine if a particular field was populated. (You can also use other boolean functions like IsNumeric or IsDate, when more appropriate).
Consider using cfparam to assign a default for all fields (except the submit button). That ensures the fields always exist, eliminating the need to use structKeyExists every time you access a field.
With the above changes, you could also simplify the WHERE clause logic. Start with an expression that is always true. Then add additional filters based on which form fields were populated.

NB: Using raw client supplied values in a queries puts your database at risk for sql injection. Always use cfqueryparam on all parameters. (I do not know the data types of your database columns, so modify the cfsqltype's as needed.)

<cfparam name="FORM.category_time" default="">
<cfparam name="FORM.date_month" default="">
...

<cfif structKeyExists(FORM, "submit")>

    <cfquery ....>
        SELECT Column1, Column2, ....
        FROM   work_timeline
        WHERE  1 = 1

        <!--- Filter on FORM.category_time when populated  --->
        <cfif LEN(FORM.category_time)>
            AND category_time = <cfqueryparam value="#FORM.category_time#" cfsqltype="cf_sql_integer">
        </cfif>

        <!--- Filter on FORM.date_month when populated --->
        <cfif LEN(FORM.date_month)>
            AND month(date_time) = <cfqueryparam value="#FORM.date_month#" cfsqltype="cf_sql_integer">
        </cfif>

        ... more cfif conditions
    </cfquery>
</cfif>

As an aside, regarding SQL

There is nothing technically wrong with using SELECT *, but in most cases it is preferable to supply a list of the columns you need. It makes the code a more readable, and also helps avoid pulling back extra data columns that are never used.
Using functions on an indexed database column usually prevents the database from utilizing the index. While it may not be feasible for your current query, when possible it is best to revise expressions like this:

WHERE month(column) = 12 AND year(column) = 2015

.. to use a more index friendly expression instead. For example:

   WHERE column >= '2015-12-01' -- Begin date at midnight
   AND   column <  '2016-01-01' -- Day AFTER end date at midnight


Transforming an array of arrays into an array with joins I'm transforming this:

[["Apples", "fruitgroup", "123"], ["Bananas", "fruitgroup", "012"], ["Jumbo Jet", "planegroup", "99"], ["Crop duster", "planegroup", "40"], ["Melons", "fruitgroup", "55"]]

into this:

["Apples or Bananas or Melons [x]", "Jumbo Jet or Crop duster [x]"]

Because basically I have to group the child arrays by whatever is in their second index, or [1], and then join them with " or ".

I am successful but I'm afraid this isn't really the best way to acheive this.  Isn't there a more simple way to do this?

var
  filters = [["Apples", "fruitgroup", "123"], ["Bananas", "fruitgroup", "012"], ["Jumbo Jet", "planegroup", "99"], ["Crop duster", "planegroup", "40"], ["Melons", "fruitgroup", "55"]],
  fieldnames = _.uniq(_.map(filters, function (filter) {
    return filter[1];
  })),
  groups = _.groupBy(filters, function (filter) {
    return filter[1];
  }),
  strings = _.map(groups, function (group) {
    return _.map(group, function (item) {
      return item[0];
    }).join(" or ") + " [x]";
  });
console.log(strings);

The approach seems just fine to me.

However, I noticed that your fieldnames variable isn't actually used for anything. So that can go.

You can also use some shortcuts in underscore.js:

var groups, strings;

groups = _.groupBy(filters, 1); // group by index 1 (no need for a callback)

strings = _.map(groups, function (group) {
  return _.pluck(group, 0).join(" or ") + " [x]"; // pluck index 0 and join
});

As much as I like functional style, it does get a bit dense, so I've spread it out a tiny bit, rather than have it all as a run-on var declaration. (If it's an enterprise-level app, I imagine maintenance is a high-priority, so better to err on the side of clarity.)

You could also go the opposite way, and inline the _.groupBy, but I wouldn't recommend it. There's no performance gain, only readability pain.

A compromise might be undescore's chaining system:

var strings = _.chain(filters)
  .groupBy(1)
  .map(function (group) {
    return _.pluck(group, 0).join(" or ") + " [x]";
  })
  .value();

But again, there's little need.


Sum the exponents of the prime factors of a number My goal is to find the sum of the exponents of the prime factors of an integer.

I wrote the code below with the complexities specified here (I am not 100% sure of the complexities though):

Runtime complexity:

findPrimesSmaller: \$O(n \log(\log n))\$
exponentInDecomposition: \$O(n)\$
primeExponentsCount: \$O(n^2 \log(\log n))\$

Space Complexity:

findPrimesSmaller: \$O(\log N)\$ (?)
exponentInDecomposition: \$O(1)\$
primeExponentsCount: \$O(\log N)\$

import static org.junit.Assert.*;

import java.util.*;

import org.junit.*;

public class PrimeCount {
  // Given a number n and its associated prime numbers decomposition:
  // n = p1 ^ (alpha1) * p2 ^ (alpha2) * ... * pn ^ (alphan)
  // where all the pi's are prime numbers smaller than n
  // return the sum of the alphai's

  public int primeExponentsCount(int n) {
    // First generate the list of prime numbers smaller than n
    ArrayList<Integer> candidatePrimeNumbers = findPrimesSmaller(n);

    // initialize the sum
    int result = 0;

    for (int i : candidatePrimeNumbers) {
      result += exponentInDecomposition(i, n);
    }

    return result;
  }

  public ArrayList<Integer> findPrimesSmaller(int n) {

    int[] allNumbers = new int[n + 1];

    // Initialize the list with all the numbers smaller than n
    for (int i = 0; i <= n; i++) {
      allNumbers[i] = i;
    }

    // Apply a Sieves of Eratosthenes method to keep only the prime numbers
    for (int i = 2; i <= n; i++) {
      for (int j = i + 1; j <= n; j++) {
        if (j > 0 && j % i == 0) {
          allNumbers[j] = -1;
        }
      }
    }

    ArrayList<Integer> primeNumbers = new ArrayList<Integer>();
    for (int i = 2; i <= n; i++) { // Remove 0 and 1 from list => start counting at 2
      if (allNumbers[i] > 0) {
        primeNumbers.add(i);
      }
    }

    return primeNumbers;
  }

  public int exponentInDecomposition(int p, int n) {
    int result = 0;

    while (n % Math.pow(p, result) == 0 && n / Math.pow(p, result) >= 1) {
      result++;
    }
    result--;

    return result;
  }

  @Test
  public void test_exponentZeroDecomposition() {
    int a = exponentInDecomposition(3, 5);
    assertEquals(0, a);
  }

  @Test
  public void test_exponentNonZeroDecomposition() {
    int a = exponentInDecomposition(3, 9);
    assertEquals(2, a);
  }

  @Test
  public void primesSmaller10() {
    ArrayList<Integer> primesLessThan10 = findPrimesSmaller(10);
    int two = primesLessThan10.get(0);
    int three = primesLessThan10.get(1);
    int five = primesLessThan10.get(2);
    int seven = primesLessThan10.get(3);
    assertEquals(2, two);
    assertEquals(3, three);
    assertEquals(5, five);
    assertEquals(7, seven);
  }

  @Test
  public void testCount0() {
    int zero = primeExponentsCount(0);
    assertEquals(0, zero);
  }

  @Test
  public void testCount1() {
    int zero = primeExponentsCount(1);
    assertEquals(0, zero);
  }

  @Test
  public void testCount10() {
    int two = primeExponentsCount(10);
    assertEquals(2, two);
  }

  @Test
  public void testCount13() {
    int one = primeExponentsCount(13);
    assertEquals(1, one);
  }

  public static void main(String[] args) {
    PrimeCount e = new PrimeCount();
    e.test_exponentZeroDecomposition();
    e.test_exponentNonZeroDecomposition();
    e.primesSmaller10();
    e.testCount0();
    e.testCount1();
    e.testCount10();
    e.testCount13();

  }
}

In addition to the other answer:

There is no need to generate all primes up to n. You can generate them up to sqrt(n). If something remains after the division by these primes, it is a prime.
There is no need to generate any prime numbers at all. This code does exactly what you need:

public int primeExponentsCount(int n) {
    if (n <= 1)
        return 0;
    int sqrt = (int) Math.sqrt(n);
    int remainingNumber = n;
    int result = 0;
    for (int i = 2; i <= sqrt; i++) {
        while (remainingNumber % i == 0) {
            result++;
            remainingNumber /= i;
        }
    }
    if (remainingNumber > 1) {
        result++;
    }
    return result;
}

The time complexity of this algorithm is O(sqrt(n)). The space complexity is O(1). As you can see, there is much room for improvement for your initial solution in terms of efficiency.

Now about the code itself:

Writing self-evident comments is a bad practice. For instance, this comment doesn't serve any purpose.

// initialize the sum
int result = 0;

I would delete it.
Error handling. It looks like your primeExponentsCount does not expect to receive a negative number. Instead of returning a wrong result, I'd throw an exception:

public int primeExponentsCount(int n) {
    if (n < 0)
        throw new IllegalArgumentException("The number must be non-negative");
    ...
}

There are too many blank lines inside methods, in my opinion(it is conventional to separate methods from each other with an empty line, but it is not common to insert random blank lines inside a method's body).


MySQL database with foreign key support A modified review request has been made here.

Can I please have someone review this simple database design below and tell me if this is the correct way to achieve what I want to do?

What I am trying to do:

When a cardID is deleted this cascades and deletes the surveyID
    associated row.
When a trackID is deleted this cascades and deletes the cardID
    associated  row and its associated surveyID row.

ENTITY RELATIONSHIP DIAGRAM

MYSQL CODE

CREATE TABLE `Card` (
 `cardID` int(11) NOT NULL AUTO_INCREMENT,
 `trackID` int(11) NOT NULL,
 `fName` varchar(21) NOT NULL,
 `mName` varchar(1) DEFAULT NULL,
 `lName` varchar(21) DEFAULT NULL,
 `email` varchar(40) NOT NULL,
 `isMember` int(1) NOT NULL,
 PRIMARY KEY (`cardID`),
 FOREIGN KEY (`trackID`) REFERENCES `Tracker`(`trackID`) ON UPDATE CASCADE ON DELETE CASCADE
) ENGINE=MyISAM DEFAULT CHARSET=latin1 AUTO_INCREMENT=3 ;

CREATE TABLE `Survey` (
 `surveyID` int(11) NOT NULL AUTO_INCREMENT,
 `cardID` int(11) NOT NULL,
 `trackID` int(11) NOT NULL,
 `q0` int(1) NOT NULL,
 `q1` int(1) DEFAULT NULL,
 `q2` int(1) DEFAULT NULL,
 `q3` int(1) DEFAULT NULL,
 `q4` int(1) DEFAULT NULL,
 `q5` int(1) DEFAULT NULL,
 PRIMARY KEY (`surveyID`),
 FOREIGN KEY (`trackID`) REFERENCES `Tracker`(`trackID`)  ON UPDATE CASCADE,
 FOREIGN KEY (`cardID`) REFERENCES `Card`(`cardID`)ON UPDATE CASCADE ON DELETE CASCADE
) ENGINE=MyISAM DEFAULT CHARSET=latin1 AUTO_INCREMENT=3 ;

CREATE TABLE `Tracker` (
 `trackID` int(11) NOT NULL AUTO_INCREMENT,
 `tName` varchar(21) DEFAULT NULL,
 `tDesc` varchar(50) DEFAULT NULL,
 PRIMARY KEY (`trackID`)
) ENGINE=MyISAM DEFAULT CHARSET=latin1 AUTO_INCREMENT=3 ;

I'd use longer attribute names (firstName instead of fName, middleName, lastName).
It's easier to read and maintain.
Are you sure that latin1 is enough for every data? Consider using UTF-8.


Node.js building a JSON restful web api structure I am building a JSON restful web api in Node.js and so far I only got the main structure, however I am not 100% sure I am doing this correct from scratch, I still have to get used to the fact that Node.js is event driven.

Directory structure:

1. main.js
2. modules
  a. router.js
  b. config.js
  c. routes.js

1. main.js - contains the main entry point - node main.js
2. modules - folder that contains all the modules.

2a. router.js - contains "singleton" class module that will handle routing.

var Router = function(routes){
    this.routes = routes;

    Router.route = function(){
        console.log('routing from singleton router...');
    };

    return Router;
};

module.exports = Router;

2b. config.js - contains the global configuration.

exports.config = {
    port: 9090
};

2c. routes.js - contains the global routing schema, the router module will get the module injected (from main.js)

exports.routes = {
    'POST /users': { 'controller': 'users', 'action': 'create' },
    'GET /users': { 'controller': 'users', 'action': 'getSelf' },
    'GET /users/:uid/posts/:pid': { 'controller': 'users', 'action': 'getPostsFromUser' }
};

This is the structure so far, I am planning to extend it to this directory structure:

1. main.js
2. modules
  a. router.js
  b. config.js
  c. routes.js
  d. loader.js (model loader, connection pool etc.)
  e. validator.js (validates input)
  d. controller.js (main controller, how to inherit from this? through inheritance?)
3. controllers - contains controllers
4. models - contains models

How to handle connection pooling in the structure? Would it be an idea to create a "registry" class, I come from a PHP background and that is how I would share objects, instead of dependency injection.

sample registry.js

var Registry = function(){
    this.data = {};

    Registry.push = function(key, value){
        return this.data[key] = value;
    };

    Registry.pull = function(key){
        return this.data[key];
    }

    return Registry;
};

module.exports = Registry;

As I said, I am kinda new to Node.js and I want to make sure I get it right from scratch.

If you got any suggestions, comments, tips I really appreciate your help and time.

Sorry if I'm not answering your specific question, but are asking about overall style, or something else?  There's no "right/wrong way" for many things in node.

For example, I would have used http://expressjs.com/ for this:

var express = require('express')
var app = express.createServer()
app.use(express.bodyParser())

app.get('/users', function(req, resp, next) {
   ...
})

app.get('/users/:uid/posts/:pid', function(req, resp, next) {
   ...
})

app.post('/users', function(req, resp, next) {
   ...
})

app.listen(9090)