T-SQL UDF to replace replace base URLs I have a problem where I need to replace every base part of URLs in a body of text.  However, I only want to replace it in specific URLs.  Only URLs that go to "resource.aspx" that have the "source" query string parameter need to get the replacement.  Other URLs could have the target base part and we don't want to touch those.  The resource urls are expected to appear inside quotes.

I made a UDF that takes the text body, the base URL you want to replace, and the base URL to replace with and searches through the text and does the base URL replacement on any "resource URLs" it finds.

Does the algorithm look sound?  Anything I missed?  Better ways to do this?

I was sad to learn that REPLACE() does not work with a pattern, even though it's second paramter is named "string_pattern" :(.  I was hoping it would work like .NET's Regex.Replace() (I know we don't have regex in t-sql, but a pattern based replace function would be nice sigh)

-- =============================================
-- Author:      John Doe
-- Create date:  11/29/2016
-- Description:  Replaces base URLs in text body of every URL that points to
--               resource.aspx that has "source" query string parameter.
--               Replaces specified base url with specified replacement
-- =============================================
CREATE FUNCTION dbo.udfReplaceResourceBaseURLs
(
    @TextBody NVARCHAR(MAX),
    @BaseUrlToReplace NVARCHAR(MAX),
    @BaseUrlReplacement NVARCHAR(MAX)
)
RETURNS NVARCHAR(MAX)
AS
BEGIN
        -- Create Copy of the original body.  We will search and modify the copy in the loop
        DECLARE @EmailBodyCopy NVARCHAR(MAX)
        SET @EmailBodyCopy = @TextBody

        DECLARE @ResourceURLPattern NVARCHAR(MAX) = '%' + @BaseUrlToReplace + '/myResource.aspx%source=%'

        -- Do replacements while resource URLs are found in the copy
        DECLARE @resourceURLStart INT = PATINDEX(@ResourceURLPattern, @EmailBodyCopy)
        WHILE @resourceURLStart > 0
        BEGIN
            -- Get the full URL of the found resource URL
            DECLARE @resourceURLEnd INT = CHARINDEX('"', @EmailBodyCopy, @resourceURLStart)
            DECLARE @resourceURL NVARCHAR(MAX) = SUBSTRING(@EmailBodyCopy, @resourceURLStart, (@resourceURLEnd - @resourceURLStart))

            -- Replace base URL to get the new URL with the desired base URL replaced in
            DECLARE @newResourceURL NVARCHAR(MAX) = REPLACE(@resourceURL, @BaseUrlToReplace, @BaseUrlReplacement)

            -- Replace the URL in the original body with our new one
            SET @TextBody = REPLACE(@TextBody, @resourceURL, @newResourceURL)

            -- Chop off the front of the body copy up to the end of the current resource URL
            -- so that we find the next resource url on next pass
            SET @EmailBodyCopy = SUBSTRING(@EmailBodyCopy, @resourceURLEnd, 999999999) -- get to index infinity to make sure grab to end

            SET @resourceURLStart = PATINDEX(@ResourceURLPattern, @EmailBodyCopy)
        END

        RETURN @TextBody
END

Your code looping over string functions doesn't seem like something SQL Server is optimized for.

Database systems are optimized for set based operations, not looping. I understand the confusion coming from procedural/Object Oriented languages, but in the end SQL isn't a programming language in it's classical form but a way to instruct an RDBMS what data you need.

Also, UDF's do all sort of weird things with execution plans, like hiding the actual cost in a plan, and often removing the option to choose a parallel plan for the optimizer.

I wouldn't try to roll my own UDF for something like this.

The best way IMO would be to figure out a way to do it inline or set based.
I don't have any sample data and expected output to work with but I could imagine a

UPDATE [table]
SET [url] = REPLACE([url], 'base_url', 'new_url')
WHERE [url] LIKE '%<regex>%';

kind of construct

You might even do it in the application code (such as retrieving a datatable, fixing the data and saving the datatable again).

If you insist on doing it in a function I'd just either write a SQLCLR function or go and download the sqlsharp library and use that.

If you are worried about performance then read the article posted by the creator of the library comparing performance.

I highly doubt your implementation will work better or faster than a .NET regex solution.


Checking that a filename does not end with "Com.js" I'm fairly new to JavaScript  and I wonder if there is a way to write this function shorter and better?

   function checkFile(file){
        var cbEnab= true;
        if (file !== undefined && file !== null) {
            cbEnab = (!jQuery.endsWith(file, "Com.js"));
        }
        return cbEnab;
    }

By mechanical transformation, I arrive at:

function checkFile(file) {
    return file == null || !jQuery.endsWith(file, "Com.js");
}

The use of === strict equality checking is not necessary; == lumps null and undefined together.

I have my doubts about the whole function, though, as the function name gives no indication that it's going to check whether its argument ends in "Com.js".


Merge values of the objects properties with the same name into array I have such input:

[ { name: 'timezone', value: 'EST' },
  { name: 'interval', value: 'day' },
  { name: 'metrics[]', value: 1},
  { name: 'metrics[]', value: 2} ]

As you may already notice - these are parameters from POST request.

What I need to do is to get such output:

[ { name: 'timezone', value: 'EST' },
  { name: 'interval', value: 'day' },
  { name: 'metrics[]', value: [ 1, 2 ] },
  { name: 'metrics[]', value: [ 1, 2 ] } ]

My code does everything it needs to do, but I'm not sure if it is written in the optimal way, am I overlooking some bugs, etc.

arrayify = (params) ->
  arrayifiedParams = {}

  for param in params
    paramName = param.name

    arrayifiedParams[paramName] = arrayifiedParams[paramName] || []
    arrayifiedParams[paramName].push param.value

  params.map (param) ->
    paramName = param.name
    if arrayifiedParams[paramName].length > 1
      param.value = arrayifiedParams[paramName]
    param

Here is the corresponding JSFiddle.

Your code looks ok overall. There are perhaps a few CoffeeScript features you can make use of, but nothing major.

However, I don't like that the input array is modified in-place (i.e. the function has side-effects). Other code may hold references to the param objects already, so altering them is potentially dangerous.

I'd rather have the function return a new, separate array with the results, leaving the input array untouched.

Here's a version that does that, and also removes duplicate params (I called it coalesce because it sounds fancier):

coalesce = (params) ->
  coalesced = {}

  for param in params
    {name, value} = param
    coalesced[name] or= []
    coalesced[name].push value

  for own name, value of coalesced
    value = value[0] if value.length is 1
    {name, value}


SQL prepared statement to search songs by various text fields As we know, using PDO and prepared statements, binding a table or column using bindParam is not possible. So for this, I've figured out to use function parameters to do it by replacing the column names with the parameters, as shown in the example bellow.

function search($filter,$input,$type,$order,$sort,$limit){
    $st=$this->conn->prepare("SELECT Title,Type,Youtube,Score,Ratings,Singer,ID
    FROM song WHERE $filter LIKE CONCAT('%',?,'%')
    AND Type=? ORDER BY $order $sort LIMIT $limit");
    $st->bindparam(1,$input);
    $st->bindparam(2,$type);
    $st->execute();
    return $st->fetchall();
}

And I call this function here:

$result=$search->search($_POST['filter'],$_POST['input'],$_POST['type'],$_POST['order'],$_POST['sort'],$_POST['limit']);

Where the values come from:

$filter----select
$input----input
$type----radio
$order----select
$sort----radio
$limit----input

As you see the table I'm selecting from is fix, that's not optional, but I have the option to choose which column I want the result to be ordered by($order), which column I'd like to search($filter), I want it to be ascending or descending($sort), and how many I'd like to see($limit).

And this code works fine, but I'm not sure if it's a safe way to do this. Are there some kind of risks of this solution, or it's safe to leave it like this?

Are there any risks of using function parameters for column names in prepared statement?

Well, of course you shouldn't. "Function parameters" is not a protection measure in any sensible way. It's just a transport, to deliver some value into a function. As is.

  Are there some kind of risks of this solution,

ANY of them. All kinds of SQL injection are welcome in this code.

Sorry for the harsh preface but I wanted to make it clear and make sure there are no such illusions left.

So you have to change the approach entirely. Luckily, it is not hard to implement.

For the field name and  order by, you will need a white list filtering.
For the limit, as it's not a column name, but a data literal, simply add it through a placeholder.

I've got an article that explains the white list approach in detail, Adding a field name in the ORDER BY clause based on the user's choice

Using a helper function from this article,

function white_list(&$value, $default, $allowed, $message) {
    if (empty($value)) {
        return $default;
    }
    $key = array_search($value, $allowed, true);
    if ($key === false) {
        throw new InvalidArgumentException($message);
    }
    return $value;
}

we can make your function 100% safe

function search($filter,$input,$type,$order,$sort,$limit)
{
    $fields = ['name', 'text', 'whatever'];
    $filter = white_list($filter, $fields[0], $fields, "Incorrect filter name");
    $order  = white_list($order, $fields[0], $fields, "Incorrect order name");
    $sort   = white_list($sort, "ASC", ["ASC","DESC"], "Invalid ORDER BY direction");

    $sql = "SELECT Title,Type,Youtube,Score,Ratings,Singer,ID
    FROM song WHERE $filter LIKE CONCAT('%',?,'%')
    AND Type=? ORDER BY $order $sort LIMIT ?"
    $st = $this->conn->prepare($sql);
    $st->execute([$input, $type, $limit]);
    return $st->fetchall();
}

Just edit the list of fields allowed (or make two lists if they have to be different for the filter and order) and you're set.


Dynamic array class in c++ So, as an exercise I wanted to write a dynamic array, like std::vector. I wanted to know if there are any things that could be done better or anything I did wrong. Thank you!

    template<typename T>
    class myVector {
    using size_type = size_t;
    public:
    void pop(void) {
      changeSize(--m_Used);
    }

    void pop(size_type index) {
      if (!(index < m_Size)) return;
      changeSize(m_Used -= index);
    }

    bool push(const T& obj) {
      if (m_Used == m_Size) {
        bool temp = changeSize(++m_Used);
        m_Buffer[m_Used - 1] = obj;
        return temp;
      }
      m_Buffer[m_Used++] = obj;
      return true;
    }

    inline size_type used(void) const { return m_Used; }
    inline size_type size(void) const { return m_Size; }

    T& operator[](size_type index){
      return m_Buffer[index];
    }

    const T& operator[](size_type index) const{
      return m_Buffer[index];
    }

    inline T* begin(void) { return m_Buffer; }
    inline T* end(void) { return begin() + m_Used; }
    inline const T* const begin(void) const { return m_Buffer; }
    inline const T* const end(void) const { return begin() + m_Used; }

  public:
    bool reserve(size_type _size) {
      if (m_Buffer and !(m_Size < _size)) return 1;
      return changeSize(_size);
    }

    myVector(void) {
      m_Buffer = (T*)malloc(sizeof(T));
      m_Used = 0;
      m_Size = 1;
    }

    myVector(const myVector<T>& other) {
      *this = other;
    }

    myVector<T>& operator=(const myVector<T>& rhs) {
      if(rhs.m_Buffer){
        m_Used = rhs.m_Used;
        m_Size = rhs.m_Size;
        free(m_Buffer);
        m_Buffer = (T*)malloc(sizeof(T) * m_Size);
        memcpy(m_Buffer,rhs.m_Buffer,sizeof(T) * m_Size);
      }
      return *this;
    }

    template<size_type N>
    myVector(const T (&inputArr)[N]) {
      *this = inputArr;
    }

    template<size_type N>
    myVector<T>& operator=(const T (&inputArr)[N]) {
      m_Size = N;
      m_Used = m_Size;
      m_Buffer = (T*)malloc(m_Size * sizeof(T));
      memcpy(m_Buffer,inputArr, N * sizeof(T));
      return *this;
    }

    myVector<T>& operator=(const std::initializer_list<T>& rhs) {
      m_Size = rhs.size();
      m_Used = m_Size;
      m_Buffer = (T*)malloc(m_Size * sizeof(T));
      memcpy(m_Buffer,rhs.begin(),m_Size * sizeof(T));
      return *this;
    }

    myVector(const std::initializer_list<T>& other) {
      *this = other;
    }

    ~myVector() {
      free(m_Buffer);
    }

    bool contains(const T& obj) const {
        return std::find(begin(),end(),obj) != end();
    }

  private:
    T* m_Buffer;
    size_type m_Used;
    size_type m_Size;

    bool changeSize(size_type _size) {
      T* newBuffer = (T*)realloc(m_Buffer, sizeof(T) * _size);
      if (newBuffer) {
        m_Buffer = newBuffer;
        m_Size = _size;
        return true;
      }
      return false;
    }
};

Overall
Please fix the formatting.
It's hard to find the public/private sections that way this is formatted.

A fundamental miss of your dynamic array is that objects are not correctly created or destroyed.
You use malloc() and realloc() to allocate memory. But these functions do not call the constructors on class type objects. Thus the memory is not correctly initialized to hold objects of type T, it is undefined behavior to use these objects until they have been correctly initialized (with a constructor).
Conversely you also need to call the destructor when the object is popped or the destructor for your vector is called.
As a result your code is fundamentally broken.

You have not correctly implemented the rule of three.
Please look it up. But basically if your object owns a pointer you need to overide the compiler implementation of several functions correctly.
As a result your code is fundamentally broken.

You have not thought about move semantics.
Since C++11 (9 years ago). C++ has had the concept of move semantics that allow objects to be moved rather than copied. In the worst case this is no worse than a copy but in the best case it is much better than a copy and makes classes a lot more efficient if you start passing them around (eg return values).
This is a nice to have but to be honest expected especially for containers in modern code.

Code Review
This line:
    using size_type = size_t;

means that somewhere (not included) you have the line:
using namespace std;

This is a problem for anything but toy code. Please stop adding this line to your code. Read any other C++ review.
See: Why is “using namespace std;” considered bad practice?

I have no problem you not checking that m_Used is not zero here.
    void pop(void) {
      changeSize(--m_Used);
    }

But it is inconsistent when you do check it here.
    void pop(size_type index) {
      if (!(index < m_Size)) return;
      changeSize(m_Used -= index);
    }

Choose one. Either always check or never check. The std::vector<> does not check. But provides the appropriate functions that allow the user of the vector to validate the state so it can be called safely.

This is not enough:
      changeSize(--m_Used);

If you remove an object from the vector you need to destroy it. This means calling its destructor. Classes of type T may depend on the constructor/destructor being called.
You can call it manually.
      m_Buffer[m_Used].~T();
      changeSize(--m_Used);

Why are you calling changeSize() in the pop?
The whole point of having a m_Used and m_Size members is that you may have unused space in your vector that you can utilize on the next push without having to re-size the vector on every single operation!

Conversely on the push. You can simply call the assignment operator.
        m_Buffer[m_Used - 1] = obj;

Since you allocated the space with malloc() or realloc() this space has not been correctly initialized with the constructor thus calling the assignment operator on this object is not valid.
You can call the constructor by using placement new
        new (m_Buffer + m_Used) T(obj);  // Call the copy constructor
                                         // using placement new
        ++m_Used;

Don't use the inline keyword unless you need to.
    inline size_type used(void) const { return m_Used; }
    inline size_type size(void) const { return m_Size; }

Here you don't need to. It does not force the compiler to actually "inline" the code (it actually does nothing on modern compilers as the compiler engineers realized long ago the programmers are bad and the compiler is much better at making that decision).

Yes perfectly good.
    inline T* begin(void) { return m_Buffer; }
    inline T* end(void) { return begin() + m_Used; }
    inline const T* const begin(void) const { return m_Buffer; }
    inline const T* const end(void) const { return begin() + m_Used; }

Though personally I would have abstracted the iterator.
    using iterator = T*;
    using const_iterator = T const*;

The other thing to think about is that the standard containers contain a couple of additional iterator methods.
    const_iterator cbegin() const;  // Notice the 'c'
    const_iterator cend()   const;

Also reverse iterators are worth using.
    <something>       rbegin();
    <const something> crbeign() const;
    // etc

Using the keyword and seems weird.
      if (m_Buffer and !(m_Size < _size)) return 1;

Yes its valid. But you need to add a special header. Also most engineers are simply used to reading &&.

Constructors at last.
Would be nice to put constructors/destructors/assignment near the top. This the first thing you need to know about a class (also the first thing people in code review want to check).
    // Sure this is fine.
    // But only space for one item?
    // Why not pre-allocate 10 so you don't need to reallocate on each push.
    // You don't need void here.
    myVector(void) {
      m_Buffer = (T*)malloc(sizeof(T));
      m_Used = 0;
      m_Size = 1;
    }

    // Normally the assignment operator is written in terms of the
    // copy constructor. You seem to have done this the complete other
    // way around. But it also mens that you should initialize all the
    // members before you call the assignment operator (it is assuming
    // you are using a fully formed object not one with random values)
    //
    // Go look up the copy and swap idiom.
    myVector(const myVector<T>& other) {
      *this = other;
    }

    // What about self assignment.
    // It is perfectly valid to assign to yourself. Your code
    // needs to work in this situation. If the rhs has a null
    // buffer why does nothing change?
    //
    // See Copy and swap idiom.
    myVector<T>& operator=(const myVector<T>& rhs) {
      if(rhs.m_Buffer){
        m_Used = rhs.m_Used;
        m_Size = rhs.m_Size;

        // This frees your buffer.
        // But what happens to all the objects of type T.
        // You just leaked them all without calling there destructor.
        free(m_Buffer);

        // What happens if this fails?
        // Well with strong exception safety you would not change the
        // current object and throw an exception. But you have lost
        // all the old state so if you throw you are in a bad state.
        m_Buffer = (T*)malloc(sizeof(T) * m_Size);
        memcpy(m_Buffer,rhs.m_Buffer,sizeof(T) * m_Size);
      }
      return *this;
    }

This is how your constructors should look (approximately).
    // Please not the use of initializer list.
    // I am using malloc here because you did (but you can do better).
    myVector()
        : m_buffer(malloc(sizeof(T) * 10))
        , m_Used(0)
        , m_Size(10)
    {}

    // Copy constructor.
    myVector(myVector const& copy)
        : m_buffer(malloc(sizeof (T) * copy.m_Used)
        , m_Used(0)           // We will update this in the code.
        , m_Size(copy.m_Used)
    {
        for(int loop = 0; loop < m_Used; ++loop) {
            push(copy.buffer[loop]);
        }
    }

    // Copy Assignment
    // Use Copy and Swap Idiom (look it up)
    myVector& operator=(myVector const& rhs)
    {
        myVector copy(rhs);   // safely make a copy.
        swap(copy);           // swap the copy and the current object.
        return *this;
    }                         // Note copy destroyed here.
                              // Thus the old array is deleted.

    // Destructor
    ~myVector()
    {
        clear();             // delete all the objects.
        free(m_buffer);
    }

    // Swapping
    void swap(myVector& other) noexcept
    {
         using std::swap;
         swap(m_buffer,    other.m_buffer);
         swap(m_Used,      other.m_used);
         swap(m_Size,      other.m_size);
    }
    friend void swap(myVector& lhs, myVector& rhs)
    {
         lhs.swap(rhs);
    }

    // Move semantics are relatively simple:
    myVector(myVector&& move) noexcept
        : m_buffer(nullptr)
        , m_Used(0)
        , m_Size(0)
    {
        swap(move);
    }
    myVector& operator(myVector&& move) noexcept
    {
        clear();
        swap(move);
        return *this;
    }

    // Now just implement push/clear

Not advisable to do this!!
    template<size_type N>
    myVector(const T (&inputArr)[N]) {
      *this = inputArr;
    }

--
You can't use memcpy() on T if it has a constructor.
      memcpy(m_Buffer,inputArr, N * sizeof(T));

In C++ this is most types. So this is going to fail to correctly construct these objects.

Self Plug
I wrote a couple of articles on how to write a vector. Please have a read. It goes through everything I have said here and then dives into some more detail.
Resource Management Allocation
Resource Management Copy Swap
Resize
Simple Optimizations
The Other Stuff
ReSize Maths


Find largest binary gap of a number N I have picked up a problem from Codility website and tried to work it out.

A binary gap within a positive integer N is any maximal sequence of consecutive zeros that is surrounded by ones at both ends in the binary representation of N.

Write a function:

class Solution { public int solution(int N); }

that, given a positive integer N, returns the length of its longest binary gap. The function should return 0 if N doesn't contain a binary gap.

I would appreciate guidance of how to refactor it and what to look for in terms of efficiency?

Optimisation I'd like to achieve would be, less type conversion, no zero items in the list, would there be a better way to find the max number in a list, or .Sort(); is good enough?

Any ideas welcome, thank you.

using System;
using System.Collections.Generic;

namespace rec
{
    class MainClass
    {
        public static void Main(string[] args)
        {
            Console.WriteLine(FindGap(777)); //
        }

        public static int FindGap(int number)
        {
            string toBinary = Convert.ToString(number, 2);

            char[] toChar = toBinary.ToCharArray();

            string previous = "";
            string current = "";
            int count = 0;
            int answer = 0;
            List<int> lastGap = new List<int>();

            for (int i = 0; i < toChar.Length; i++)
            {
                if (toChar[i].ToString() == "1")
                {
                    // temp becomes 1
                    current = toChar[i].ToString();

                    if (current == "1")
                    {
                        lastGap.Add(count);
                        count = 0;
                        continue;
                    }
                }
                if (toChar[i].ToString() == "0" && current == "1")
                {
                    current = "0";

                    count++;

                    previous = current;
                }
                else if (toChar[i].ToString() == "0" && previous == "0")
                {
                    current = "0";
                    count++;
                    previous = current;
                }
            }
            lastGap.Sort();
            answer = lastGap.Last();

            if (answer > 1)
            {
                return answer;
            }
            else return 0;
        }
    }
}

If you're just looking for the longest stretch of zeros you don't need to use a list at all. You just iterate through the binary representation until you find a stretch of zeros. If you hit a new high score, you can overwrite the old high score with the new one and keep moving forward, forgetting about the past.

This is called a "greedy algorithm."

    public static int FindGap(int number)
    {
        string binaryRep = Convert.ToString(number, 2);

        int maxGapSize = 0;
        int currGapSize = 0;

        foreach(char ch in binaryRep)
        {
            if (ch == '0')
            {
                currGapSize++;
            }
            else
            {
                maxGapSize = Math.Max(currGapSize, maxGapSize);
                currGapSize = 0;
            }
        }

        return maxGapSize;
    }

If you want to get really fancy, you don't even need to convert the number to a string. You can use bit shifts to "iterate" through the bits in the number. The number 9 is 1001 in binary. Shift the bits over one, and you've got 100. Shift it over again, and you've got 10. You can use modulus operations to see if the least significant digit is 0 or 1.

This is just a fancy way to repeatedly divide by 2. In fact, you could solve this problem by dividing by 2 instead but this way works as well. (There's also probably a more terse way to write this, but this is perfectly efficient.)

    public static int FindGap2(int number)
    {
        int maxGapSize = 0;
        int currGapSize = 0;

        if (number == 0)
            return 0;

        while (number % 2 == 0)
            number = number >> 1;

        while (number > 0)
        {
            if(number%2 == 0)
            {
                currGapSize++;
            }
            else
            {
                maxGapSize = Math.Max(currGapSize, maxGapSize);
                currGapSize = 0;
            }
            number = number >> 1;
        }

        return maxGapSize;
    }


Simple JS XMLHttp wrapper class I was recently in a situation where I had to use vanilla js (no es6 and no http libraries, so I came up with this simple wrapper for get calls :

var Http = (function() {
    function Http() {
        this.xhr = new XMLHttpRequest();
    }
    Http.prototype.get = function(url, onload) {
        this.xhr.onreadystatechange = function() {
            if (this.readyState === 4 && this.status == 200) {
                onload(this.responseText);
            }
        }
        this.xhr.open("GET", url);
        this.xhr.send();
    }
    return Http;
})()

And I would use it like this :

var http = new Http();
http.get('endpoint', function() {});

Can you see any potential problems with this approach like memory leaks insecurities etc. and would you and if possible, how, add something to improve it, like ability to handle multiple get() in row from the same http object?

I am concerned that you have added a layer of false generalization around a very specific, usually one-time use, request object.  I would think the class would be better named Request or similar to better clarify what this wrapper represents as currently stands.

If your intent is to truly define a class that provides generalized request functionality ( get, post, put, etc.) then you should think more closely about how (and if) the class holds state for individual request objects. You probably would not store a common request object for every request on ‘this’, but rather would create a new request object for every method call.   If you find yourself no longer holding request state in your class, then you then would probably consider whether and instantiable class is really what you want.

On a different front, your current code really does nothing to handle errors in the request. If you are spending the time to build a common request abstraction like this, you should add appropriate error handling.

Finally, if you are in the business of building a new request abstraction, then I would strongly consider building of the much more modern and now widely supported Fetch API.


Querying all records from one table, add column from another with foreign key constraint Apologies for the cryptic title, I don't quite know how to title this question. I'm asking here rather than StackOverflow since I've got a working query, I'm just wondering if it can be improved.

Anyway, I've got three tables, administration, mapping and used_mappings. Administration holds details of individual financial administrations, mapping is a "static" table holding details for all available financial mappings, used_mappings holds records of what mappings are used by what administration (not every administration uses every mapping). The used_mappings table also has a column, custom_name, so the columns look as follows;

Administration

| id | name    |
|----|---------|
| 1  | Admin 1 |
| 2  | Admin 2 |

Mapping

| id | name      |
|----|-----------|
| 1  | Mapping 1 |
| 2  | Mapping 2 |
| 3  | Mapping 3 |
| 4  | Mapping 4 |
| 5  | Mapping 5 |

Used mapping

| id | administration_id | mapping_id | custom_name       |
|----|-------------------|------------|-------------------|
| 1  | 1                 | 1          | Revenue           |
| 2  | 1                 | 3          | Cost of sales     |
| 3  | 2                 | 1          | Bar revenue       |
| 4  | 2                 | 3          | Cost of bar sales |
| 5  | 2                 | 4          | Wages             |

I now need to fetch all records from mapping, regardless of them being used, and where a custom_name is defined in used_mappings for a specific administration, add it on as an extra column (and have custom_name in the results be null if no custom_name exists.)

I first tried a simple LEFT JOIN;

SELECT  M.id,
        M.name,
        U.custom_name as customName
FROM    mapping M
LEFT JOIN used_mapping U ON U. = M.id
WHERE U.administration_id = 2;

And although it does fetch the correct custom_names, because of the WHERE U.administration_id = 1; it only fetches rows from used_mapping where that condition is met. In other words, in this example it fetches

| id | name         | customName        |
|----|--------------|-------------------|
| 1  | Mapping 1    | Bar revenue       |
| 3  | Mapping 3    | Cost of bar sales |
| 4  | Mapping 4    | Wages             |

rather than

| id | name      | customName        |
|----|-----------|-------------------|
| 1  | Mapping 1 | Bar revenue       |
| 2  | Mapping 2 | null              |
| 3  | Mapping 3 | Cost of bar sales |
| 4  | Mapping 4 | Wages             |
| 5  | Mapping 5 | null              |

I've got it working with a slightly more complex LEFT JOIN;

SELECT  id,
        name,
        custom_name
    FROM mapping
AS M
LEFT JOIN (
    SELECT custom_name,
            mapping_id
    FROM used_mappings
    WHERE administration_id = 2
) AS U ON U.mapping_id = M.id;

but it takes twice as long compared to fetching all rows from mapping (obviously because for every record in mapping, it runs a query on used_mappings as well). Hence my question, is there a way of making the query more efficient?

SELECT  M.id,
        M.name,
        U.custom_name as customName
FROM    mapping M
LEFT JOIN used_mapping U ON U. = M.id
WHERE U.administration_id = 2;

The problem is in the where statement. Here it checks whether the administration_id = 2. For mappings where there is no match in used_mapping this id will be null. So it gets filtered out. Essentially you're performing an inner join here.

Instead you can also allow rows that have administration_id is null:

SELECT  M.id,
        M.name,
        U.custom_name as customName
FROM    mapping M
LEFT JOIN used_mapping U ON U. = M.id
WHERE U.administration_id = 2 or U.administration_id IS NULL;


Should a Factory ever be generated per request? I'm working with AutoFac to do some DI. I think I've got a decent grip on things, but just ran into a question I had with my code and thought I'd check:

public class AutofacRegistrations
{
    public static void RegisterAndSetResolver()
    {
        // Create the container builder.
        var containerBuilder = new ContainerBuilder();

        // Register the Web API controllers.
        containerBuilder.RegisterApiControllers(Assembly.GetExecutingAssembly());

        //  Only generate one SessionFactory ever because it is expensive.
        containerBuilder.Register(x => new NHibernateConfiguration().Configure().BuildSessionFactory()).SingleInstance();

        //  Everything else wants an instance of Session per HTTP request, so indicate that:
        containerBuilder.Register(x => x.Resolve<ISessionFactory>().OpenSession()).InstancePerApiRequest();
        containerBuilder.Register(x => LogManager.GetLogger(MethodBase.GetCurrentMethod().DeclaringType)).InstancePerApiRequest();

        //  TODO: I don't THINK I should actually be making a DAO factory per API request. I think the whole point of a Factory is to make them once.
        containerBuilder.RegisterType<NHibernateDaoFactory>().As<IDaoFactory>().InstancePerApiRequest();
        containerBuilder.RegisterType<StreamusManagerFactory>().As<IManagerFactory>().InstancePerApiRequest();

        // Build the container.
        ILifetimeScope container = containerBuilder.Build();

        // Create the depenedency resolver.
        var dependencyResolver = new AutofacWebApiDependencyResolver(container);

        // Configure Web API with the dependency resolver.
        GlobalConfiguration.Configuration.DependencyResolver = dependencyResolver;
    }
}

You can see that I am registering an NHibernateDaoFactory and a StreamusManagerFactory as InstancePerApiRequest. This doesn't feel right to me because I think creating a Factory is supposed to be a relatively expensive endeavour and they should probably be .SingleInstance() similiar to the ISessionFactory.

However, my NHibernateDaoFactory has a dependency on Session:

...

public NHibernateDaoFactory(ISession session)
{
    if (session == null) throw new NullReferenceException("session");

    Session = session;
}

public IErrorDao GetErrorDao()
{
    return ErrorDao ?? (ErrorDao = new ErrorDao(Session));
}

....

and my ManagerFactory has a dependency on the DaoFactory:

...

public StreamusManagerFactory(ILog logger, IDaoFactory daoFactory)
{
    if (logger == null) throw new NullReferenceException("logger");
    if (daoFactory == null) throw new NullReferenceException("daoFactory");

    Logger = logger;
    DaoFactory = daoFactory;
}

public IErrorManager GetErrorManager()
{
    return ErrorManager ?? (ErrorManager = new ErrorManager(Logger, DaoFactory.GetErrorDao()));
}

...

This leaves me having to create new factories per request. Is this correct?

Just got one thing to say on your code:

Your Comments add close to no value!

your code is littered with comments and empty lines

//Create the Container Builder
var containerBuilder = new ContainerBuilder();

The comment is in fact almost exactly the same as the code below. And it's not only there, it's the same with every comment in the AutofacRegistrations you posted.

Other than that, if your factory has a dependency on Session you maybe did something wrong. Instead you could try to pass the Session to the already instantiated singleton factory, when calling the GetErrorDao() Function.

That would instead look something like that:

public IErrorDao GetErrorDao(Session session)
{
     return ErrorDao ?? (ErrorDao = new ErrorDao(session));
}

this would decouple both your NHibernateDaoFactory and your ManagerFactory and let them be free to use a singleton pattern instead. Which in turn greatly reduces the stress your application puts on the system.

You'd need to cascade this up the Dependency Chain, but that's a small price to pay to have a decoupled Factory Instance for all Requests / Sessions instead of one for each Session.
You'd thus need to modify your GetErrorManager too:

public IErrorManager GetErrorManager(Session session)
{
    return ErrorManager ?? (ErrorManager = new ErrorManager(Logger, DaoFactory.GetErrorDao(session)));
}


Numbers, pairs, and a difference Problem statement:

Write a function which returns the total number of combinations whose difference is k.
For example, 4 8 15 16 23 42 and the difference 7 has 2 pairs, 8, 15 and 16, 23.

I liked the Finding number of number pairs with given difference question so much (despite its brokenness) that not only did I review it, but I also decided to write up my own implementation of it (following the approach I described in my review). I was intrigued by this comment:

You need the handle the case of elements being equal too, though. That means an array with 2x duplicates such as 4 4 8 8 15 15 16 16 23 23 42 42 should return 4x the number of combinations as without duplicates. - JS1

When a pair is found, I count how many numbers are matching at lowIndex and highIndex respectively, and then calculating how many pairs there are.
NumberPairsDiff.java
public class NumberPairsDiff {

    public static int pairsWithDifference(int[] input, long difference) {
        int[] sorted = Arrays.copyOf(input, input.length);
        Arrays.sort(sorted);

        int pairsFound = 0;

        int lowIndex = 0;
        int highIndex = 0;

        while (lowIndex < sorted.length && highIndex < sorted.length) {
            long current = (long) sorted[highIndex] - sorted[lowIndex];
            if (current < difference) {
                highIndex++;
            } else if (current > difference) {
                lowIndex++;
            } else {
                // Find out how many values are equal at `highIndex`
                int highMatching = findEqual(sorted, highIndex);
                highIndex += highMatching;

                if (difference == 0) {
                    // 1+2+3+4+5+...+n = 0.5x^2 + 0.5x
                    int add = (int)(0.5f * highMatching * highMatching + 0.5 * highMatching);
                    pairsFound += add;
                    lowIndex += highMatching;
                } else {
                    // Find out how many values are equal at `lowIndex`
                    int lowMatching = findEqual(sorted, lowIndex);
                    lowIndex += lowMatching;
                    pairsFound += lowMatching * highMatching;
                }
            }
        }

        return pairsFound;
    }

    private static int findEqual(int[] sorted, int index) {
        int matching = 1;
        while (index < sorted.length - 1 && sorted[index] == sorted[index + 1]) {
            index++;
            matching++;
        }
        return matching;
    }

}

Obligatory parametrized unit test, NumberPairsDiffTest.java:
@RunWith(Parameterized.class)
public class NumberPairsDiffTest {

    @Parameterized.Parameters
    public static Collection<Object[]> data() {
        return Arrays.asList(new Object[][]{
                {new int[]{ 4, 4, 8, 8, 15, 15, 16, 16, 23, 23, 42, 42 }, 4, 4},
                {new int[]{ 4, 4, 8, 8, 15, 15, 16, 16, 23, 23, 42, 42 }, 7, 8},
                {new int[]{ 4, 4, 8, 8, 15, 15, 16, 16, 23, 23, 42, 42 }, 0, 18},
                {new int[]{ 1 }, 0, 1},
                {new int[]{ 2, 2 }, 0, 3},
                {new int[]{ 3, 3, 3 }, 0, 6},
                {new int[]{ 4, 4, 4, 4 }, 0, 10},
                {new int[]{ 2, 2, 4 }, 0, 4},
                {new int[]{ 1, 4, 7, 11, 14, 17 }, -3, 4},
                {new int[]{ 1, 4, 7, 11, 14, 17 }, 5, 0},
                {new int[]{ Integer.MIN_VALUE, Integer.MAX_VALUE }, (long)(Integer.MAX_VALUE) * 2L + 1L, 1},
        });
    }

    @Parameterized.Parameter(value = 0)
    public int[] input;

    @Parameterized.Parameter(value = 1)
    public long diff;

    @Parameterized.Parameter(value = 2)
    public int expected;

    @Test
    public void test() {
        String string = String.format("Array %s with diff %d", Arrays.toString(input), diff);
        assertEquals(string, expected, NumberPairsDiff.pairsWithDifference(input, diff));
    }

}

Primary concerns

Readable, understandable code?
Missed edge-cases?
Better approaches?

The basic algorithm you use is logical, and well presented. The general style is standard, and I see no issues there. I believe the algorithm can be changed slightly to make the comparison code simpler, but the performance will still be comparable.
I looked for edge cases I would have expected, specifically relating to large differences, but you appear to have covered that well. Unfortunately, your pair-counts for diff == 0 are all off. For example, your third test case:

{new int[]{ 4, 4, 8, 8, 15, 15, 16, 16, 23, 23, 42, 42 }, 0, 18},

should have a count of 6, and not 18. There are only 6 pairs with difference 0. The pair at index 0, and 1, is the same pair as index 1 and 0, so you can't double (or, for some reason triple) count them.
This also applies to cases like: {new int[]{ 1 }, 0, 1},.... you have an input array of just a single element... but you have 1 pair with diff 0? Really???
There is one small performance improvement related to how far you need to iterate in the loop - you have: while (lowIndex < sorted.length && highIndex < sorted.length) { but that can be just while (highIndex < sorted.length) { since lowIndex is always less than or equal to highIndex. This is particularly confusing if your input difference is negative.
For some reason, and I can't explain it, I prefer left and right for those variable names too... lowIndex seems not-quite-right, but I understand it anyway. Just a niggly thing.
Additionally, there are a few places where I would do things in a more Java-8 way. Let's deal with the smaller items first...
Small things
Consider this code:

int[] sorted = Arrays.copyOf(input, input.length);
Arrays.sort(sorted);

I would (now) use the following:
int[] sorted = IntStream.of(input).sorted().toArray();

As for negative difference input values, I would just handle that as an up-front special condition. The count of pairs with a negative difference will be the same as the count with a positive difference and the same magnitude. Just convert negative difference to positive, and handle the Long.MIN_VALUE special case.
Algorithm
I would alter the algorithm a little. I would have a simple scan of the data to count value duplicates, and store that count in a separate array. With that data structure, the search for duplicates later is much easier. Consider this:
    if (input.length <= 1) {
        return 0;
    }

    int[] sorted = IntStream.of(input).sorted().toArray();

    // remove and count duplicate values
    int[] counts = new int[sorted.length];
    int size = 0;
    for (int i = 0; i < sorted.length; i++) {
        if (sorted[i] != sorted[size]) {
            sorted[++size] = sorted[i];
        }
        counts[size]++;
    }
    size++;

The above code ends up with a simpler data set to navigate. Unfortunatley, it makes the case where diff==0 a little more complicated... you need to use factorials for that, and not simple products. (Edit, I extracted that logic completely)....
Still, I put the whole method together as:
import java.util.stream.IntStream;

public class NumberPairsDiff {

    public static int pairsWithDifference(int[] input, long difference) {

        if (input.length <= 1) {
            return 0;
        }

        if (difference == Long.MIN_VALUE) {
            return 0;
        }

        if (difference < 0) {
            difference = -difference;
        }

        int[] sorted = IntStream.of(input).sorted().toArray();

        // remove and count duplicate values
        int[] counts = new int[sorted.length];
        int size = 0;
        for (int i = 0; i < sorted.length; i++) {
            if (sorted[i] != sorted[size]) {
                sorted[++size] = sorted[i];
            }
            counts[size]++;
        }
        size++;

        if (difference == 0) {
            int pairsFound = 0;
            for (int i = 0; i < size; i++) {
                pairsFound += factorial(counts[i] - 1);
            }
            return pairsFound;
        }

        int left = 0;
        int right = 0;
        int pairsFound = 0;

        while (right < size) {
            long diff = (long)sorted[right] - (long)sorted[left];
            if (diff < difference) {
                right++;
            } else if (diff > difference) {
                left++;
            } else {
                pairsFound += counts[left++] * counts[right++];
            }
        }
        return pairsFound;

    }

    private static int factorial(int n) {
        int f = n;
        while (--n > 1) {
            f *= n;
        }
        return f;
    }

}

The above function fails a bunch of your test cases because it produces the correct results ;-)


Bash Database Backup to AWS S3 I've written a bash script that backs up databases and uploads the backups to an S3 bucket. I have the bucket configured to delete backups that are older than 90 days.

Is there anything that can be improved in this script?

#!/usr/bin/env bash

# Backup MySQL databases to S3

# Follow the installation instructions listed on the user guid doc:
# http://docs.aws.amazon.com/cli/latest/userguide/installing.html

# Note backups will be deleted after 90 days. This is setup in the management tab:
# https://s3.console.aws.amazon.com/s3/buckets/backups.databases/?tab=management

# DB details
DB_USER=""
DB_PASSWORD=""
DB_HOST=""
DB_IGNORED=(Database mysql information_schema performance_schema cond_instances)

# S3 details
S3_BUCKET=backups.databases
S3_FOLDER=backups/databases

# Temp storage location
TMP_DIR=/tmp

# Check if dependencies exist
dependencies=(aws mysql mysqldump tar rm echo)
for dependency in "${dependencies[@]}"
do
    if [[ $(command -v "$dependency") = "" ]]
    then
        # Tell the user that the dependency is not installed, then exit the script
        $(which echo) "Dependency $dependency isn't installed. Install this dependency to continue."
        exit 0
    fi
done

DB_DATABASES=($($(which echo) "show databases;" | $(which mysql) -h ${DB_HOST} -p${DB_PASSWORD} -u ${DB_USER} 2> /dev/null))

# Filename for the backups
time=$($(which date) +%b-%d-%y-%H%M)
filename="backup-$time.tar.gz"

for database in "${DB_DATABASES[@]}"
do
    # Skip ignored databases
    if [[ "${DB_IGNORED[@]}" =~ "$database" ]]
    then
        continue
    fi

    # Dump database SQL schema to an SQL file stored in the temp directory
    $(which echo) "Backing up $database..."
    $(which mysqldump) -h ${DB_HOST} -u ${DB_USER} -p${DB_PASSWORD} ${database} > "$TMP_DIR/$database.sql" 2> /dev/null
    $(which tar) -cpzf "$TMP_DIR/$database-$filename" "$TMP_DIR/$database.sql" 2> /dev/null

    # Upload the exported SQL file to S3
    $(which echo) "Uploading to S3..."
    $(which aws) s3 cp "$TMP_DIR/$database-$filename" "s3://$S3_BUCKET/$S3_FOLDER/$database-$filename"
    $(which echo) "Database $database uploaded to S3"

    # Remove the temporary files
    $(which echo) "Cleaning up..."
    $(which rm) -f "$TMP_DIR/$database-$filename"
    $(which rm) -f "$TMP_DIR/$database.sql"
done

I had to add 2> /dev/null  to the end of the mysql and mysqldump commands due to a warning message showing.

  Warning: Using a password on the command line interface can be
  insecure.

Is there a better way of handling this?

Any feedback would be greatly appreciated.

is there a reason for $(which foo) ?

If foo is in your path, it will be found, if not which will give you an error.

Expressions like command will have a return code, so do not write

if [[ $(command -v "$dependency") = "" ]]

just use

if command -v "$dependency" > /dev/null
then ...


Change the background color of each section of the page when the user scrolls I want to change the background color of each section of the page when the user scrolls. The code works but I feel like it's too inefficient

var colors = ['blue','yellow','green','purple','brown'];
var myScope;
var sections = [$('#section1'),$('#section2'),$('#section3'),$('#section4'),$('#section5')];
var sectionTop = [$('#section1').offset().top, $('#section2').offset().top, $('#section3').offset().top, $('#section4').offset().top, $('#section5').offset().top];
var zeus;

$(window).scroll(function () {
myScope = $(window).scrollTop() + 300;

    if (myScope >= sectionTop[0]  && myScope < sectionTop[1]) {
    sections[0].css('background-color', colors[0]);
    } else {
    sections[0].css('background-color', 'white');
    };

    if (myScope >= sectionTop[1]  && myScope < sectionTop[2]) {
    sections[1].css('background-color', colors[1]);
    } else {
    sections[1].css('background-color', 'white');
    };

    if (myScope >= sectionTop[2]  && myScope < sectionTop[3]) {
    sections[2].css('background-color', colors[2]);
    } else {
    sections[2].css('background-color', 'white');
    };

    if (myScope >= sectionTop[3]  && myScope < sectionTop[4]) {
    sections[3].css('background-color', colors[3]);
    } else {
    sections[3].css('background-color', 'white');
    };

    if (myScope >= sectionTop[4] ) {
    sections[4].css('background-color', colors[4]);
    } else {
    sections[4].css('background-color', 'white');
    };

});

I also tried to replace all those "if" with a "for" loop but it doesn't work, it doesn't pop any errors or anything, it just doesn't work.

for (var i = 0; i < sections.length-1; i++) {
        var j = i++;
        if (myScope >= sectionTop[i] && myScope < sectionTop[j]) {
        sections[i].css('background-color', colors[i]);
        } else {
        sections[i].css('background-color', 'white');
        };
    };

Heres your code slimmed down, the problem you were having with the loop was that you weren't getting offset().top of your sectionTop[i].

var myScope = zeus = false;
var colors = ['blue','yellow','green','purple','brown'];
var sectionId = '#section';
var sections = 5;

$(window).on('scroll', function() {
myScope = $(window).scrollTop() + 300;

for (var i = sections; i >= 0; i--) {
   var sid = i > 0 ? i : 1;
   var css = 'white';
   if(myScope >= $(sectionId + sid).offset().top
   && myScope < $(sectionId + sid).offset().top) {
    css = colors[i];
   }

   $(sectionId + sid).css('background-color', css);
}

});


Creating an adjacency list in C++ I want to create an adjacency list based on data I get from a text file. Could I make this simpler?

#include <iostream>
#include <fstream>
#include <sstream>

using namespace std;

struct Vertex;
struct Edges;

struct Edges
{
    // edges link the vertex
    Vertex * linkTo;
    // pointer to vertex node
    Edges * next;
};

struct Vertex
{
    int parent;
    int child1;
    int child2;
    int child3;
    int child4;
    int child5;
    int child6;
    int child7;
    int child8;
    Vertex * next;
    Edges * Links;
    // pointer to edges list
    void Add_data(Vertex * node);
};

class Graph
{
    public:
      Vertex vertices;
    // call the vertex struct to the Graph
    private:
      void BFS();
};

//-------------------------------- Function Implementation--------------------------

void Add_data(Vertex * node)
{
    ifstream file("Table.txt");
    string line;
    int parent;
    int child1;
    int child2;
    int child3;
    int child4;
    int child5;
    int child6;
    int child7;
    int child8;

    while(getline(file,line))
    {
        istringstream data(line);
        if(!(data>>parent>>child1>>child2>>child3>>child4>>child5>>child>>6>>child7>>child8))
        {
            cout << "Error " <<endl;
        }
        else
        {
            node->parent = parent;
            node->child1 = child1;
            node->child2 = child2;
            node->child3 = child3;
            node->child4 = child4;
            node->child5 = child5;
            node->child6 = child6;
            node->child7 = child7;
            node->child8 = child8;
            node->next = NULL;
        }
    }
}

You don't have enough comments, and the comments that you have don't explain anything. Comments are supposed to explain why the code is the way it is (in other words: what the goal is), not restate the obvious. Everyone who knows graph theory knows that edges link vertexes, and everyone who knows C knows that something typed as Vertex * is a pointer to a vertex. Those comments have no point and should be removed.
Where's the implementation of Graph::BFS()?
Where's the code that calls ::Add_data?
What is ::Add_data even supposed to do? It appears to only support one-line files; was that your intention? Is it the same function as Vertex::Add_data?
What's the point of the linked list, when it never gets initialized?
Whenever you're manually repeating something, either by typing similar stuff over and over again, or by copy-pasting, you're doing something wrong. Computers are good at automating repetitive tasks. When you write code, you should think of writing loops. So code like

if(!(data>>parent>>child1>>child2>>child3>>child4>>child5>>child>>6>>child7>>child8))

should never appear (besides the obvious syntax error of the spurious >> between child and 6).


Summing all primes below 2,000,000 - Project Euler #10 in Assembly I am currently learning assembly for university, and I'd like to hear some feedback concerning what I've written.
Currently I have implemented Problems 1, 2, 3 and 10 from ProjectEuler. The goal is to sum all primes ranging from 0 to 2,000,000.

For problem 10 I use the formula 6n +- 1 to check for primes. To actually check them, I test divisors 2, 3, 5, 7, ... I'm currently only using the general purpose registers of the x64 architecture.

Do you have any feedback concerning this code? Can I do something better? Optimize something?

I use NASM in x64 mode.
This is my current implementation:

; CurrentPrime          Current prime to test
; Sum               Sum
; CurrentN              n of 6n +- 1
; PrimeMode             Wether we're doing + or - 1 (0 means -)
%define Sum r9
%define CurrentPrime r8
%define CurrentN r10
%define PrimeMode r12

section .text
global main
main:
    mov Sum, 5 ; 2+3 not included in 6n +- 1
    mov CurrentN, 0 ; This gets inc'ed below

loop:

    ; Increase n
    inc CurrentN

    ; Break every 10000 Ns for debugging reasons
    mov rax, CurrentN
    mov rbx, 10000
    mov rdx, 0
    div rbx
    cmp rdx, 0
    jne _debug_next
    nop ; Break here
_debug_next:

    ; multiply it by 6 and save it to CurrentPrime
    mov rax, CurrentN
    mov rbx, 6
    mul rbx
    mov CurrentPrime, rax

    ; Start with 6n-1
    dec CurrentPrime

    ; Reset registers for testing
    mov PrimeMode, 0
    ; This is our divisor
    mov rcx, 2

next_prime_test:

    ; When the number to test is >= 2000000, then exit
    cmp CurrentPrime, 2000000
    jge finish

    ; When divisor_to_test > (number_to_test/2), then this is prime
    mov rax, CurrentPrime
    mov rbx, 2
    mov rdx, 0
    div rbx
    cmp rax, rcx
    jl is_prime

    ; Test divisor for divisor
    mov rax, CurrentPrime
    mov rbx, rcx
    mov rdx, 0
    div rbx
    cmp rdx, 0
    je check_next_prime

    ; We only check odd numbers.
    ; But to get from 2 to 3, we may not inc when we're at 2.
    cmp rcx, 2
    je skip_add

    inc rcx

skip_add:

    ; This divisor isn't a divisor of our number. Let's try with the next divisor.
    inc rcx
    jmp next_prime_test

is_prime:
    ; This number is prime, add it to our result.
    add Sum, CurrentPrime

check_next_prime:

    ; If we're doing 6n-1, increase by 2 to get 6n+1, then check again
    cmp PrimeMode, 0
    ; If we've done 6n+1, then go for the next n
    jne loop

    mov PrimeMode, 1 ; Set plus
    mov rcx, 2 ; Reset rcx to start over with a new prime
    add CurrentPrime, 2 ; 6n-1 +2 = 6n+1

    ; Check 6n+1
    jmp next_prime_test

finish:
    ; Result is in rax
    mov rax, Sum

    ret

Because all of your variables Sum, CurrentPrime, CurrentN, and PrimeMode reside in registers your code could/should benefit from this fact.

You've calculated CurrentPrime = CurrentN * 6 - 1 using:

mov rax, CurrentN     ; multiply it by 6 and save it to CurrentPrime
mov rbx, 6
mul rbx
mov CurrentPrime, rax
dec CurrentPrime    ; Start with 6n-1

Not only does this clobber the registers rax, rbx, and rdx, it also uses the mul instruction variant that produces an 128-bit result. Intel advices against this form when the upper 64-bits of the result are not needed which is the case in your program. A better multiplication then comes from using the 2 operand form of imul.

mov  CurrentPrime, CurrentN
imul CurrentPrime, 6
dec  CurrentPrime

This shorter and faster solution also doesn't use any additional registers!

When you reset your variables and registers you consistently use the mov instruction.

mov CurrentN, 0 ; This gets inc'ed below
mov PrimeMode, 0
mov rdx, 0

Because these variables really are registers too, and because the preferred way to zero a register is to xor it with itself, you should write instead:

xor  CurrentN, CurrentN
xor  PrimeMode, PrimeMode
xor  rdx, rdx

This produces much shorter code.

This is how you check if PrimeMode is zero.

cmp PrimeMode, 0
jne loop

Because this variable really is a register, and because the preferred way to find out if a register is zero is to test it with itself, you should write instead:

test PrimeMode, PrimeMode
jnz  loop

In next snippet you want to toggle PrimeMode when the prime at 6n-1 was found.

cmp PrimeMode, 0    ; If we've done 6n+1, then go for the next n
jne loop
mov PrimeMode, 1 ; Set plus

That's fine but you could use the fact that PrimeMode is equal to zero here and just have it incremented instead of using the space consuming immediate value:

test PrimeMode, PrimeMode
jnz  loop
inc  PrimeMode             ;Set plus

Divisions are costly.

In order to conclude that the current number is prime, you perform a division by 2.

; When divisor_to_test > (number_to_test/2), then this is prime
mov rax, CurrentPrime
mov rbx, 2
mov rdx, 0
div rbx
cmp rax, rcx
jl is_prime

You should replace this wasteful div by a simple shift to the right:

mov  rax, CurrentPrime
shr  rax, 1
cmp  rax, rcx
jl   is_prime

Why in next part of the code doesn't your division use the rcx register straight away? Why do you first move the divisor in rcx to rbx? That's just a superfluous instruction!

; Test divisor for divisor
mov rax, CurrentPrime
mov rbx, rcx
mov rdx, 0
div rbx
cmp rdx, 0
je check_next_prime

No longer clobbering the rbx register and applying many of the above tips gives:

mov  rax, CurrentPrime
xor  rdx, rdx
div  rcx
test rdx, rdx
jz   check_next_prime

Avoid redundant operations.

In your most inner loop you keep checking if CurrentPrime exceeds 2000000, but you forget that the value of CurrentPrime does not change within this inner loop. Best move the test above the next_prime_test label introducing an additional label MAYBE_next_prime_test.

    mov  PrimeMode, 0             ; Reset registers for testing
    ;;;mov  rcx, 2     <----- Remove this line
MAYBE_next_prime_test:
    cmp  CurrentPrime, 2000000    ; When the number to test is >= 2000000, then exit
    jge  finish
    mov  rcx, 2     <----- This line replaces both others
next_prime_test:

    ...

    inc rcx
    jmp next_prime_test

    ...

    ;;;mov  rcx, 2     <----- Remove this line
    add  CurrentPrime, 2          ; 6n-1 +2 = 6n+1
    jmp  MAYBE_next_prime_test    ; Check 6n+1
finish:
    mov  rax, Sum                 ; Result is in rax

I've also avoided the duplication of the mov rcx, 2 instruction that you already had in the top part of the program. Shaves off some bytes!


Reversing each word in a sentence Below code reverse a word  given by user input  example.

input : This is code Review

output : siht si edoc weiveR

steps

step 1: check for whitespace if found set the start and end position and
        pass to reverseword

step 2: check for null terminator  and set start and end position pass to reverseword

#include<stdio.h>
#include <ctype.h>
#define MAX 100
#define SPACE 1
#define NEXT 1
#define MIN_CHAR 1
void  reverseword(int start,int end,char *input)
{
        char temp;
        int i;
        while(end > start)
        {
                temp=input[start];
                input[start]=input[end];
                input[end]= temp;
                start++;
                end--;
        }
}

int main()
{
        char input[MAX];
        scanf("%[^\n]",input);
        int pos;
        int start=0,end=0;
        for(pos=0 ; input[pos] ; ++pos)
        {
                if(isspace(input[pos]) && pos>MIN_CHAR) // case when space is found
                {
                        end = pos-SPACE;
                        reverseword(start,end,input);
                        start= pos+SPACE;
                }
                if(input[pos+NEXT]=='\0' && pos>MIN_CHAR) // case for last world
                {
                        end =pos;
                        reverseword(start,end,input);
                }
        }
}

Enable warnings

The -Wall on GCC leads to:

reverse.c:10:13: warning: unused variable ‘i’ [-Wunused-variable]

Do not repeat yourself

You check pos > MIN_CHAR in two places. This could be done only once:

    if (pos > MIN_CHAR)
    {
        if(isspace(input[pos])) // case when space is found
        {
            end = pos-SPACE;
            reverseword(start,end,input);
            start= pos+SPACE;
        }
        if(input[pos+NEXT]=='\0') // case for last world
        {
            end =pos;
            reverseword(start,end,input);
        }
    }

Be consistent in your code style

The code style seems a bit inconsistent. For instance, sometimes you have whitespaces before "=", sometimes after, sometimes both, sometimes none... You'll find various coding styles online and different tools to check you code compliancy.

Declare variables in smallest possible scope

I find it a good habit to define variable in the smallest scope in order to make things easier to track. In your case, temp could be declared inside the loop.

Getting rid of the your #define-d values

All your values (except for MAX) could be removed because the link between the name and the value is hard to understand at first.:w

So far, I've got:

#include<stdio.h>
#include <ctype.h>
#include <string.h>
#define MAX 100

void reverseword(int start, int end, char *input)
{
    while (end > start)
    {
        char temp = input[start];
        input[start] = input[end];
        input[end] = temp;
        start++;
        end--;
    }
}

int main()
{
    // Updated for testing purposes
    //        char input[MAX];
    //        scanf("%[^\n]",input);
    char * original = "this is code review";
    char * input = strdup(original);
    printf("%s\n", input);
    int pos;
    int start = 0;
    for (pos = 0 ; input[pos] ; ++pos)
    {
        if (pos > 1)
        {
            if (isspace(input[pos])) // case when space is found
            {
                int end = pos-1;
                reverseword(start, end, input);
                start = pos + 1;
            }
            if (input[pos+1] == '\0') // case for last world
            {
                int end = pos;
                reverseword(start, end, input);
            }
        }
    }
    printf("%s\n", input);
}

Typo: You meant "word", not "world"

Not much to add here.

Handle the last word at the end

You can handle the final case the end. Then you can merge the condition check togetherin the loop.

Bug found

We have the following behaviors:

"a this is code review" -> "siht a si edoc weiver"
" a this is code review" -> "a  siht si edoc weiver"

I have to go, I'll try to continue asap.


jQuery code to total the items in a shopping cart I implemented a shopping cart with asp.net and jquery on my website, and here is my js code which calculates amount and total sum on client side:

 $(document).ready(function () {
            update();
        $(".quant").change(function() {

            update();
         });
        function update() {

            var id = $('.quant').attr('data-id');
            var sum = 0.0;
            var quantity;
            $('#myTable > tbody  > tr').each(function () {

                quantity = $(this).find('.quant').val();
                var price = parseFloat($(this).find('.price').attr('data-price').replace(',', '.'));
                var amount = (quantity * price);

                sum += amount;
                $(this).find('.amount').text('' + amount + ' грн');

            });

            $('.total').text(sum + ' грн');

            $.get(
             '/Cart/AddTocart',
             {
                 id: id,
                 returnUrl: '',
                 quantity: quantity

             }
         );
        }

        });

Some people review my code and said me that it needs refactoring:

this part of code  var id = $('.quant').attr('data-id');

is wrong because if we have 1 element jquery works correctly, but if we have array of elements  - jquery "takes" only first element and attr doesn't work good ,  so how can i  fix it?? This id I pass to server side.  I thought it needs write something like this:

var id = $(this).attr('data-id');

but id will always undefined.
Problem here:

var quantity;
      $('#myTable > tbody  > tr').each(function () {
         quantity = $(this).find('.quant').val();
         ...
}

It recalculates all elements of quantity and quantity "saves" last known value; what's wrong here??
I was told that I pass first id and last quantity, but it is all present when I call function in $(document) - Please explain this to me.

Here is html code:

        Товар
        К-сть
        Назва Товару
        Ціна
        Загальна ціна

    @foreach (var line in Model.Cart.Lines)
    {

        IEnumerable<FurnitureImages> images = line.Furniture.Images;
        FurnitureImages mainImage = images.Where(x => x.IsMainImage).FirstOrDefault();

               @if (mainImage != null)
               {

               }

            <input type="text" data-id="@line.Furniture.FurnitureId" data-price="@line.Furniture.Price" value="@line.Quantity" class="quant" />

            @line.Furniture.Name
            @((line.Furniture.Price).ToString("#.## грн"))
            @((line.Quantity * line.Furniture.Price).ToString("#.## грн"))

                @using (Html.BeginForm("RemoveFromCart", "Cart", new { Id = line.Furniture.FurnitureId }))
                {
                    @Html.Hidden("Id", line.Furniture.FurnitureId)
                    @Html.HiddenFor(x => x.ReturnUrl)
                    <input class="btn btn-sm btn-warning" type="submit" value="Видалити з кошику" />
                }

    }

        Всього до оплати:
        @Model.Cart.ComputeTotalValue().ToString("#.## грн")

this part of code  var id = $('.quant').attr('data-id');

Presuming there are multiple inputs with class "quant" (perhaps 1 for each row in the table), then this code will get a collection of elements and then as is desribed in the documentation for .attr():

  Get the value of an attribute for the first element in the set of matched elements or set one or more attributes for every matched element.

So yes it will get the value of the data-id attribute of the first element with that class name.

Consider the following snippet (try running it to see the result):

//find all elements with class name "quant"
var inputsWithClassQuant = $('.quant');
console.log('inputsWithClassQuant.length: ',inputsWithClassQuant.length);
//attr only returns attribute of first element:
console.log("inputsWithClassQuant.attr('data-id') (first element only): ",inputsWithClassQuant.attr('data-id'));

<input type="text" data-id="30" data-price="20" value="3" class="quant" />
<input type="text" data-id="35" data-price="10" value="0" class="quant" />
<input type="text" data-id="36" data-price="15" value="2" class="quant" />

  Problem here:

var quantity;
$('#myTable > tbody  > tr').each(function () {
 quantity = $(this).find('.quant').val();
 ...
}

Yes, quantity = $(this).find('.quant').val(); overwrites the previous value each time so after the loop (i.e. the call to .each()), quantity will contain the value of the last element with class name quant.

  I was told that I pass first id and last quantity, but it is all present when I call function in $(document) - Please explain this to me.

This is basically a summary of points #1 and #2 above.

Perhaps a better implementation would move the code to call the AJAX request (i.e. to '/Cart/AddTocart') with the id and quantity of the row that changed, then call update() to update the total. Something like the code below.

Another suggestion is to update the request sent to the server to send the entire list of items with the respective quantities. That way, if a quantity is decreased or cleared, the cart can be accurately updated. It all depends on the back-end API - i.e. if it has endpoints to add/remove/update items with quantities, etc.

I also changed the input type of the quantity inputs to "Number" - that way only numbers can be entered, and many browsers will add up/down controls to the side of the input for the user to click on.

$(document).ready(function() {
  update();
  $(".quant").change(function() {
    //this: context of the input that was changed
    console.log('calling /Cart/AddTocart; id:',$(this).attr('data-id'),' quantity: ', $(this).val());
    $.get(
      '/Cart/AddTocart', {
        id: $(this).attr('data-id'),
        returnUrl: '',
        quantity: $(this).val()
      });
    update();
  });

  function update() {
    var sum = 0.0;
    var quantity;
    $('#myTable > tbody  > tr').each(function() {

      quantity = $(this).find('.quant').val();
      var price = parseFloat($(this).find('.price').attr('data-price').replace(',', '.'));
      var amount = (quantity * price);

      sum += amount;
      $(this).find('.amount').text('' + amount + ' грн');
    });
    $('.total').text(sum + ' грн');
  }
});

      Товар
      К-сть
      Назва Товару
      Ціна
      Загальна ціна

      <input type="number" data-id="97" data-price="30" value="1" class="quant" />

    Kart
    30
    30

      <input class="btn btn-sm btn-warning" type="submit" value="Видалити з кошику" />

      <input type="number" data-id="99" data-price="60" value="0" class="quant" />

    Kart
    10
    30

      <input class="btn btn-sm btn-warning" type="submit" value="Видалити з кошику" />

      Всього до оплати:
      @Model.Cart.ComputeTotalValue().ToString("#.## грн")


Join 2 arrays together There are 2 arrays. One that consists of Categories and one of Products. Each product pertains to its specific category. I want to join each product to its right category (a category can have multiple products). Each product that 'finds' its category will go in this category's products array.

for ($i = 0; $i < count($prods); $i++)
{
    for ($u = 0; $u < count($cats); $u++)
    {
        if ($prods[$i]['category_code'] === $cats[$u]['category_style_code'])
        {
            if ( !isset($cats[$u]['products']) )
            {
                $cats[$u]['products'] = array();
            }
            array_push($cats[$u]['products'], $prods[$i]);
        }
    }
}

It results in something like:

Array
(
    [0] => Array
        (
            [id] => 1
            [category_style_code] => GA
            [products] => Array
                (
                    [0] => Array
                        (
                            [id] => 1
                            [default_price] => 37.50
                            [category_code] => GA
                        )

                    [1] => Array
                        (
                            [id] => 2
                            [default_price] => 15.00
                            [category_code] => GA
                        )
                )
        )
)

Let's say there are many categories and many products. How would you optimize this code (or do it differently)?

Tweaks

for ($i = 0; $i < count($prods); $i++)
{
    for ($u = 0; $u < count($cats); $u++)
    {

I'd avoid calling count on each iteration of the loop.  Save the value at initialization time instead.

for ( $i = 0, $n = count($prods); $i < $n; $i++ ) {
    for ( $u = 0, $m = count($cats); $u < $m; $u++ ) {

I'd flip these around.

foreach ( $cats as &$category ) {
    foreach ( $prods as $product ) {

The switch to foreach means that you aren't having to muck with the count.  It will handle that in the background.

            array_push($cats[$u]['products'], $prods[$i]);

When adding a single value, []= is more efficient than array_push.

            $category['products'][] = $product;

When you use the reference version of foreach, you should explicitly break the reference using unset when done.

unset($category);

Note that if $cats were the inner loop, we'd need to do this for every iteration of the outer loop.

So in whole, that gives us

foreach ( $cats as &$category ) {
    foreach ( $prods as $product ) {
        if ( $product['category_code'] === $category['category_style_code'] ) {
            if ( ! isset($category['products']) ) {
                $category['products'] = array();
            }

            $category['products'][] = $product;
        }
    }
}

unset($category);

New Algorithm

But we can do even better.  Note that this runs count($cats) times count($prods) iterations.  But we don't actually have to check every category against every product.  We just need a way to look up the category for a given code.

    $category_of = array();
    foreach ( $cats as &$category ) {
        $category_of[$category['category_style_code']] = &$category;
    }
    unset($category);

Now our $prods loop can just look up the category.

    foreach ( $prods as $product ) {
        if ( isset($category_of[$product['category_code']]) ) {
            if ( ! isset($category_of[$product['category_code']]['products']) ) {
                $category_of[$product['category_code']]['products'] = array();
            }

            $category_of[$product['category_code']]['products'][] = $product;
        }
    }

If we don't mind having empty products arrays in a category, we can speed up the second loop by moving the check into the first loop.

    $category_of = array();
    foreach ( $cats as &$category ) {
        $category_of[$category['category_style_code']] = &$category;

        if ( ! isset($category['products']) ) {
            $category['products'] = array();
        }
    }
    unset($category);

Now each $category['products'] is guaranteed to be set, so we only do this check once per category not once per product.  Note that we may not need to do the check.  if we know that $category['products'] is not set until we do this, we can just do the assignment.

Currently we check that a given category exists before assigning.  If this will always be true, then we can remove the check and just assign:

    foreach ( $prods as $product ) {
        $category_of[$product['category_code']]['products'][] = $product;
    }

Now we iterate count($cats) plus count($prods) times.  This sum should be smaller than the product in most cases.  The exceptions would be single element cases (only one product or one category or both).  This will use more memory (for the $category_of array), but hopefully you aren't right up against the memory limit anyway.

Final Version

    $category_of = array();
    foreach ( $cats as &$category ) {
        $category_of[$category['category_style_code']] = &$category;

        $category['products'] = array();
    }
    unset($category);

    foreach ( $prods as $product ) {
        $category_of[$product['category_code']]['products'][] = $product;
    }

Another advantage of this version is that it is the first loop that does more work.  Since there are more products than categories (because there are multiple products per category but only one category per product in this model), this should speed things up.

Remember our assumptions though.  If they don't hold true, then the code should change:

$category['products'] is not set with data we need for any category prior to this code.
There is always a category for every $product['category_code'].

The original code did not make these assumptions.  It's unclear if that is because they don't hold as it needed the checks for other purposes.  Additional context might have made this clearer.

An advantage of this version is that it will throw an error if the second assumption does not hold.  The original code would just leave things in an inconsistent state.


3 layer design and multiple dataset I'm new to C# and OO programing.
I have an aspx page with 3 lists of checkboxes and I would like to generate them from the DB.
how should I structure my code?

Here is an example of what I did so far:

ASPX page

<asp:CheckBoxList ID="CheckBoxList1" runat="server"></asp:CheckBoxList>
<asp:CheckBoxList ID="CheckBoxList2" runat="server"></asp:CheckBoxList>
<asp:CheckBoxList ID="CheckBoxList3" runat="server"></asp:CheckBoxList>

ASPX Code behind:

protected void Page_Load(object sender, EventArgs e)
{
    if (!IsPostBack)
    {
        GenerateCheckBoxList1();
        GenerateCheckBoxList2();
    }
}

private void GenerateCheckBoxList1()
{
    CBLists cbl = new CBLists();
    CheckBoxList1.DataSource = cbl.GetCheckBoxList1();
    CheckBoxList1.DataTextField = "name";
    CheckBoxList1.DataValueField = "id";
    CheckBoxList1.DataBind();
}

private void GenerateCheckBoxList2()
{
    CBLists cbl = new CBLists();
    CheckBoxList2.DataSource = cbl.GetCheckBoxList2();
    CheckBoxList2.DataTextField = "country";
    CheckBoxList2.DataValueField = "country_id";
    CheckBoxList2.DataBind();
}

a class file to handle the connection to the db and return the DataSet:

public class CBLists
{
    private SqlConnection conn = new SqlConnection(ConfigurationManager.ConnectionStrings["CONN"].ConnectionString);
public DataSet GetCheckBoxList1()
{
    try
    {
        DataSet ds = new DataSet();
        string cmdstr = "select * from table1 where id<>999 order by id";
        SqlDataAdapter adp = new SqlDataAdapter(cmdstr, conn);
        adp.Fill(ds);
        return ds;
    }
    catch (Exception ex)
    {
        return null;
    }
    finally
    {
        conn.Close();
    }

}

public DataSet GetCheckBoxList2()
{
    try
    {
        DataSet ds = new DataSet();
        string cmdstr = "select * from table2";
        SqlDataAdapter adp = new SqlDataAdapter(cmdstr, conn);
        adp.Fill(ds);
        return ds;
    }
    catch (Exception ex)
    {
        return null;
    }
    finally
    {
        conn.Close();
    }
}

Is this the "correct" way to do it? multiple methods in a separate file?

In 3-Layer design, we have below three layers

Presentation Layer: In this case your aspx and code behind files.

Business Layer: It seems this is missing in your project. You may include this for various purposes e.g., filtering items based on different scenarios, logged in user, etc.

Data Layer: This layer interacts with data store, external services, etc. CBLists class in this case.

Also, in OO programming class names are singular so CBLists should be changed to CBList.


JavaScript flipping clock Here are two simple classes - Digit class maintains digit behaviour - primarily flipping animations, and Clock class handles everything that is related to time data.

I'm going to use this code for job application, please tell me if there are any improvements that could be done and what do you think of it.

Here is the link for working demo: https://abm0.github.io/flipping-clock.

class Digit {
  constructor({ selector, value = 0 }) {
    const digitEl = document.querySelector(selector);

    this.flipperEls = digitEl.querySelectorAll('.flipper');
    this.prevDigitEls = digitEl.querySelectorAll('.prev .digit');
    this.nextDigitEls = digitEl.querySelectorAll('.next .digit');

    this.value = value;
    this.prevValue = null;

    this.renderInitialValue();
  }

  setValue(nextValue) {
    this.prevValue = this.value;
    this.value = nextValue;

    if (this.value === this.prevValue) return;

    this.flip();
  }

  renderInitialValue() {
    const {
      prevDigitEls,
      nextDigitEls,
    } = this;

    [...prevDigitEls, ...nextDigitEls].forEach(el => (el.innerHTML = this.value));
  }

  flip() {
    this.nextDigitEls.forEach(el => el.innerHTML = this.value);
    this.flipperEls.forEach(el => el.classList.add('turned'));

    setTimeout(() => {
      this.prevDigitEls.forEach(el => (el.innerHTML = this.value));

      this.flipperEls.forEach((el) => {
        el.classList.remove('turned');
      });
    }, 500);
  }
}

class Clock {
  constructor(props) {
    const baseEl = document.querySelector("#clock");
    const currentTime = this.getCurrentTime();

    this.digits = [
      'hours-tens',
      'hours-ones',
      'minutes-tens',
      'minutes-ones',
      'seconds-tens',
      'seconds-ones'
    ];

    this.buildDigits(currentTime);
  }

  getCurrentTime() {
    const date = new Date();

    const time = {
      hours: date.getHours(),
      minutes: date.getMinutes(),
      seconds: date.getSeconds(),
    };

    this.formatValues(time);

    return time;
  }

  formatValues(time) {
    Object.keys(time).forEach(key => {
      if (key === "ampm") return;

      let value = time[key];

      if (parseInt(value) < 10) {
        time[key] = "0" + value;
      }

      time[key] = time[key].toString();
    });
  }

  getDigitProps(digitName) {
    const type = digitName.split('-')[0];
    const position = digitName.split('-')[1];

    let positionIndex;

    switch (position) {
      case 'tens':
        positionIndex = 0;
        break;

      case 'ones':
        positionIndex = 1;
        break;
    }

    return { type, position, positionIndex };
  }

  buildDigits(time) {
    this.digits.forEach((digitName) => {
      const { type, position, positionIndex } = this.getDigitProps(digitName);

      const selector = `#${type} .${position}-digit`;

      this[digitName] = new Digit({
        selector,
        value: time[type][positionIndex]
      });
    });
  }

  tick() {
    const time = this.getCurrentTime();

    this.digits.forEach((digitName) => {
      const { type, positionIndex } = this.getDigitProps(digitName);

      this[digitName].setValue(time[type][positionIndex]);
    });

  }
}

const clock = new Clock();

setInterval(() => {
  clock.tick();
}, 1000);

Review
If you are after a entry level job then your code shows you can code, which is a good start.
As an emploier I would ask.

Why did you choose to use class syntax over more traditional code styles?
How long did it take you to write this code?

Bad points
Some points that detract from the code quality
JS

Clock constructor takes an argument that is not used.constructor(props)
Unused variable in Clock constructor const baseEl = document.querySelector("#clock");
Hard coded object is tested for a key it will never have if (key === "ampm") return;
Clock can only exist as a single instance, but nothing prevents clock being constructed many times.
Setting element content via innerHTML even though the content has no markup.this.nextDigitEls.forEach(el => el.innerHTML = this.value);
Redundancy suggests you are not sure about type coercion if (parseInt(value) < 10) {
You have Not used getters and setters yet have the functions Digit.setValue, Clock.getCurrentTime
The Clock takes on a Digit s responsibility ? Clock.getDigitProps
Statement with only two outcomes written using switch not as a ternary in function Clock.getDigitProps
Lots of redundant code. An example const {prevDigitEls, nextDigitEls } = this; followed by the line [...prevDigitEls, ...nextDigitEls].
Use event listener to get the flip animation end rather than using setTimeout

CSS & HTML
Its a complete mess, I was going to fix it but it is unusable and needs to be worked from the ground up.

Bad name for flip animation class turned which you add to an element to start the animation and remove to reset.
Your code uses ECMAScript6+, yet you include legacy browser CSS, all the legacy CSS is wasted as the clock will not start on those browsers.
It would be far better to have a container that contains data properties and then build the elements in code. For example HTML only needs

 to create the clock

Rewrite
Its just the code.
Personally I would not have written it as follows because the class syntax forces you to break the rules of encapsulation (No privates) and as its only ever to be a single instance Clock which in reality only needs the tick function which could just be a function.
;(() => {
    const digitNames = ['hours-tens', 'hours-ones', 'minutes-tens', 'minutes-ones', 'seconds-tens', 'seconds-ones'];
    const pad0 = num => ("" + num).padStart(2, "0");
    const time = (d = new Date()) => pad0(d.getHours()) + pad0(d.getMinutes()) + pad0(d.getSeconds());
    const elementsText = (elements, text) => { for (const el of elements) { el.textContent = text } }
    class Digit {
        constructor(name, timeString) {
            this.index = digitNames.indexOf(name);
            const [type, position] = name.split("-");
            const container = document.querySelector(`#${type} .${position}-digit`);
            this.flipperEls = container.querySelectorAll('.flipper');
            this.nextDigitEls = container.querySelectorAll('.next .digit');
            this.currentDigitEls = container.querySelectorAll('.prev .digit');
            this.next = this.val = timeString[this.index];
            this.current = this.val;
        }
        set time(timeStr) {
            if (timeStr[this.index] !== this.val) {
                this.next = this.val = timeStr[this.index];
                this.flippers = "turn";
                setTimeout(() => {
                    this.current = this.val;
                    this.flippers = "turned";
                }, 500);
            }
        }
        set next(value) { elementsText(this.nextDigitEls, value) }
        set current(value) { elementsText(this.currentDigitEls, value) }
        set flippers(action) {
            for (const el of this.flipperEls) {
                if (action === "turn") { el.classList.add('turned') }
                else { el.classList.remove("turned") }
            }
        }
    }
    class Clock {
        constructor() {
            const timeStr = time();
            this.digits = digitNames.map(name => new Digit(name, timeStr));
            (this.tick = this.ticker.bind(this))(); // start the clock
        }
        ticker() {
            const timeStr = time();
            this.digits.forEach(digit => digit.time = timeStr);
            const now = Date.now();
            const nextSecondIn = ((now / 1000 | 0) + 1) * 1000 - now;
            setTimeout(this.tick, nextSecondIn + (nextSecondIn > 500 ? 0 : 1000));
        }
    }
    new Clock();
})();


Azure cache getoradd (without locking) Inspired by the non-locking implementation proposed in this post, the following code is an attempt to (about) do the same using the azure cache. As I'm totally green on the Azure cache I'd appreciate input on best practices or pitfalls. I should add that this method is assumed to be the only interaction with the azure cache in the application.

public class AzureCacheWrapper
{
    private readonly DataCache cache;

    public AzureCacheWrapper(DataCacheFactory cacheFactory)
    {
        this.cache = cacheFactory.GetDefaultCache();
    }

    public T GetOrAdd<T>(string key, Func<T> factoryMethod)
    {
        var factoryMethodAsLazy = new Lazy<T>(factoryMethod);
        var cachedFactory = cache.Get(key) as Lazy<T>;
        if (cachedFactory == null)
        {
            try
            {
                cache.Add(key, factoryMethodAsLazy);
                cachedFactory = factoryMethodAsLazy;
            }
            catch (DataCacheException ex)
            {
                if (ex.ErrorCode != DataCacheErrorCode.KeyAlreadyExists)
                {
                    throw;
                }

                // We know for sure that the key exists at this point:
                // Two concurrent callers tried to get it, and this thread
                // happened to be deadlock victim.
                cachedFactory = (Lazy<T>)cache.Get(key);
            }
        }

        return cachedFactory.Value;
    }
}

I do not have an Azure account, so I cannot test this, but does this work in general? The linked question uses a MemoryCache which is in-memory and probably references the added objects directly. As Azure cache is a distributed system, that will probably not work and the values need to be serialized in some way to be passed to the remote cache.

As I was writing this, I stumbled upon this SO question which clearly shows that Lazy<T> is serializable and thus can probably be stored in Azure, but the value will be evaluated on serialization. Basically, using Lazy<T> does not give you any advantage here, because of the aforementioned side-effect of putting it in the cache.

            // We know for sure that the key exists at this point:
            // Two concurrent callers tried to get it, and this thread
            // happened to be deadlock victim.

That state is probably not true, because the cache item with that key might have expired between the exception and the call to Get. Granted, that this is relatively unlikely and heavily depends on how small the cache timeout is, but it could happen. This scenario could be avoided if you retried the whole method upon receiving a DataCacheErrorCode.KeyAlreadyExists error code.


Calculate next position and velocity after "bounce" Intro and code

I have the following function that calculates an object's "next" position and velocity (in 1 dimension) given a boundary [0, max].

public TwoVector<Double> getNextPositionAndVelocity(double position, double velocity, double max) {
    if (position + velocity < 0) {
        return TwoVector.<Double>builder()
                .x(Math.abs(position + velocity))
                .y(Math.abs(velocity))
                .build();
    }
    if (position + velocity > max) {
        return TwoVector.<Double>builder()
                .x(max - ((position + velocity) % max))
                .y(-Math.abs(velocity))
                .build();
    }
    return TwoVector.<Double>builder()
            .x(position + velocity)
            .y(velocity)
            .build();
}

Explanation

The first condition checks if adding the current velocity to the current position would cause the object to be beyond the "left" boundary.  If that's the case, the next position should be equal to the amount the object would be beyond the left boundary added to the left boundary and the next velocity should point the object to the "right".
The second condition checks if adding the current velocity to the current position would cause the object to beyond the "right" boundary.  If that's the case, the next position should be equal to the amount the object would be beyond the boundary subtracted from the boundary and the next velocity should point the object to the "left".
If neither condition is true, we can add the current velocity to the current position to get the next position without issue.

Question

Are there more straightforward functions to calculate each of these "next" values?  Perhaps a function that "collapses" these branches?

Review

You have redundant code. Store position + velocity in a variable.
Use well-known variable names when the code is of a mathematical nature. Distance x Velocity v A clamped area [a,b].
It is not clear from your description what should happen if the distance is so far away from the boundaries, that when reflecting the movement you are still out of bounds, but on the other side.
.x(max - ((position + velocity) % max)) This does not correspond to your description of

  the next position should be equal to the amount the object would be beyond the boundary subtracted from the boundary

Proposed Solution

Use mathematical variable names.
Allow damped bouncing when the distance is too far out of bounds (Damped bouncing reflects the value x, alternating around the boundaries until it is clamped with the area [a,b]).
Make both 0 and max a variable, respectively a and b.

public TwoVector<Double> getNextPositionAndVelocity(double x, double v, double a, double b)
{
    assert a < b;
    double xn = x + v;

    // the loop allows for damped bouncing
    while (true) {
        if (xn < a) {
            xn = a - xn;
            v = Math.abs(v);
        }
        else if (xn > b) {
            xn = b - (xn - b);
            v = -Math.abs(v);
        }
        else {
            break;
        }
    }

    return TwoVector.<Double>builder().x(xn).y(v).build();
}


Spam detection in PHP for comment system I'am trying to build a PHP/MySQL/jQuery comment system.  Off late I started to realize that manual spamming is a serious issue that need to be addressed carefully. So I thought of building a PHP function (initially, later can be implemented in OOPS concept) that will sniff the content and give points (spam points) based on some criteria.  Getting a spam point less than will make the content inactive and send for moderation, whereas posts with spam points greater than 2 won't make the content eligible to be inserted in DB. This is a very basic piece of codes, so I want you to give me your valuable suggestion as how to make this code much better and if I am doing something wrong here.

<?php

#spam points < 2 { will be send for moderation}
#spam points > 2 { wont be posted , ie wont be inserted in DB}

function sniffSpams($content)
{
    $spam_points    =    0;
    $url_pattern    =   '#(www\.|https?://)?[a-z0-9]+\.[a-z0-9]{2,4}\S*#i';

    preg_match_all($url_pattern, $content, $matches, PREG_PATTERN_ORDER);

    if(! empty($matches))
    {
        //url is/are present

        $get_number_of_urls    =   count($matches[0]);       //get the number of urls/emails present in content

        if($get_number_of_urls > 2)
            $spam_points += $get_number_of_urls;            //1 point per link
        else
        {
            $spam_words_uri  = array('free', 'vote', 'play');

            //if less thamn 2 , check for length of url
            foreach ($matches[0] as $url)
            {
                if(strlen($url) > 150) //long url mostly are spam
                   $spam_points += 1;

                foreach($spam_words_uri as $spam)
                {
                    if(stripos($url, $spam) !== false )
                        $spam_points += 1;
                }

            }

        }

    }

   $spam_words  = array('Levitra', 'viagra', 'casino', '*');

   foreach($spam_words as $spam)
   {
        if(stripos($content, $spam) !== false )
            $spam_points += 1;

   }

    return $spam_points;

}

echo sniffSpams('the * dsjdjsd ');

Minor (and subjective) remarks:

Function name: The function does not sniff spam but calculate a score for the given content. Therefore I'd rename the function to something like getSpamScore
The variable $matches is not declared. Declaring it before using in in preg_match_all allows IDEs to perform code analysis and other developers don't have a hard time looking for its declaration.
Two empty lines just take up space. Your code should be clear enough from structure not to need any extra indentions (which you don't have) or double blank lines. As the preg_match_all is directly related to the if and only to it, I'd not have any lines at all between the match and the if-statement.  (This applies to all other lines too)
//url is/are present comment: more of a personal thing maybe: I don't like obvious comments. Personally I prefer the if it needs a comment it needs to be refactored way of writing comments (non in about almost every case). Comments need maintenance time (updating comments for changed code). But time should only be spent here if it provides value. Additionally comments tend to loose their relation to the code they originally were attached to (mostly by refactoring). This greatly confuses other developers.
$get_number_of_urls naming: this variable contains a verb. Verbs indicate actions (e.g. a method). In PHP I'd expect this variable to be a closur, not an integer. Yet you don't actually get the number or urls, you already have them. A proper name might be number_of_urls instead.
Stick to one convention of styling code: in your case this especially applies for where to have whitespaces. In your if and foreach lines you mix up foreach( foreach ( and your if conditions sometimes have whitespaces in between and sometimes not. I suppose this is some kind of work-in-progress code. Yet later styling costs time. Code you write always should meet production ready requirements (regarding style). It takes some time at the beginning to stick with someones conventions, but you'll get used to it and do it automatically after some time.
In my opinion this function has too many responsibilities (at least two). I'd suggest splitting this function into two sub-functions. This function itself only calls and sums of the results of the independent tests. When going OO this of course wants a design pattern ;)