How to refactor code to use threads? I have a WebPostRequest class which looks like this:

public class WebPostRequest
{
    public const string Success = "OK";

    private WebRequest _webRequest;
    private List<string> _paramsList;

    public WebPostRequest(string url)
    {
        try
        {
            _webRequest = WebRequest.Create(url);
        }
        catch (Exception)
        {
            throw;
        }
        _webRequest.Method = "POST";
        _webRequest.ContentType = "application/x-www-form-urlencoded";
        _paramsList = new List<string>();
    }

    public void Add(string key, string value)
    {
        _paramsList.Add(String.Format("{0}={1}", key, HttpUtility.UrlEncode(value)));
    }

    public void UploadPdf(string postKey, string name, params UserControl[] userControls)
    {
        string fileName = FileHelper.GetTimestampedName(name, "pdf");
        string filePath = PdfHelper.Save(fileName, userControls);
        FtpHelper.UploadFile(filePath);
        this.Add(postKey, fileName);
    }

    public void UploadImage(string postKey, string name, Image image)
    {
        if (image != null)
        {
            string fileName = FileHelper.GetTimestampedName(name, "jpg");
            FtpHelper.UploadFile(fileName, image.ToByteArray());
            this.Add(postKey, fileName);
        }
    }

    public string GetResponse()
    {
        // Build a string containing all the parameters
        string parameters = String.Join("&", _paramsList.ToArray());
        _webRequest.ContentLength = parameters.Length;

        try
        {
            // Write the parameters into the request
            using (StreamWriter streamWriter = new StreamWriter(_webRequest.GetRequestStream()))
            {
                streamWriter.Write(parameters);
            }

            // Execute the query
            HttpWebResponse httpWebResponse = (HttpWebResponse)_webRequest.GetResponse();
            StreamReader streamReader = new StreamReader(httpWebResponse.GetResponseStream());
            return streamReader.ReadToEnd();
        }
        catch (Exception)
        {
            throw;
        }
    }
}

The WebPostRequest class uses a WebRequest object to POST to a web server and get the response.

The class is used like this:

bool success = false;
try
{
    WebPostRequest webPostRequest = new WebPostRequest(url);

    webPostRequest.Add("key_1", "value_1");
    webPostRequest.Add("key_2", "value_2");

    webPostRequest.UploadPdf("key_3", "pdf_name_1", userControl1);
    webPostRequest.UploadPdf("key_4", "pdf_name_2", userControl2);

    webPostRequest.UploadImage("key_5", "picture_name_1", image1);
    webPostRequest.UploadImage("key_6", "picture_name_1", image2);

    string response = webPostRequest.GetResponse();
    if (response != WebPostRequest.Success)
    {
        throw new Exception(response);
    }
    success = true;
}
catch (Exception ex)
{
    //display error messaage
}
if (success)
{
    //display success message
}

What i would like to do is add a window to display a status message and a progress bar (with marquee style). The window needs to be displayed from the WebPostRequest class, because this class is used in multiple locations in the app. The uploading tasks which may take a long time need to be executed on a different thread so that the UI thread remains responsive. Note that all the files need to be uploaded before the POST is made, so the GetResponse method needs to wait for the uploading tasks to finish. Note also that the helper methods can throw errors which are handled outside the WebPostRequest class.

What tips/suggestions can you give me? Any idea is appreciated.

I would not show any UI from WebPostRequest. For a fast solution you can just wrap the whole upload part and provide some sort of a listener whose methods will be called to notify about progress.

Thus WebPostRequest will not know nothing about UI layer. UI will not be blocked and be able to track upload progress.

Here's a C#-like pseudo code that illustrates what I mean:

class ProgressListener
{
    public void OnProgress(OperationContext)
    {
        //notiy UI in the thread-safe way -> use Control.Invoke for Windows Forms
    }

    public void OnFinish(OperationContext)
    {
        //show success message in UI
    }

    public void OnError(OperationContext)
    {
        //show detailed error message in UI
    }
}

//on UI class
void PerformUpload()
{

    ProgressListener listener = new ProgressListener(Form f);

    Thread thread = new Thread(() =>
    {
        try
        {
            try
            {
                WebPostRequest webPostRequest = new WebPostRequest(url);
                webPostRequest.Add("key_1", "value_1");
                webPostRequest.Add("key_2", "value_2");

                webPostRequest.UploadPdf("key_3", "pdf_name_1", userControl1);

                listener.OnProgress( /*operaton context*/)

                webPostRequest.UploadPdf("key_4", "pdf_name_2", userControl2);

                listener.OnProgress( /*operaton context*/)
                webPostRequest.UploadImage("key_5", "picture_name_1", image1);

                listener.OnProgress( /*operaton context*/)
                webPostRequest.UploadImage("key_6", "picture_name_1", image2);

                listener.OnProgress( /*operaton context*/)

                string response = webPostRequest.GetResponse();
                if (response != WebPostRequest.Success)
                {
                    listener.OnProgress( /*operaton context*/)

                    throw new Exception(response);
                }
                success = true;

                listener.OnFinish( /*operaton context*/)
            }
            catch (Exception ex)
            {
                listener.OnError( /*operaton context*/)
                //display error messaage
            }
        }
        catch(Exception ex)
        {
            listener.OnError( /*operaton context*/)
            //logging
        }
    });

    thread.Name = "uploader";
    thread.Start();
}


Calculate all cyclic subgroups of a group under multiplication of modulo n (group theory) The task was to calculate all cyclic subgroups of a group \$ \textbf{Z} / n \textbf{Z} \$ under multiplication of modulo \$ \text{n} \$ and returning them as a list of lists. The first level has all subgroups and the secend level holds the elements of these groups.

The programm first calculates the numbers \$ \text{g} \$ that are coprimes of \$ \text{n} \$. Each element of a cyclic subgroup can than be obtained by calculating the powers of \$ \text{g} \$.

A explanation of what cyclic groups are can be found on wikipedia: Group (mathematics).

# coding=utf-8

"""Basic usage:
$ python subgroups.py 1
[[1]]
$ python subgroups.py 7
[[1], [1, 2, 4], [1, 2, 3, 4, 5, 6], [1, 6]]
"""

import argparse
from fractions import gcd

def main():
    """Prints out the results of `calculate_subgroups()`
    """

    parser = argparse.ArgumentParser()
    parser.add_argument('modulus', type=int)
    args = parser.parse_args()

    if args.modulus > 0:
        print(calculate_subgroups(args.modulus))
    else:
        print("Please enter an integer > 0.")

def calculate_subgroups(modulus):
    """For a group of integers modulo n, n is called the `modulus`. With the
    modulus of a cyclic group, calculate all subgroups of Z/nZ.
    See Wikipedia for reference:
    http://en.wikipedia.org/wiki/Group_(mathematics)#Modular_arithmetic

    Basic usage:
    >>> calculate_subgroups(1)
    [[1]]
    >>> calculate_subgroups(4)
    [[1], [1, 3]]
    >>> calculate_subgroups(7)
    [[1], [1, 2, 4], [1, 2, 3, 4, 5, 6], [1, 6]]
    >>> calculate_subgroups(9)
    [[1], [1, 2, 4, 5, 7, 8], [1, 4, 7], [1, 8]]
    >>> calculate_subgroups(18)
    [[1], [1, 5, 7, 11, 13, 17], [1, 7, 13], [1, 17]]
    """

    # All cyclic groups have the trivial subgroup [1]
    subgroups = [[1]]
    coprimes = get_coprimes(modulus)

    for i in range(1, len(coprimes)):
        # Subgroups of cyclic groups always contain 1
        potential_subgroup = [1]

        exponent = 1
        coprime_potency = pow(coprimes[i], exponent, modulus)
        while coprime_potency != 1:
            potential_subgroup.extend([coprime_potency])
            exponent += 1
            coprime_potency = pow(coprimes[i], exponent, modulus)

        potential_subgroup = sorted(potential_subgroup)

        if potential_subgroup not in sorted(subgroups):
            subgroups.append(potential_subgroup)

    return subgroups

def get_coprimes(number):
    """Returns the coprimes of a number as a list.
    Basic usage
    >>> get_coprimes(7)
    [1, 2, 3, 4, 5, 6]
    >>> get_coprimes(10)
    [1, 3, 7, 9]
    """

    coprimes = []

    for i in range(1, number + 1):
        if gcd(number, i) == 1:
            coprimes.extend([i])

    return coprimes

if __name__ == '__main__':
    import doctest
    doctest.testmod()
    main()

Two things to consider:

pow is an overkill. To calculate the subgroup, I'd continuously multiply a power by the generator:

subgroup = [1]
power = generator
while power != 1:
    subgroup.append(power)
    power = (generator * power) % modulus

BTW, now you don't have to special case a trivial subgroup.
I would also strike out calculated powers from the list of candidate generators (that is, coprimes). This way you'd avoid recalculating the same subgroup over and over again.


Cyclic queue using an array I tried to implement a cyclic Queue using an array, as a task from a coding interview.

Please comment about style, as well as any edge cases I may have missed.

using System;
using Microsoft.VisualStudio.TestTools.UnitTesting;

namespace CirucularArray
{
    [TestClass]
    public class CyclicQueueUsingArray
    {
        [TestMethod]
        public void QueueTest()
        {
            CyclicQueue Q = new CyclicQueue(5);
            Q.Enqueue(1);
            Assert.AreEqual(1, Q.Front);
            Q.Enqueue(2);
            Assert.AreEqual(1, Q.Front);
            Assert.AreEqual(2, Q.Rear);
            Q.Enqueue(3);
            Assert.AreEqual(1, Q.Front);
            Assert.AreEqual(3, Q.Rear);

            Assert.AreEqual(1, Q.Dequeue());
            Assert.AreEqual(2, Q.Front);
            Assert.AreEqual(3, Q.Rear);

            Assert.AreEqual(2, Q.Dequeue());
            Assert.AreEqual(3, Q.Front);
            Assert.AreEqual(3, Q.Rear);

            Assert.AreEqual(3, Q.Dequeue());
        }

        [TestMethod]
        public void QueueOverflowTest()
        {
            CyclicQueue Q = new CyclicQueue(5);
            Q.Enqueue(1);
            Q.Enqueue(2);
            Q.Enqueue(3);
            Q.Enqueue(4);
            Q.Enqueue(5);
            Assert.AreEqual(1, Q.Dequeue());
            Q.Enqueue(6);
            Assert.AreEqual(2, Q.Front);
            Assert.AreEqual(6, Q.Rear);
        }
    }

    public class CyclicQueue
    {
        public int[] _buffer;
        private int _start;
        private int _end;
        private int _size;
        private int _maxSize;
        public CyclicQueue(int maxSize)
        {
            _maxSize = maxSize;
            _buffer = new int[_maxSize];
            _start = 0;
            _end = 0;
        }
        public void Enqueue(int newValue)
        {
            if (_size == _maxSize)
            {
                throw new OutOfMemoryException("Queue at full capacity");
            }
            if (_size > 0)
            {
                _end = (_end + 1) % _maxSize;
            }
            _buffer[_end] = newValue;
            _size++;
        }

        public int Dequeue()
        {
            if (_size == 0)
            {
                throw new IndexOutOfRangeException("Queue is empty");
            }

            int result = _buffer[_start];
            _size--;

            _start = (_start + 1) % _maxSize;
            return result;
        }
        public int Front
        {
            get { return _buffer[_start]; }
        }

        public int Rear
        {
            get { return _buffer[_end]; }
        }
    }
}

Yay exceptions!

Though it isn't reservered for execution-ending errors, I would avoid throwing an OutOfMemoryException, because usually it signals something very inconvient indeed. IndexOutOfRangeException also seems inappropriate: I would probably use InvalidOperationException in both Enqueue and Dequeue.

The constructor could do with a check to ensure maxSize is positive, so that it doesn't throw with a cryptic error.

Front and End don't throw on an empty buffer, and probably should.

API and Encapsulation

There is no reason this class couldn't be generic: I would make it so. Indeed, because it stores ints at the moment, it would not be clear to a consumer looking at the members that Front and Rear return elements rather than indexes.

I'd expect a Count => _size property, so that the Queue can be used for the sort of things Queues tend to be used (e.g. in your previous questions).

Do you have a particular use-case in mind for exposing _buffer? What good can come of this? There is no information made available as to the content of the buffer, so it can't be used for anything because nothing except the CyclicQueue knows how to use it. I would strongly suggest making this private (also, if it is public, it should ideally following the ProperCamelCase naming conventions like your other public members).

Since this is a non-resizing queue, the MaxSize probably ought to be public information. I'd also consider making it MaxSize => _buffer.Length to reduce redundancy. You could also make _buffer readonly to signal it's not meant to be changed to the maintainers.

Correctness

I'm not sure the _size > 0 check in Enqueue works. Consider a sequence of Enqueue, (Dequeue, Enqueue)*n on a newly created CyclicQueue: this will leave _end = 0 while _start is incremented. I think this can be resolved by setting _end = _maxSize - 1 in the constructor and removing the check.

Misc/Boring Stuff

You could do with a little more white-space between things... at the very least, be consistent with your between-member spacing.
You could do with tests which test the exceptions (i.e. checking it throws if you try to enqueue/dequeue too many things.
As always, inline-documentation would be appreciated. This should describe the non-resizing nature of the queue, when it throws exceptions, and resolve the confusion concerning whether Front is an index or element.


Replace array element with multiplication of neighbors in Scala Given an array of integers, update the index with multiplication of previous and next integers,

 Input: 2 , 3, 4, 5, 6
 Output: 2*3, 2*4, 3*5, 4*6, 5*6

Following is a scala implementation for the same. Kindly review.

import scala.util.Random

object NeighborMultiplication extends App {
  val numbers = List.fill(10)(Random.nextInt(10))

  println(numbers mkString ",")

  def multiplication(l: List[Int], carryOver: Int = 1, useCarryOver: Boolean = false ): List[Int] = l match {
    case Nil => List()
    case x::Nil => List(carryOver * x)
    case x::y::Nil => List(carryOver * x * y, y * x)
    case x::y::z::Nil => List(carryOver * x * y, x * z, y * z)
    case x::y::z::tail =>
      if (useCarryOver) List(carryOver * y, x * z, y * tail.head) ++ multiplication(tail, z, true)
      else List(x * y, x * z, y * tail.head) ++ multiplication(tail, z, true)
  }

  println(multiplication(numbers).mkString(","))

}

The tricky part of this problem is how to handle the special cases for the start and end of the list, as well as how to handle short lists with fewer than three elements.

The fact that you need to consider up to three elements at a time means that you need a lot of base cases for recursion, though.  It's also undesirable to expose the special cases in the form of the carryOver and useCarryOver parameters.

A better approach would be to take advantage of the List.sliding function.  (Note that .sliding may produce a group with just two elements instead of three, if the input lst has length two.)

def multiplication(lst: List[Int]): List[Int] = lst match {
  case _::_::_ =>
    (lst.head :: lst ++ List(lst.last))
      .sliding(3)
      .map(group => group.head * group.last)
      .toList
  case _ => lst
}


Determine a poem's rhyme scheme The TopCoder question specifies (in summary):

  Two words are considered rhyming if they have the same ending pattern (defined below). This comparison is case-insensitive (see Example 1.)

  An ending pattern is a substring of a word such that:

  The word ends with that substring,
  The substring contains exactly one contiguous string of vowels,
  The first letter of the substring is a vowel, and
  The substring must either be the whole string, or the letter immediately preceding the start of the substring must be a nonvowel.

  For example, the ending pattern of "bought" is "ought", the ending pattern of "spying" would be "ying", and the ending pattern of "all" would be "all". (Note that "spy" has no vowels, and thus is not a legal word.)

  Example 1:

{"     ",
 "Measure your height",
 "AND WEIGHT      ",
 "said the doctor",
 "",
 "And make sure to take your pills",
 "   to   cure   your    ills",
 "Every",
 "DAY"}

  Returns: " aab ccde"
  Even though "height" and "weight" don't actually rhyme in English, they do by the rules laid out above.

I want to get general feedback and feedback about if I can make this more Ruby-like, if there are any obvious inefficiencies, and if I need to organize my class differently.

#!/usr/bin/env ruby -w
require 'set'

class Poetry
  def find_ending(str)
    vowels = Set.new(['a', 'e', 'i', 'o', 'u'])
    vowel_seen = false
    ending = ""
    str.reverse.each_char.with_index do |c, i|
      if vowels.include?(c) or
         (c == 'y' and i != 0 and i != str.length-1)
        vowel_seen = true
        ending = c + ending
      elsif !vowel_seen
        ending = c + ending
      else
        break
      end
    end
    ending
  end

  def rhyme_scheme(poem_array)
    scheme = ""
    next_label = 'a'
    ending_labels = {}
    poem_array.each do |line|
      if line.strip.empty?
        scheme += ' '
      else
        word = line.split(' ')[-1]
        ending = self.find_ending(word.downcase)
        label = ending_labels[ending]
        if label.nil?
          scheme += next_label
          puts ending_labels
          ending_labels[ending] = next_label
          puts ending_labels
          if next_label == 'z'
            next_label = 'A'
          else
            next_label = next_label.next
          end
        else
          scheme += label
        end
      end
    end
    scheme
  end
end

poetry = Poetry.new

poem_array = ["I hope this problem",
 "is a whole lot better than",
 "this stupid haiku"]

puts "'#{poetry.rhyme_scheme(poem_array)}'"

I am a bit late, I hope the answer is still useful. Some notes:

First, my tradicional advice: don't use imperative programming! I mean, it's ok if you are writing assembler or C, but in a language like Ruby you're squandering its potential. Try to learn to program in functional style, favouring the good old math expressions (with its abstractions, immutability, higher-order functions, modulatization and many other cool things).
I'd use regular expressions to get the rhyming substring of each verse, no custom code can beat their compactness and expression power.
There are two ways of solving the problem, one is traversing the verses only once, folding the verses into an output. The second one is more modular, traverse the verses to build the intermediate data structures needed to calculate the solution. This second approach is, in this case, less verbose.

I'd write (second approach):

def rhyme_scheme(verses)
  ending_regexp = /((?:[aeiou]|\By\B)+[^aeiou]*)$/
  endings = verses.map { |verse| verse.strip.downcase[ending_regexp, 1] }
  ending_scheme_pairs = endings.uniq.compact.zip([*"a".."z", *"A".."Z"])
  schemes = ending_scheme_pairs.to_h.merge(nil => " ")
  schemes.values_at(*endings).join
end


Pool volume estimator I am looking to make the following code more efficient since it it rather lengthy. However I need to include in the code constructor classes that include accessor methods for the container number, height, radius 1, radius 2, and a method that computes the liquid that the container can hold. So I am looking to leave these concepts in there but make it more efficient if it is possible.

public class PoolCleanRequest {
final static double PI = 3.14;
private String num;
private double height;
private String height1;
private String height2;
private String radius1;
private String radius2;
private double r1;
private double r2;
private String rad2;
private String rad3;
public PoolCleanRequest(String num1){
    num = num1;
}
public void setHeight(double h){
    height=h;
    height1=Double.toString(h);
    height2=Double.toString(h);
}
public void setR1(double rr){
    r1 = rr;
    radius1=Double.toString(rr);
    radius2=Double.toString(rr);
}
public void setR2(double rrr){
    r2 = rrr;
    rad2=Double.toString(rrr);
    rad3=Double.toString(rrr);
}
public String getNum(){
    return num;
}
public String getHeight1(){
   return height1;
}
public String getHeight2(){
   return height2;
}
public String getRadius1(){
    return radius1;
}
public String getRadius2(){
    return radius2;
}
public String getRad1(){
    return rad2;
}
public String getRad3(){
    return rad3;
}
public double getPoolCost(){
    return ((PI*height/3)*(r1*r1+r2*r2+r1*r2))/1000;
}
public static void main( String[] args ) {
    PoolCleanRequest pool1 = new PoolCleanRequest("1.0");
    PoolCleanRequest pool2 = new PoolCleanRequest("2.0");
    PoolCleanRequest height1 = new PoolCleanRequest("20.0");
    PoolCleanRequest height2= new PoolCleanRequest("15.0");
    PoolCleanRequest radius1 = new PoolCleanRequest("3.0");
    PoolCleanRequest radius2 = new PoolCleanRequest("5.0");
    PoolCleanRequest rad1 = new PoolCleanRequest("2.0");
    PoolCleanRequest rad3 = new PoolCleanRequest("4.0");
    pool1.setHeight(20.0);
    height1.setHeight(20.0);
    height2.setHeight(15.0);
    radius1.setR1(3.0);
    radius2.setR1(5.0);
    rad3.setR2(4.0);
    rad1.setR2(2.0);
    pool1.setR1(3.0);
    pool1.setR2(5.0);
    pool2.setHeight(15.0);
    pool2.setR1(2.0);
    pool2.setR2(4.0);
    System.out.println("Container Num: " + pool1.getNum());
    System.out.println("H: " + height1.getHeight1());
    System.out.println("R1: " + radius1.getRadius1());
    System.out.println("R2: " + radius2.getRadius2());
    System.out.println("Litres: " + pool1.getPoolCost());
    System.out.println();
    System.out.println("Container Num: " + pool2.getNum());
    System.out.println("H: "+ height2.getHeight2());
    System.out.println("R1: " + rad1.getRad1());
    System.out.println("R2: " + rad3.getRad3());
    System.out.println("Litres: " +  pool2.getPoolCost());
  }
}

This code gives the correct following output:

Container Num: 1.0
H: 20.0
R1: 3.0
R2: 5.0
Litres: 1.0257333333333334

Container Num: 2.0
H: 15.0
R1: 2.0
R2: 4.0
Litres: 0.43960000000000005

The first problem that jumps out at me is that many of your variables are "stringly typed".  This class is a calculator, right?  Why are you storing your numbers as strings rather than as doubles?

Why does the constructor take a num parameter, and what does it mean?

There is no need to redefine π.  Just use Math.PI.

Why is there a getPoolCost() method?  Your code would be more flexible if you offered a method to calculate the volume, and a separate method to calculate the cost based on the volume.

I don't think that there is much of a reason to define a bunch of setters and getters.  Just define one function that accepts all of the dimensions as parameters, and be done with it.


Short Erlang function to demonstrate hd and tl I'm simultaneously learning Erlang and writing a short guide on list operations. Here is a trivial function I wrote to demonstrate hd and tl:

headAndTail(List) ->
  Head = hd(List),
  Tail = tl(List),
  {Head, Tail}.

This works, but it throws "Warning: variable 'Head' is unused." I know this function could be written with {hd(List), tl(List)}, but is the way I did it considered bad practice?

First, your code sample does not produce warnings under the Erlang compiler (I'm almost certain that specific version does not matter).

$ cat bar.erl
-module(bar).

-export([headAndTail/1]).

headAndTail(List) ->
  Head = hd(List),
  Tail = tl(List),
  {Head, Tail}.

$ erlc bar.erl
$

Now, regarding bad practises, the only comment is that what you are trying to do with hd and tl is usually done with pattern matching in functional programming languages:

headAndTail([Head|Tail]) ->
    {Head, Tail}.

Another minor bad practise is that Erlang's common naming convention does not use CamelCase for function names, preferring names_with_underscores instead. CamelCase is ok for variables.


Checking substring in 8086 ASM I have tried like this to check a substring in a mainstring in 8086.  Is there any shorter way of doing this?  My implementation seems lengthy.

DATA SEGMENT
STR1 DB 'MADAM'
LEN1 DW ($-STR1);       storing the length of STR1
STR2 DB 'MADAA'
LEN2 DW ($-STR2);       stroing the length of STR2
DATA ENDS

CODE SEGMENT

LEA SI, STR1
LEA DI, STR2
MOV DX, LEN1
MOV CX, LEN2
CMP CX, DX;             comparing main & substring length
JA EXIT;                if substring size is bigger than there is no chance to be found it in main string
JE SAMELENGTH;          if main & sub string both have same length the we can compare them directly
JB FIND;                general case (substring length < mainstring length): we can apply our main process

SAMELENGTH:
        CLD
        REPE CMPSB
        JNE RED
        JMP GREEN

FIND:
        MOV AL, [SI];   storing the ascii value of current character of mainstring
        MOV AH, [DI];   storing the ascii value of current character of substring
        CMP AL,AH;      comparing both character
        JE CHECK;
        JNE NOTEQL

NOTEQL:
        INC SI;         if both character don't match then we would point to the next char of main string
        DEC DX;         DX keeps track of how many character of mainstring is left to process
        CMP DX, 0000H;  checking if there are any character left in the main string for further comparison
        JE RED;         if no character is left in main string then obviously the substring doesn't exists in main string
        JMP FIND

CHECK:
        MOV CX, LEN2;   CX is used internally for REPE CMPSB. So storing length of the substring in CX would limit the number of characters for comparison to exact length of substring.
        ;               For example to compare between "madam" & "ada" we need to compare *ada* portion of main string with substring ada, no more, no less
        MOV SP, SI;     storing the index of current character of main string so if the following REPE CMPSB find mismatch then the process can be started over from the next character of main string (SEE line 1 of TEMPRED) by going to TEMPRED > FIND
        ADD SP, 0001H
        CLD
        REPE CMPSB
        JNE TEMPRED
        JMP GREEN

TEMPRED:;               substring not found starting from the current character of main string, but it is possible to find match if we start from next character in main string
        MOV SI,SP;      going to the next character of main string (after REPE CMPSB of CHECK segment)
        DEC DX
        LEA DI, STR2;   reloading substring index in DI (after REPE CMPSB of CHECK segment)
        JMP FIND;       if a character matches but the following substring mismatches in main string then we start over the same process from the next character of main string by going to FIND segment

GREEN:
        MOV BX, 0001H;  substring found
        JMP EXIT

RED:
        MOV BX, 0000H;  substring not found
        JMP EXIT

EXIT:
    CODE ENDS
    END
    RET

There are a number of things that could be improved with this code.  I hope you find these suggestions helpful.

Specify which assembler

Unlike C or Python, there are a great many variations in assembler syntax, even for the same architecture, such as the x86 of this code.  Generally, it's useful to note which assembler, which target processor and which OS (if any) in the comments at the top of the file.  In this case, it looked most like 16-bit TASM, so that's the compiler I used to test this code.

Use an ASSUME directive

The code would not assemble for me until I added an ASSUME directive. The ASSUME directive doesn't actually generate any code.  It simply specifies which assumptions the assembler should make when generating the output.  It also helps human readers of your code understand the intended context.  In this particular case, I added this line just after the CODE SEGMENT declaration:

ASSUME CS:CODE, DS:DATA, ES:DATA

The CS and DS assumptions are obvious, but the ES assumption is less so. However, the code uses the CMPSB instruction and based on the context, this means an implicit assumption that ES also points to the DATA segment.  In my case, (emulated 16-bit DOS), I had to add a few statements to the start of the code to actually load the DS and ES segment registers appropriately.

Avoid instructions outside any segment

The EXIT code currently looks like this:

EXIT:
        CODE ENDS
        END
        RET

The problem is that the CODE ENDS closes the CODE segment and the END directive tells the assembler that there is no more code and thus the RET instruction may or may not be assembled, and may or may not actually be placed in the CODE segment.  You probably meant instead to do this:

EXIT:
        RET
        CODE ENDS
        END

Eliminate convoluted branching

Avoid needless branching.  They make your code harder to read and slower to execute.  For example, the code currently has this:

        JA EXIT
        JE SAMELENGTH
        JB FIND

SAMELENGTH:
        CLD
        REPE CMPSB
        JNE RED
        JMP GREEN
        ; ... code elided
GREEN:
        MOV BX, 0001H;  substring found
        JMP EXIT

RED:
        MOV BX, 0000H;  substring not found
        JMP EXIT
EXIT:

This could be very much simplified:

        JA EXIT
        JB FIND
        ;  fall through to same length
SAMELENGTH:
        XOR BX,BX      ; assume string not found
        CLD
        REPE CMPSB
        JNE EXIT
        INC BX         ; indicate that string was found
EXIT:

There are a number of such simplifications possible with little effort.

Know your instruction set

The code currently has this set of instructions

        DEC DX
        CMP DX, 0000H
        JE RED

However, the DEC instruction already sets the Z flag, so the CMP instruction is not needed.

Use REPNE SCASB as appropriate

The code at the location FIND is largely the same as would have been done by using REPNE SCASB.  The only difference is in which registers are used.  The code you have isn't necessarily wrong, but it could probably be shorter.

Avoid using SP as a general register

Just after CHECK, the code saves a copy of the pointer (not an index as the comment falsely claims) to the SP register.  However, SP is a stack pointer, so this code can only be used in an environment in which the stack is not used.  That could be the case, but it makes the code much less portable to code it that way, especially because the AX or BX registers could just as easily have been used here.

Consider using standard length lines

The comments in the code are very long and the semicolon is right after the instruction. Neither of these things are necessarily wrong, but they are different from the usual convention which is to align the semicolon character in some column and making sure that lines are no more than 72 characters long (some use 78).


Select random row using PDO What do you think of this way to generate a random number from the total rows in a table? I would like to create a page that random.php precisely generates a random ID and shows it.

require 'includes/config.php';

$pdo->query('SELECT id FROM xxxx');
$pdo->resultset();
$total = $pdo->rowCount();

//echo random id
$id = rand(1, $total);

$pdo->query('SELECT * FROM xxxxx WHERE id = :id AND status = :status');
$pdo->bind(':id', $id);
$pdo->bind(':status', 1);
$rand = $pdo->single();

echo $rand['id'];

This query will return a random id from the DB with one simple SQL statement instead of using PHPs built in rand function.

MySQL:

$pdo->query('SELECT id FROM xxxxx ORDER BY RAND() LIMIT 1');

MS SQL:

$pdo->query('SELECT id FROM xxxxxx order by NEWID()');

I'm not entirely sure what database you're using, but this would eliminate a few lines of code.

Also, I'm not sure if you're using a database class to wrap around PHPs PDO class but I generally build an array and store my bound parameters in it then execute it. Looks a little neater.


Efficiently displaying elements in an array I wrote the below method to list out the elements of an array, as well as adding the Oxford comma to the list if desired. However, my for-loop looks, perhaps, a little convoluted. Is there a better way to handle clearly both the Oxford comma and the ability to add a conjunction?

function listOut($array = array(), $oxford = false, $conjunction = 'and') {
    $count = is_array($array) ? count($array) : 0;

    switch($count) {
        case 0: // Either not an array or an empty array
            return '';

        case 1: // Only one element in the array, return it
            return $array[0];

        case 2: // Two elements; join them together with the conjunction
            return "{$array[0]} {$conjunction} {$array[1]}";

        default: // At least two elements, join with commas and conjunction
            $s = ", ";
            $t = "";
            for($i = 0; $i < $count; $i++) {

                // Add a conjunction if it's the last element in the array
                if($i + 1 === $count) {
                    $s = $oxford ? $s . "$conjunction " : " $conjunction ";
                }

                // Only prepend comma if not the first element
                if($i > 0) {
                    $t .= $s;
                }

                // Add the element to the return string
                $t .= $array[$i];
            }
            return $t;
    }
}

$array1 = array('orange');
$array2 = array('white', 'gold');
$array3 = array('black', 'white', 'purple');

echo listOut($array1); // orange
echo listOut($array2); // white and gold
echo listOut($array3); // black, white and purple

echo listOut($array1, true); // orange
echo listOut($array2, true); // white and gold
echo listOut($array3, true); // black, white, and purple

You can effectively get rid of the switch statement as well as the is_array check in the count ternary if you do the check in the beginning (and exit early at that - so not wasting any cycles).

You can also get rid of

# Only prepend comma if not the first element
if($i > 0) {
    $output .= $separator;
}

If you add a ternary concat to $output (at the expense of gaining 2 opcode calls over the if/else). This would be the function after cleaning it:

function listOut($array = array(), $oxford = false, $conjunction = 'and', $separator = ", ") {
    if(!is_array($array)){ # We're expecting an array
        return false;
    }

    $count = count($array) ?: 0;
    $output = "";

    if($count == 2){ return implode(" $conjunction ", $array); }

    for($i = 0; $i < $count; $i++) {
        # Add a conjunction if it's the last element in the array
        if($i + 1 === $count) {
            $separator = $oxford ? $separator . "$conjunction " : " $conjunction ";
        }

        # Add the element to the return string
        $output .= ($i > 0 ? $separator : "") . $array[$i];
    }

    return $output;
}

and the test outputs:

$array = 'blue';
$array1 = array('orange');
$array2 = array('white', 'gold');
$array3 = array('black', 'white', 'purple');
$array4 = array('black', 'white', 'purple', 'gold');

echo listOut($array) . "
";
echo listOut($array1) . "
"; // orange
echo listOut($array2) . "
"; // white and gold
echo listOut($array3, true) . "
"; // black, white and purple
echo listOut($array4) . "
"; // black, white and purple

Results:

false
orange
white and gold
black, white, and purple
black, white, purple and gold


Changing a div that contains a view link I've written some code to load a section of my application between views. While my code works, I'm wondering if there is a better way to do it. (partial views perhaps?) I currently load the views, then show/hide them depending on a button click.

@*Sidebar*@
@Html.Partial("_AdministrationSidebar")

@*PAGE CONTENT*@

@*page-content-wrapper*@

@*wrapper End*@

This is a nice simple approach to an SPA. I assume you are returning partial pages from your actions.

A slight improvement would be to hide all pages before showing. This way you don't have to touch the other functions when adding a new page.

Add a class to the page divs, like class='partial-page'. Then in your show page function:

function createItem(Id) {
   $(".partial-page").hide();
   $("#create").load("RolesAdmin/Create/" + Id);
}

You could also make things more generic and allow for a query string. e.g.

function showPage(url, param){
   $(".partial-page").hide();
   if ($.isPlainObject(param) {
     $("#create").load(url + '?' + $.param(param));
   }
   else {
     $("#create").load(url + '/' + param);
   }
}

function createItem(Id) {
   showPage("RolesAdmin/Create", id)
}

function showSomeOtherPage(id, name) {
   // RolesAdmin/SomeOtherPage?id=123&name=freddy
   showPage("RolesAdmin/SomeOtherPage", {id:id, name:name})
}

Further, you can support the back button by using jquery.history.js


Cut Wood / Facebook Interview This question was asked through the Facebook interview and the suggested solution is solving via binary search.

I have put the binary search solution but it still looks too complicated.

Question:

  Given an int array wood representing the length of n pieces of wood and an int k. It is required to cut these pieces of wood such that more or equal to
  k pieces of the same length len are cut. What is the longest len you can get?

Input: wood = [5, 9, 7], k = 3
    Output: 5
    Explanation:
    5 -> 5
    9 -> 5 + 4
    7 -> 5 + 2

Input: wood = [5, 9, 7], k = 4
Output: 4
Explanation:
5 -> 4 + 1
9 -> 4 * 2 + 1
7 -> 4 + 3

package AmazonOthers.src;

import java.util.Arrays;

public class CutWood {

    public static void main(String[] args) {
        int[] array1 = {5,9,7};
        int k = 3;
        System.out.println( woodCut(array1,k) ) ;

        int[] array2 = {124,232,456};
        Arrays.sort(array2);
        int k2 = 7;
        System.out.println( woodCut(array2,k2) ) ;

        int[] array3 = {3,6,7,11};
        Arrays.sort(array3);
        int k3 = 8;
        System.out.println( woodCut(array3,k3) ) ;

    }

    private static int woodCut(int[] array1,int k) {

        int l = 0;
        int h = Integer.MAX_VALUE;

        while(l<h){
            int mid = l + (h-l)/2;
            if(isValid(array1, mid, k)){
                l = mid +1;
            }else{
                h=mid-1;
            }
        }
        return h;

    }

    private static boolean isValid(int[] array1, int mid, int k) {

        int count = 0;
        for(int val: array1){
            count+=val/mid;
        }
        return count>=k;
    }

}

Overall I think you have about the simplest you could hope for. The only thing I'd do is set your bounds better. You know that you can do min(array)/ceil(k/len(array)) and you know you at most you can do max(array)/floor(k/len(array)).

I was going to suggest going for extra credit with a more sophisticated algorithm, but I don't think you can beat what the binary search approach. At worst you're going to be 32 or 64 * O(n). Given the simplicity of the algorithm I think that will likely beat sorting and all the alternatives I can think of either explicitly or implicitly do a sort which would be O(n log n).


Granular vs a aggregated tests when dealing with arrays I write code in a Test-Driven way and I often build my functions starting with tests for the easy edge cases.
For example, given a flat array of Items that have a category property, return an array of Groups, each built with all the items for one of the categories in the input.
One edge case I find helpful to test first is the behavior when the items are all from the same category. This gives me a chance to see how the Group should look like.
In this case there are (at least) two approaches one can take: two granular tests vs. a single one.
Granular
One could write two granular tests for these conditions:

Given an array with items all from the same category, the output should be an array with a single group
A group should contain all the items for its category and only those

In Swift, these tests might look something like this
func testGroupingArrayOfSameCategoryReturnsOneGroup() {
    let items = [
        Item(name: "a", category: .foo),
        Item(name: "b", category: .foo),
        Item(name: "c", category: .foo),
    ]

    let groups = groupByCategory(items)

    XCTAssertEqual(groups.count, 1)
}

func testGroupingBuildsGroupWithAllItemsForCategory() throws {
    let items = [
        Item(name: "a", category: .foo),
        Item(name: "b", category: .foo),
        Item(name: "c", category: .foo),
    ]

    let groups = groupByCategory(items)

    let group = try XCTUnwrap(groups.first)
    XCTAssertEqual(group.category, .foo)
    XCTAssertEqual(group.items.count, 3)
    XCTAssertEqual(group.items[0].name, "a")
    XCTAssertEqual(group.items[1].name, "b")
    XCTAssertEqual(group.items[2].name, "c")
}

I like how the two behaviors of having a 1:1 match with groups and categories and how the groups are built are separated, but I see a lot of duplication between those tests.
Aggregated
A different approach would be to check both facets of the behavior in the same test.
func testGroupingArrayOfSameCategoryReturnsOneGroupWithAllItemsForCategory() {
    let items = [
        Item(name: "a", category: .foo),
        Item(name: "b", category: .foo),
        Item(name: "c", category: .foo),
    ]

    let groups = groupByCategory(items)

    XCTAssertEqual(groups.count, 1)
    let group = try XCTUnwrap(groups.first)
    XCTAssertEqual(group.category, .foo)
    XCTAssertEqual(group.items.count, 3)
    XCTAssertEqual(group.items[0].name, "a")
    XCTAssertEqual(group.items[1].name, "b")
    XCTAssertEqual(group.items[2].name, "c")
}

Which do you think is clearer and why?

I prefer the second option: using a test for both facets of the behavior.
While it has a lot of assertions, I feel it covers the behavior well in one go, removing duplication.
From a pure code economy point of view, we achieve the same thoroughness as the two tests scenario with just one extra line of code.
Moreover, in the two tests example, the first test would pass regardless of how the output is built, as long as it contains only one element. Yes, the second test is there to make sure the Group is built properly, but I'm somehow uncomfortable with how close to a false positive the test that only checks the array count is.
If the second tests was to be removed, code coverage tools wouldn't register a change in coverage, but the thoroughness of what is checked would decrease.


Comparing run-length decoding algorithms I'm working on Ocaml.org 99 problems, and I solved the run-length decode one.

Here is the solution given by the site:

let decode l =
    let rec many acc n x =
            if n = 0 then acc else many (x :: acc) (n-1) x in
    let rec aux acc = function
            | [] -> acc
            | One x :: t -> aux (x :: acc) t
            | Many (n,x) :: t -> aux (many acc n x) t  in
    aux [] (List.rev l);;

And here is mine:

let decode l =
    let rec aux acc = function
            | [] -> acc
            | One elem :: t -> aux (elem :: acc) t
            | Many(cnt, elem) :: t -> if cnt > 0 then aux (elem::acc) (Many(cnt - 1, elem) :: t) else aux acc t in
    List.rev (aux [] l);;

Test:

type 'a rld =
    | One of 'a
    | Many of int * 'a;;

(* result expected : ["a"; "a"; "a"; "a"; "b"; "c"; "c"; "a"; "a"; "d"; "e"; "e"; "e"; "e"] *)
decode [Many (4,"a"); One "b"; Many (2,"c"); Many (2,"a"); One "d"; Many   (4,"e")];;

Could my solution become more optimal?

Elegance is a matter of taste, so you'll probably get different answers for that point. Personally, I find your solution easier to understand: it makes good use of the algebra defined through the rld type.

From an efficiency standpoint, your algorithm creates as Many values as the repetition count in the original one. That means that your algorithm will generate memory allocation. It will also have to match it again as many times through recursion, so there's a computational extra load as well. The solution proposed by the site is more efficient in that regard: it only allocates the necessary values to construct the resulting list, and only matches once each element of the input list.


Rendering logos for guitars and basses listed in a database Right now I'm still a beginner in databases and still learning all about it. I have a problem with the design of my database and I don't really know where to go from here. Basically I'm building a music store website that has guitars and bass as products. Now, I have a total of six brands in my database, including its corresponding logo images for displaying in a webpage. Both guitar and bass can use this database for retrieving the needed brand.

brandId  name  image
1         Ibanez    xyz.jpg
2         Fender    abc.jpg
3         Gibson    hjk.jpg
4         PRS        wyx.jpg
5         Musicman cde.jpg
6         ESP        ihl.jpg

Now, I would display those brands for guitar using asp repeater. In this case, the guitar webpage has no problem displaying all the brands because it has all products for that brand. Here is the aspx code for the guitar:

<asp:Content ID='Content1' ContentPlaceHolderID='ContentPlaceHolder1' Runat='Server'>

<asp:Repeater ID="Repeater1" runat="server" OnItemDataBound="repeater_ItemDataBound" ItemType="brand" SelectMethod="GetBrandData">
    <ItemTemplate>

             <asp:LinkButton ID="linkButton" OnClick="Repeater1_OnClick" runat="server" CommandArgument='<%# Item.brandId %>'>
                 <asp:Image ID="brandImage" runat="server" height="250px" width="300px" />
             </asp:LinkButton>

    </ItemTemplate>

And here is the code-behind for the guitar:

 protected void Page_Load(object sender, EventArgs e)
{

}

public void repeater_ItemDataBound(object sender, RepeaterItemEventArgs e)
{
    Image img = e.Item.FindControl("brandImage") as Image;
    img.ImageUrl = "../Images/Brands/" + ((brand)e.Item.DataItem).image;
}

public List<brand> GetBrandData()
{
    MusicStoreDBEntities obj = new MusicStoreDBEntities();
    List<brand> list = new List<brand>();
    list = (from g in obj.brands select g).ToList();

    return list;

}

Meanwhile the bass can only display three brands which is Ibanez,Fender and Musicman. This is where the problem starts. It basically has the same aspx code as the guitar but I've made this change to the sql query so that it can return only those three brands.

public List<brand> GetBrandData()
{
    MusicStoreDBEntities obj = new MusicStoreDBEntities();
    List<brand> list = new List<brand>();
    list = (from g in obj.brands where g.name=="Ibanez"||g.name=="Fender"||g.name=="Musicman" select g).ToList();

    return list;

}

I have multiple where clauses in that query so that i can just retrieve those three brands. I really feel like this is not the most efficient way and will cause problems if lets say, i wanted to add a seventh brand and I would need that to display on the bass webpage, then i would need to manually edit query everytime, which is not ideal. I wanted to know if there are other solutions for managing the query or solutions for the design aspect of the database.

I hope you guys can give me some input on this.

The database should know which as bass

Type
ID
name

1 guitar
2 bass

Brand
ID
name
image

where you link the Brand to the Type and a Brand

TypeBrand
IDtype
IDbrand

select Brand.Name
from Brand
join TypeBrand
  on Brand.ID = TypeBrand.IDbrand
 and TypeBrand.IDtype = 2


Any problems with this prime checker script? I have just made what seems to me to be the first reliable prime-checking program that works.  However, I am not sure if it is completely reliable, or if there are problems.  It relies on list-making more than math, and it can be slow for bigger numbers.  Do you think there are any problems?

def primeChecker(x):
    if x <= 1 or isinstance(x,float) == True:
        return str(x) + " is not prime"
    else:
        div = ["tepi"] #tepi denotes test element please ignore
        for i in range(1,x+1,1):
            if x % i == 0:
                div.append("d") #d denotes divisible
            else:
                div.append("n") #n denotes not divisible
        if div[1] == "d" and div[x] == "d" and div.count("n") == x - 2:
            return str(x) + " is prime"
        else:
            return str(x) + " is not prime"

def primeCheckList(d,e):
    for i in range(d,e + 1):
        print primeChecker(i)

Here are some of the tools I used in primeChecker() to see if it was working right.

print div #these are testing tools
print range(1,x+1,1)
print div[1]
print div[x]
print div[2:x]
print div.count("n")
print x - 2

edit: Barry's feedback

OK, based on what Barry told me, i was able to clean up and otherwise format my code to be more concise, no more lists needed. it seems to run smoother, and is otherwise easier to read. here it is:

def isPrime(x):
    if x <= 1 or isinstance(x,float):
        return False
    else:
        for divisor in range(2,x,1):
            if x % divisor == 0:
                return False
        return True

def primeChecker(n):
    if isPrime(n):
        return "%d is prime" % n
    else:
        return "%d is not prime" % n

def primeCheckList(d,e):
    for i in range(d,e + 1):
        print primeChecker(i)

any more feedback to give?

There is lots of room for improvement here. Let me give some broad guidelines:

First, Your function checking if its input is prime should return a boolean and probably be called something like isPrime or is_prime. The string you are returning is nice for being human readible, but the goal should be to build components that can be used elsewhere, and the function as written isn't really something you can build on. And given the boolean function, you can easily build your version on top of that:

def prime_checker(n):
    if is_prime(n):
        return "%d is prime" % n
    else:
        return "%d is not prime" % n

Second, there is some improper use of Booleans (and lack thereof).  You have isinstance(x,float) == True. The isinstance function already returns a boolean, there's no need to compare against True. That clause can just be if x <= 1 or isinstance(x, float):. But then, does it make sense that 2.7 is not prime? Maybe that should raise an Exception. You are also building up a list of size x consisting of values that are intended to indicate whether or not the value at that index divides x. You chose:

div.append("d") #d denotes divisible

Which you realized was cryptic because you had to comment what that meant. Rather than relying on a comment to explain what you're doing, it's much better for the code to comment itself. In this case, what should we append to indicate that a number is a true divisor? How about:

div.append(True)

Third, you're building up a long list of things that should be Booleans but are instead strings. But that is a whole lot of extra work for what we actually want to count: the number of factors. The simplest explanation for why it's extra work is that first you're walking through all potential divisors, and then you're walking through it again to count how many were actual divisors. Instead, you can do this in one go:

divisors = 0
for divisor in range(1, x+1):
    if x % divisor == 0:
        ++divisors
return divisors == 2 #because a number prime iff it has exactly 2 divisors, 1 and itself

Once you write it that way, this has lots of room for simple optimizations. For instance, obviously every integer is divisible by 1 and itself, so you can exclude those two:

for divisor in range(2, x):
    if x % divisor == 0:
        return False # because we have a divisor other than 1 or x
return True

After that, things get more interesting. For instance, let's say we're checking a number like 5077 for primality. If it is NOT prime, then there are two numbers, a and b, such that 1 < a <= b < n and a*b == n. What is the largest possible value for a as a function for n? I'll leave that as an exercise, but the last algorithm I proposed assumes the maximum possible value is x - 1, and we can do much better.


Ensure that ICommand implementation properly uses weak references I am building an implementation of System.Windows.Input.ICommand that is like the commonly used RelayCommand, but doesn't keep a strong reference to targets of the delegates it uses. Such a structure is useful for when a control wants to expose some of its functionality as a command (which can be assigned to a ViewModel). Using a traditional RelayCommand would cause a memory leak, because the RelayCommand would contain strong references to the delegates that it runs, which in turn contain strong references to the control. Here is the code:

public class WeakRelayCommand : ICommand
{
    public WeakRelayCommand(Action<object> executeCallback, Func<object, bool> canExecuteCallback = null)
    {
        ExecuteTargetReference = new WeakReference<object>(executeCallback.Target);
        if (canExecuteCallback != null)
            CanExecuteTargetReference = new WeakReference<object>(canExecuteCallback.Target);
        ExecuteCallbackInfo = executeCallback.GetMethodInfo();
        CanExecuteCallbackInfo = canExecuteCallback?.GetMethodInfo();
    }

    private WeakReference<object> ExecuteTargetReference;
    private WeakReference<object> CanExecuteTargetReference;
    private MethodInfo ExecuteCallbackInfo;
    private MethodInfo CanExecuteCallbackInfo;

    public event EventHandler CanExecuteChanged;

    public bool CanExecute(object parameter)
    {
        if (CanExecuteCallbackInfo == null)
            return true;
        object canExecuteTarget = null;
        if (CanExecuteTargetReference?.TryGetTarget(out canExecuteTarget) ?? false)
        {
            return (bool)CanExecuteCallbackInfo.Invoke(canExecuteTarget, new[] { parameter });
        }
        else
            return false; //Target of the delegate for CanExecute has been garbage collected
    }

    public void Execute(object parameter)
    {
        object executeTarget = null;
        if (ExecuteTargetReference?.TryGetTarget(out executeTarget) ?? false)
        {
            ExecuteCallbackInfo.Invoke(executeTarget, new[] { parameter });
        }
        //Cannot execute the command if the target of the Execute delegate has been garbage collected
    }

    public void RaiseCanExecuteChanged()
    {
        CanExecuteChanged?.Invoke(this, EventArgs.Empty);
    }
}

The basic summary is that instead of holding strong references to delegates, which in turn hold strong references to the delegates' targets of invocation, it instead holds a reference to the MethodInfo associated with the actual method of the delegate, and holds a WeakReference to the targets of invocation separately from the method. This way, the targets of invocation are free to be garbage collected by the garbage collector.

I have tested this implementation and it works just fine as an implementation of ICommand, and it appears to properly use WeakReferences to achieve my objective. Can I get a verification that this is in fact the case?

The implementation works fine if the passed delegate is an instance method, because that prevents the method from being garbage collected.

But that is not necessarily the case. Lambda expression, for instance, will be garbage collected sometime because they are not referenced somewhere else.
That may result in strange not reproducible bugs that are hard to debug.

The following code (with a simplified version of the WeakRelayCommand class) shows that the weak reference approach does not work for static methods and lambda expressions with closures.

void Main()
{
    var longLiveObject = new DummyObject();
    var commands = GetActions(longLiveObject);

    GC.Collect();
    GC.WaitForPendingFinalizers();

    foreach (var command in commands)
        Console.WriteLine(command.Name + "\t IsAlive:" + command.IsAlive);
}

private static WeakAction[] GetActions(DummyObject longLiveObject)
{
    var localObj = new DummyObject();
    return new []
    {
        new WeakAction("InstanceMethod (short living)", localObj.InstanceMethod),
        new WeakAction("InstanceMethod (long living)", longLiveObject.InstanceMethod),
        new WeakAction("StaticMethod", DummyObject.StaticMethod),
        new WeakAction("LamdaExpression (without closure)", o => { }),
        new WeakAction("LamdaExpression (with closure)", o => localObj.InstanceMethod(o)),
    };
}

// Define other methods and classes here
public class DummyObject
{
    public void InstanceMethod(object obj) { }
    public static void StaticMethod(object obj) {}
}

public class WeakAction
{
    private readonly WeakReference<object> _targetReference;
    private readonly MethodInfo _callbackInfo;

    public WeakAction(string name, Action<object> executeCallback)
    {
        Name = name;
        _targetReference = new WeakReference<object>(executeCallback.Target);
        _callbackInfo = executeCallback.GetMethodInfo();
    }

    public string Name { get; }
    public bool IsAlive
    {
        get
        {
            object obj;
            return _targetReference.TryGetTarget(out obj);
        }
    }
}

Output:

InstanceMethod (short living)        IsAlive:False
InstanceMethod (long living)         IsAlive:True
StaticMethod                        IsAlive:False
LamdaExpression (without closure)   IsAlive:True
LamdaExpression (with closure)      IsAlive:False


Adding or Replacing a Page item in a QueryString in C#. Can this code be improved?  I'm avoiding using the QueryString collection because the query string may contain eg: OrderBy=Column1&OrderBy=Column2 and it didn't seem helpful to start with those switched into QueryString["OrderBy"]=="Column1,Column2";

    var query = "";
    if (urlHelper.RequestContext.HttpContext.Request.Url != null)
    {
        query = urlHelper.RequestContext.HttpContext.Request.Url.Query.TrimStart('?');
        if (query.Length > 0)
        {
            var kvp =
                query.Split('&').Select(x => x.Split('='))
                    .Select(x => new { Key = x[0], Value = (x.Length > 0) ? x[1] : ""})
                    .Where(x => x.Key != "page")
                    .ToList();
            kvp.Add(new { Key = "page", Value = number.ToString() });
            query = String.Join("&", kvp.Select(x => x.Key + "=" + x.Value).ToArray());
        }
        else query = String.Format("page={0}", number);
    }

I think that yes this code can be improved.

use a regular expression:

            var newQuery = Regex.Replace(currentQuery, @"(?i)(page=\d*&?)", "").TrimEnd('&');
            if (newQuery.Length > 0)
                newQuery += string.Format("&Page={0}", currentPageNo);
            else
                newQuery = string.Format("Page={0}", currentPageNo);


The first thing that will help is to switch this from a recursive algorithm to an iterative one. This will prevent the stack overflow that prevents you from solving 25x25, and will be a bit faster to boot.

However to speed this up more, you will probably need to use a smarter algorithm. If you track what numbers are possible in each square, you will find that much of the time, there is only 1 possibility. In this case, you know what number goes there. You then can update all of the other squares in the same row, col, or box as the one you just filled in. To implement this efficiently, you would want to define a set (either a bitset or hashset) for what is available in each square, and use a heap to track which squares have the fewest remaining possibilities.


React typeahead component In order to get a better grasp of React I decided to create a typeahead component. I'd really love some feedback on this in order to improve, learn or avoid mistakes regarding my code. I'm unsure whether I should split up my components even more than I already did.

import React from 'react';
import ReactDOM from 'react-dom';
import SearchListItem from './search_list_item.jsx';

class SearchList extends React.Component {

  constructor(props) {
    super(props);

    this.state = {
      cursor: 0,
      data: ['Bananas', 'Apples', 'Ape', 'Oranges', 'Ora', 'Cherries', 'Pears', 'Kiwi'],
      searchedList: [],
      searchInput: '',
      showDropdown: true
    }

    this.handleOnchange = this.handleOnchange.bind(this);
    this.handleKeyPress = this.handleKeyPress.bind(this);
    this.handleClickedItem = this.handleClickedItem.bind(this);
  }

  handleOnchange(value) {
    this.setState({ showDropdown: true })
    this.setState({ searchInput: value });
    const dataItem = this.state.data
    let searchedListItems = dataItem.filter(dataItem => dataItem.includes(this.state.searchInput));
    var listToRender = [];
    if (searchedListItems) {
      listToRender.push(searchedListItems);
    } else {
      var itemToRemove = this.state.searchedListItems
      listToRender.remove(listToRender, itemToRemove);
    }
    this.setState({ searchedList: listToRender });
  }

  handleKeyPress(e) {
    const { cursor, searchedList } = this.state;
    const arrowUp = 38;
    const arrowDown = 40;
    const enter = 13;

    if (e.keyCode === arrowUp && cursor > 0) {
      this.setState( prevState => ({
        cursor: prevState.cursor - 1
      }))
    } else if (e.keyCode === arrowDown && cursor < searchedList[0].length - 1) {
      this.setState( prevState => ({
        cursor: prevState.cursor + 1
      }))
    } else if (e.keyCode === enter && cursor >= 0) {
      this.setState({
        searchInput: searchedList[0][this.state.cursor],
        showDropdown: false,
        cursor: 0
       });
    }
  }

  handleClickedItem(data) {
    this.setState({
      searchInput: data,
      showDropdown: false,
      cursor: 0
    })
  }

  submitForm(e) {
    e.preventDefault();
    alert
  }

  renderSearchList() {
    const { cursor } = this.state
    if (this.state.searchedList[0]) {
      const searchList = this.state.searchedList[0].map((data, index) => {
        return (
          <SearchListItem
            key={index}
            id={index}
            handleClickedItem={this.handleClickedItem.bind(null, data)}
            isActive={cursor === index ? 'active dropdown-li' : 'dropdown-li'}
            data={data} />
        );
      });
      return (

            {searchList}

      );
    }
  }

  render() {
    return (

      <form onSubmit={this.submitForm} >
        <input
          className="input"
          type="text"
          value={this.state.searchInput}
          onChange={event => this.handleOnchange(event.target.value)}
          onKeyDown={this.handleKeyPress} />
        </form>
        { this.state.showDropdown ? this.renderSearchList() :
 }

    );
  }
}

ReactDOM.render(<SearchList/>, document.getElementById('main'));

I think there are many things you could improve, I will try to review your code and give some pointers.

Instead of using React.Component, you should directly access Component:

import React, { Component } from 'react';

class SearchList extends Component {

In older examples, you could find it in the way you do it, but more recent example use this form.

Not sure why you call so many times the setState() method, it is not needed and not a good practice, as every time you call it React will re-render the component.

If you change some data that effects the visualization, you will find out that your component is rendered many times. Of course if you do not change something that impacts the visualization, you won't see, but the engine will run silently, until one day in the far future a small change will turn in such case.

handleOnchange(value) {
  const dataItem = this.state.data
  let searchedListItems = dataItem.filter(dataItem => dataItem.includes(this.state.searchInput));
  var listToRender = [];
  if (searchedListItems) {
    listToRender.push(searchedListItems);
  } else {
    var itemToRemove = this.state.searchedListItems
    listToRender.remove(listToRender, itemToRemove);
  }
  this.setState({
      showDropdown: true,
      searchInput: value,
      searchedList: listToRender
  });
}

About variable names: data is a very bad name for a variable, you should avoid. Believe me, I know as I used so many times and eventually I figured out I shouldn't do.

Maybe you can use allItems instead.

About the method logic.

Array.filter() will always return an array even empty. So your check:

if (searchedListItems) {

is always true, you should:

if (searchedListItems.length > 0) {

About: listToRender.remove(listToRender, itemToRemove)

listToRender should be an empty list, no way there are values inside, according to your code. Anyway I'm sure you never arrive here, because javascript Array does not have the method remove().

I think you can just improve your filter, if you want to remove more items.

this.setState({
    showDropdown: true,
    searchInput: value,
    searchedList: this.state.allItems
      .filter(dataItem =>
          (dataItem.includes(this.state.searchInput) ||
           this.state.searchedListItems.indexOf(dataItem) !== -1))
});

Or you could assign it directly, as you did't change the list before this line, but if you are going to add more logic the concat method should do the stuff. Then just access the array directly.

I don't know where you saw this thing but don't do it:

this.setState( prevState => ({
  cursor: prevState.cursor - 1
}))

You could just simply do:

this.setState({ cursor: this.state.cursor -1 })

Much better.

Another issue with the following method is that you duplicate to much code and use to many time the this.setState().

handleKeyPress(e) {
  const { cursor, searchedList } = this.state;
  const arrowUp = 38;
  const arrowDown = 40;
  const enter = 13;

  if (e.keyCode === arrowUp && cursor > 0) {
    this.setState( prevState => ({
      cursor: prevState.cursor - 1
    }))
  } else if (e.keyCode === arrowDown && cursor < searchedList[0].length - 1) {
    this.setState( prevState => ({
      cursor: prevState.cursor + 1
    }))
  } else if (e.keyCode === enter && cursor >= 0) {
    this.setState({
      searchInput: searchedList[0][this.state.cursor],
      showDropdown: false,
      cursor: 0
     });
  }
}

You don't have to do it:

handleKeyPress(e) {
  const {
      searchedList,
      arrowUp,          // you can move this in your state
      arrowDown,        // or better as module const
      enter             // but not inside the method.
  } = this.state;

  let cursorIncr = 1;
  let newState = {
      cursor: this.state.cursor
  };

  if (e.keyCode === arrowUp && cursor > 0) {
    newState.cursor -= 1;
  } else if (e.keyCode === arrowDown && cursor < searchedList[0].length - 1) {
    newState.cursor += 1;
  } else if (e.keyCode === enter && cursor >= 0) {
    newState.cursor = 0;
    newState["searchInput"] = searchedList[0][this.state.cursor];
    newState["showDropdown"] = false;
  }

  this.setState(newState);
}

And now I jump back a bit for this:

listToRender.push(searchedListItems);

I see you read the data like:

searchedList[0]...

Looks like you never care about more lists inside this array of arrays, and it's ok as you always have one.
So consider to use concat:

listToRender = listToRender.concat(searchedListItems);

Consider to move this method as a stateless component:

renderSearchList() {
  const { cursor } = this.state
  if (this.state.searchedList[0]) {
    const searchList = this.state.searchedList[0].map((data, index) =>  {          return (
          <SearchListItem
            key={index}
            id={index}
            handleClickedItem={this.handleClickedItem.bind(null, data)}
            isActive={cursor === index ? 'active dropdown-li' : 'dropdown-li'}
            data={data} />
        );
      });
    return (

          {searchList}

    );
  }
}

Like this:

const SearchListItems = ({handleClick, cursor, items}) => {
    const searchItems = items.map((item, index) => (<SearchListItem
                key={index}
                id={index}
                handleClickedItem={handleClick}
                isActive={cursor == index ? 'active dropdown-li' : 'dropdown-li'}
                data={item}/>));

    return (
{searchItems}
)
}

Just a function and it reads the props from the input parameter, using the spread operator of ecmascript-6.

You could have this stateless component in the same file, or place in another, as it is convenient for your use case.

Maybe you could have a better names.

Then change your render to use the new component:

render() {
   return (

    <form onSubmit={this.submitForm} >
      <input
        className="input"
        type="text"
        value={this.state.searchInput}
        onChange={event => this.handleOnchange(event.target.value)}
        onKeyDown={this.handleKeyPress} />
    </form>
    { this.state.showDropdown &&
           <SearchListItems
            handleClick={this.handleClickedItem.bind(null, data)}
            cursor={this.state.cursor}
            items={this.state.searchedList[0]}/>}

)
}


Ruby Twitter Search by punctuations With twitter search the punctuation and special characters will be considered part of the term you're searching for, so searching for '#twitter!' will return '#twitter!', "twitter?', '#twitter', etc. What this module does is check if the search includes any type of punctuation searched with and if it does you can sort the array to add those tweets first. This code does the trick but I feel like there might be a cleaner way to do this. Right now I am just using array.sort and setting the array attributes index to 0 if it matches the regex which is looking for a punctuation used in the search and matching it with the tweets.

require 'twitter'

module TwitterSearch
  extend self

  TWITTER_CLIENT = Twitter::REST::Client.new do |config|
    config.consumer_key        = ""
    config.consumer_secret     = ""
    config.access_token        = ""
    config.access_token_secret = ""
  end

  # search returns
  # Check out what @researchgoddess is up to at #sourcecon!
  # What a welcome from @SourceCon! Thanks @CareerBuilder for hosting.#
  # RT @JRoberts257: Happy hour at #SourceCon! Thanks @CareerBuilder for
  # Happy hour at #SourceCon! Thanks @CareerBuilder for sponsoring. ht
  # @RT @cybsearchjoe: #SourceCon is rocking
  # etc

  def search(text)
    tweets = TWITTER_CLIENT.search("#{text} filter:images", result_type: "recent").take(30).collect do |tweet|
        "#{tweet.text}"
    end
    # looks to see if there is puncuation at the end of the text "!.?{}[]" It will ignore the # at the beginning
    tweets = sort_tweets(text, tweets) if text[1..text.length] =~ /[[:punct:]]/
    puts tweets
  end

  # sorts tweets based off index given in match_phrase
  def sort_tweets(text, tweets)
    tweets.sort do |phrase, other_phrase|
      match_phrase(phrase, text, tweets) <=> match_phrase(other_phrase, text, tweets)
    end
  end

  # if phrase matches punc_text(text) the phrase will be inserted at the beginning of the array else it will return its previous index.
  def match_phrase(phrase, text, tweets)
    phrase.match(/#{punc_text(text)}/i).nil? ? tweets.index(phrase) + 1 : 0
  end

  # adds backslash to punctuation '#sourcecon//?|!|.'
  def punc_text(text)
    text[1..text.length].gsub(/([[:punct:]])/){|punc| "\\#{punc}"}
  end
end

TwitterSearch.search('#sourcecon!')

I don't think you can do too much better. The things I would have done differently would be:

Make the helper methods private using private_class_method ...
I'm not quite sure why you are stripping out the leading # in several places. If you know that all searches are going to start with # then I would strip it out at the beginning (or expect the caller not to pass it in).
if text[1..text.length] =~ /[[:punct:]]/ could be written as if text =~ /.[[:punct:]]/
text[1..text.length].gsub(/([[:punct:]])/){|punc| "\\#{punc}"} There are two better ways to do this (1) Using backreferences text[1..text.length].gsub(/([[:punct:]])/, '\\\1') but (2) it really looks like you are trying to duplicate RegExp.escape
phrase.match(/#{punc_text(text)}/i).nil?, I would think that phrase.downcase.include?(text.downcase) would be cleaner


Best Approach for Assignment Code in Node.js (ES6 style) Is there any way to improve this code? Using ES6 style i am using latest stable version of node.js.

const UserSchema = new mongoose.Schema({
date: string;
dateObj: {
    year: number,
    month: number,
    day: number
};
project: string;
task: string;
hours: number;
});

let data = new Data();
data.date = new Date(body.date);
    data.dateObj = {
        year: body.dateObj.year,
        month: body.dateObj.month,
        day: body.dateObj.day
    }
    data.project = body.project;
    data.task = body.task;
    data.hours = body.hours;
}

For flat objects (i.e. whose values are only primitive types), there's Object.assign(), which shallow-copies all enumerable keys and values from one object to the other:

Object.assign(data, body);

However, in your case, shallow-copying means the dateObj member of the body would get copied by reference, instead of creating a new object and recursively copying its keys and values.

You can also try using the spread operator ..., which was introduced recently:

let data = new Data();

Object.assign(data, {
    // Shallow-copy all keys and values from body:
    ...body,

    // Overwrite certain keys:
    date: new Date(body.date),
    dateObj: { ...body.dateObj },
});

Whether or not this code is readable or intuitive is up to you to decide.

Also, as a minor note, Object.assign returns the first object you pass to it, so instead of this:

let data = new Data();
Object.assign(data, { /* ...properties */ });

You can write this:

let data = Object.assign(new Data(), { /* ...properties */ });


Check IEnumerable Count Let's say I have a collection of expensive-to-generate objects stored in an IEnumerable:

IEnumerable<Expensive> expensiveObjects = CreateExpensiveIEnumerable();

Now I will iterate over everything in expensiveObjects and afterwards, I want to know if it was empty.

Here is how I am doing that currently.

var count = 0;
foreach(var e in expensiveObjects)
{
    count++;
    //other processing
}
if (count == 0)
{
    //do something
}

This works fine, but I'm asking here as a fishing expedition to figure out if there are any clever ways to implement this functionality without either:

iterating over at least part of the collection twice by using the Any() method
Performing a ToList() up-front on my collection

Any other suggestions or should I just stick with a simple counting variable?

I think your approach is reasonable if you don't want (or can't) have the whole collection in memory. One minor change: if you don't need the count, just whether the collection was empty or not, use a simple bool:

bool any = false;

foreach(var e in expensiveObjects)
{
    any = true;

    //other processing
}

if (any)
{
    //do something
}


Simple JavaScript HTML table generator This is my simple implementation on JS HTML table generator which generates a table based on an array of JS objects.

function Column(caption,position,isVisible,values){
  this.caption = caption;
  this.position = position;
  this.isVisible = isVisible;
  this.values = values;
}

Column.prototype.printValue = function (index){
      return  ""+ this.values[index] +"	";
};

var columnArray = [
                    new Column("Id",0,true,[1,2,3]),
                    new Column("Name",1,true,["Tim","Jim","Tom"]),
                    new Column("Date",2,true,["27-03-2014","01-01-2014","11-04-2015"])
                  ];

function columnSorter(columnA,columnB){
  if(columnA.position < columnB.position)
    return -1;
  if(columnA.position > columnB.position)
    return 1;
  return 0;
}

columnArray.sort(columnSorter);

function generateTable(columnArray){
  var htmlString = "

";

  for(var col in columnArray){
    htmlString += ""+columnArray[col].caption+"	";
  }

  htmlString += "

";

  htmlString += "
";
  for(var i=0;i<columnArray[0].values.length;i++){
    htmlString += "
";
    for(col in columnArray){
      htmlString += columnArray[col].printValue(i);
    }
    htmlString += "
";
  }
  htmlString +="
";

  htmlString += "

";
  return htmlString += "
";
}

Since position is a number, you can simplify your columnSorter function. It becomes a one-liner, so you could even do:

columnArray.sort(function(a, b) { return a.position - b.position; });

Note though that some people consider this less readable. It's definitely less explicit in what it does, so you may wish to explain it in a comment.
You're using a for..in construct. This will give unexpected results if I extend Array.prototype. Either use a regular for loop, or check for columnArray.hasOwnProperty(col) in each iteration.
Why add an empty <tfoot>?
You're closing your <thead> in the wrong spot. <tbody> and <tfoot> should be children of <table>, not children of <thead>, so close your <thead> earlier.
It doesn't make much sense to return x += y;. Either return x + y;, or do x += y; return x;.


Still an observer pattern? Usually when I see tutorials about Observer Pattern I see an unique method called notify, but I'm wondering. What if I have different methods that can be called in different moments but needs to notify the others when this happen? Like events, am I doing this wrong? or still begin the observer pattern?

#include <iostream>
#include <algorithm>
#include <vector>

class Observer
{
public:
    virtual void notifyBefore() = 0;
    virtual void notifyAfter() = 0;
};

class Subject
{
public:
    void attachObserver(Observer * observer)
    {
        observers.push_back(observer);
    }

    void detachObserver(Observer * observer)
    {
        auto index = std::find(observers.begin(), observers.end(), observer);
        if (index != observers.end())
        {
            observers.erase(index);
        }
    }

    virtual void notifyBefore()
    {
        for (auto current : observers)
        {
            current->notifyBefore();
        }
    }

    virtual void notifyAfter()
    {
        for (auto current : observers)
        {
            current->notifyAfter();
        }
    }
private:
    std::vector<Observer *> observers;
};

class ConcreteObserver : public Observer
{
public:
    void notifyBefore()
    {
        std::cout << "You called me before..." << std::endl;
    }

    void notifyAfter()
    {
        std::cout << "You called me after..." << std::endl;
    }
};

class ConcreteSubject : public Subject
{
public:

};

int main()
{
    auto subject = new ConcreteSubject;
    subject->attachObserver(new ConcreteObserver);

    subject->notifyBefore();

    for (int i = 0; i < 5; ++i)
        std::cout << i << std::endl;

    subject->notifyAfter();
}

Yes its still the observer pattern (details in my comment on the question).

But there are a few implementation details that are bad for a C++ context.

Good interface for your observer:

class Observer
{
public:
    virtual void notifyBefore() = 0;
    virtual void notifyAfter() = 0;
};

But you should also make the destructor virtual. It is quite probable that these objects will be destroyed via a pointer to the base class. Without a virtual destructor this is undefined behavior.

You have the basics of subject correct.

class Subject
{
public:
    void attachObserver(Observer * observer)
    {
        observers.push_back(observer);
    }

My one problem is here. You are passing a pointer to an observer. Pointers do not express ownership. So it is unclear for future users of your object if you should pass a dynamically allocated object or a a pointer to an automatic object. It becomes clear by reading your class that you are not taking ownership of the pointer and your class always assumes that it is not NULL. So in this case it is probably better to pass it by reference.

If there are cases were ownership can be passed then you should use smart pointers. A quick search on google for smart pointers or ownership semantics should give you lots of references.

    void attachObserver(Observer& observer)
    {
        observers.push_back(&observer);  // Save the address of the observer
                                         // It is the responsibility of the observer
                                         // to make sure it lives long enough or
                                         // detaches itself on destruction.
    }

Implementation:

class ConcreteObserver : public Observer
{
public:
    void notifyBefore()
    {
        std::cout << "You called me before..." << std::endl;
    }

    void notifyAfter()
    {
        std::cout << "You called me after..." << std::endl;
    }
};

In C++11 we added the override keyword. This should be used in case you refactor your class. This will help in finding locations were you have misnamed the functions:

 void notifyAfter() override ....

This is not Java

auto subject = new ConcreteSubject;
subject->attachObserver(new ConcreteObserver);

You should prefer to use automatic objects (that is perfect here). If you are going to use new then you should wrap the result in a smart pointer to make sure the allocation is exception safe.


Testing the performance of a loop for a vector of unique_ptr I wrote this simple program that tests how long it takes for a loop to iterate through a vector<unique_ptr<>>. I'm wondering if this a good way to perform such a speed test? Are there better methods to this?

Also, I can't seem to use myVector.max_size()(which is 1073741823) to allocate the maximum amount of entries for a vector. My program crashes even if I go above my current amount of 70 million.

Results:

> Basic took 0ms
> BasicR took 0ms
> Range took 0ms
> Iterator took 0ms
> IteratorR took 36ms

Program:

// main.cpp
#include <iostream>
#include <memory>
#include <vector>
#include <chrono>
#include <iterator>

namespace LoopType {
    enum Enum {
        Range,
        Basic,
        BasicR,
        Iterator,
        IteratorR
    };
}
class Test {
public:
    explicit Test(const int a) { mA = a; }
    int getNum() { return mA; }
private:
    int mA;
};

typedef std::vector<std::unique_ptr<Test>> TestVector;

// Loop Types
void RangeBasedForLoop(TestVector& vec) {
    for (auto& test : vec) {
        int num = test->getNum();
    }
}
void BasicLoop(TestVector& vec) {
    for (unsigned i = 0; i < vec.size(); ++i) {
        int num = vec[i]->getNum();
    }
}
void BasicLoopR(TestVector& vec) {
    for (unsigned i = vec.size() - 1; i > 0; --i) {
        int num = vec[i]->getNum();
    }
}
void IteratorLoop(TestVector& vec) {
    TestVector::iterator it;
    for (it = vec.begin(); it != vec.end(); ++it) {
        (*it)->getNum();
    }
}
void IteratorLoopR(TestVector& vec) {
    TestVector::reverse_iterator it;
    for (it = vec.rbegin(); it != vec.rend(); ++it) {
        (*it)->getNum();
    }
}

// Testing & Printing
long long TestLoop(TestVector& vec, void(*loop)(TestVector&)) {
    auto t1 = std::chrono::high_resolution_clock::now();
    loop(vec);
    auto t2 = std::chrono::high_resolution_clock::now();
    return std::chrono::duration_cast<std::chrono::milliseconds>(t2 - t1).count();
}
void PrintLoopTime(LoopType::Enum type, TestVector& vec) {
    switch (type) {
    case LoopType::Basic:
        std::cout << "Basic took " << TestLoop(vec, &BasicLoop) << "ms" << std::endl;
        break;

    case LoopType::BasicR:
        std::cout << "BasicR took " << TestLoop(vec, &BasicLoopR) << "ms" << std::endl;
        break;

    case LoopType::Range:
        std::cout << "Range took " << TestLoop(vec, &RangeBasedForLoop) << "ms" << std::endl;
        break;

    case LoopType::Iterator:
        std::cout << "Iterator took " << TestLoop(vec, &IteratorLoop) << "ms" << std::endl;
        break;

    case LoopType::IteratorR:
        std::cout << "IteratorR took " << TestLoop(vec, &IteratorLoopR) << "ms" << std::endl;
        break;
    }
}

int main() {
    const unsigned long seventyMillion = 70000000UL;
    std::cout << "Creating vector..." << std::endl;
    std::vector<std::unique_ptr<Test>> testVector;
    testVector.reserve(seventyMillion);
    for (unsigned long i = 0; i < seventyMillion; ++i) {
        testVector.emplace_back(std::make_unique<Test>(i));
    }
    std::cout << testVector.size() << " Elements" << std::endl << std::endl;
    std::cout << "Testing loops..." << std::endl << std::endl;

    PrintLoopTime(LoopType::Basic, testVector);
    PrintLoopTime(LoopType::BasicR, testVector);
    PrintLoopTime(LoopType::Range, testVector);
    PrintLoopTime(LoopType::Iterator, testVector);
    PrintLoopTime(LoopType::IteratorR, testVector);

    std::cout << std::endl << "Any key to exit...";
    std::cin.get();
    return EXIT_SUCCESS;
}

Most of your loops are executing in no time at all because the optimizer is eliminating them since they have no side effects.  You need to do something observable with them, like summing all elements and returning that sum from the function.  Declare a new variable int sum = 0 before the loops, replace the int num = lines with sum +=, then return sum; at the end of the function.  This will require changing the return type for all of them to int, and changing the type of pointer passed to TestLoop.

std::vector::max_size returns a theoretical maximum, not one determined on your current system specs.  In your case, it returns 0x3FFFFFFF, which is the maximum number of 32 bit pointers that can be stored in a 32 bit address program.  This max size also doesn't consider any additional storage space required by objects stored within the vector.  Every element you have is a unique_ptr, which when allocated will take 4 bytes for the int, plus additional overhead for the memory management system (so at least 12 or more likely 16 bytes).

Your program crashes with more then 70 million elements because it runs out of memory.


Drink price calculator I have just started a course on C and wanted to test what I have learned so far. I did so by making a small program that tells you how much you should charge for a shot of a spirit you put in.
The code will ask if you want to do a calculation.  It will follow up with a question about the name, price and size of a product and respond with the calculation.
I have just learned how to use a Struct and just worked on Arrays. My next step will be to try and implement arrays to store the Structs you made and have the option to return to previous input.
How did I do so far and what could I improve on?
#include <cs50.h>
#include <stdio.h>
#include <string.h>
#include <math.h>
#include <ctype.h>

//structs
typedef struct
{
    string name;
    float price;
    int size;
} Bottles;

//prototypes
Bottles get_newBottle(void);

//main
int main(void)
{
    string newBottles;
    //Ask user for input, repeat untill answered correctly.
   do
   {
       do
        {
            newBottles = get_string("Do you want to calculate a new shot y/n? ");
            if (strcmp(newBottles, "y") == 0)
            {
                //Build struct
                Bottles newBottle = get_newBottle();
                int shotSize = get_int("What is the size of your pour of %s in ML? ", newBottle.name);
                int bevCost = get_int("What percentage beverage cost do you want your drink to be? ");
                float shotCost = (newBottle.price / newBottle.size) * shotSize;
                float bevPrice = ((shotCost / bevCost) * 100) *1.21;
                printf("The cost of your shot of %s is %.2f Euro. \n", newBottle.name, shotCost);
                printf("At the cost of %.2f you will have to charge %.2f Euro including 21 percent VAT. \n", shotCost, bevPrice);

            }
            else if(strcmp(newBottles, "n") == 0)
            {
                printf("ok. doei!\n");
            }
        }
        while (! (strcmp(newBottles, "y") == 0) && ! (strcmp(newBottles, "n") == 0));
    }
    while (strcmp(newBottles, "y") == 0);
    return 0;
}

//extra functions
Bottles get_newBottle(void)
{
    string name = get_string("What is the name of your product? ");
    float price = get_float("What is the price of your product? ");
    int size = get_int("What is the size in ML of your product? ");

    Bottles newName = {name, price, size};
    return newName;
}

"Utility" libraries
It's time to take the training wheels off, so to speak. You need to stop using cs50.h and replace it with standard calls to the C libraries. The implementation for get_string has a careful, dynamically-allocated buffer algorithm that is really not necessary for most purposes and can be replaced with simpler calls that use a fixed-size buffer. Advantages of a fixed-size buffer, other than extremely reduced complexity, include that it can enforce "domain-specific limits" (i.e. your product name cannot exceed 128 characters, or whatever) where get_string cannot. get_string is especially overkill for your yes/no prompt.
For an overrun-safe, simple method that is a fixed-buffer replacement for get_string, consider fgets.
Spelling
untill -> until
Memory leaks
You call get_string in a loop, which allocates a new buffer every time; and you never free it. So this will happily eat all of your RAM if you let it.
Comparisons
! (strcmp(newBottles, "y") == 0)

should just be
strcmp(newBottles, "y") != 0

New structure initialization
It's somewhat unusual to be returning a structure instance from get_newBottle, though it isn't the end of the world. More typical is to see either

initialization of an existing structure passed by pointer, or
allocation and initialization of a new structure via malloc then returning a pointer.

This impacts performance more for huge structures, but it's still a good idea to learn.


Alert on partition growth I have a script that alerts me via email when a partition grows past a defined threshold in one hour. My script is run from cron hourly. Everything seems fine for 99% of the time, but occasionally I get an email from my script stating that growth in the past hour is something like ".08%", well below my current threshold of 0.75%. Is something wrong with my logic?

#Drive to monitor
monDrive=/opt
#Enter a number .01-100. i.e 50 = 50%, and 0.5 = 1/2 of a percent.
percentThreshold=0.75
#Email address to send alerts to
emailAlert=alerts@domain.com

##Do not modify anything below this point
prevReading=/root/diskcheckreading.txt

checkDiskUse() {
        df "$monDrive" | awk 'END{printf "%0.2f",$2/$1*100; exit}'
}

checkDiskFree() {
        df "$monDrive" | awk 'END{printf "%0.2f",$3/1048576; exit}'
}

#Check if file exists with previous reading
[ -f $prevReading ] && prev=$(cat $prevReading) || { checkDiskUse > $prevReading; exit; }

#Validate file content
printf "%0.2f" "$prev" > /dev/null || exit

#Get current percentage used of monitored drive
current=$(checkDiskUse)

#Get current free capacity of monitored drive
diskFree=$(checkDiskFree)

#Calculate the difference
results=$(bc -l <<< $current-$prev )
#Check if the difference exceeds the threshold
if  [[ $results > $percentThreshold ]]
         then printf "\"$monDrive\" has grown $results%% in the past hour.     Remaining capacity on $monDrive is $diskFree GB.\n\n$(hostname):$(pwd)/$(basename $0)" | mail -s "Disk Alert - $(hostname):$monDrive" $emailAlert
fi

#log current reading for next check
echo $current > $prevReading

Everything seems fine for 99% of the time, but occasionally I get an
  email from my script stating that growth in the past hour is something
  like ".08%", well below my current threshold of 0.75%. Is something
  wrong with my logic?

When the tools seem to defy your logic, play with simple examples.

The code that generates the message with ".08%" is this, roughly:

printf "\"$monDrive\" has grown $results%% in the past hour. | mail ...

where the $results variable comes from:

results=$(bc -l <<< $current-$prev )

and the threshold is checked with:

if  [[ $results > $percentThreshold ]]

Now let's play a little:

$ [[ 0.08 > 0.75 ]] && echo true || echo false
false
$ [[ .08 > 0.75 ]] && echo true || echo false
true

A-ha! Apparently, 0.08 is smaller than 0.75, as expected but .08, with the leading zero omitted, is not smaller than 0.75, at least in Bash.

But why do you have .08 instead of 0.08 in the first place?
Let's play a bit more:

$ bc -l <<< 2.81-0.79
2.02
$ bc -l <<< 0.81-0.79
.02
$ bc -l <<< 0-0.79
-.79

Ah. So when the result is below 1, apparently, bc drops the leading zero.

So how to fix this? It seems there's no obvious way to prevent bc from dropping the leading zero. But as you're already doing many calculations using awk, you can move this calculation there too:

results=$(awk '{print $1 - $2}' <<< "$current $prev")

Note the space between $current and $prev,
so that they can be used as $1 and $2 in awk.

Code review

   df "$monDrive" | awk 'END{printf "%0.2f",$2/$1*100; exit}'

In the END block of awk you don't need to exit,
as the command will exit anyway.
I also recommend to add more spaces to make the expression easier to read:

    df "$monDrive" | awk 'END { printf "%0.2f", $2 / $1 * 100 }'

This seems completely pointless, you can delete it:

#Validate file content
printf "%0.2f" "$prev" > /dev/null || exit