Vectorizing Two Sample Kolmogorov-Smirnov test I'm implementing the Two-sample Kolmogorov-Smirnov test in MATLAB. I must admit I know very little of formal statistics and was simply trying to implement the description in wikipedia's page.

Right now I take as input the two vectors to be compared x1 and x2, and optionally the p-value that the check will run on. It outputs 1 if the vectors pass the test and 0 if they do not.

A couple of questions I have:

  Is there a better way to compute the empirical distribution functions?

Right now I evaluate the empirical distribution function at the points defined in the vector t. In my naive approximation, I made it go from the smallest value of the vectors to the largest, using twice as many points as the vector with the most points. I have no idea is this is the best way to compute it, and suspect the vector doesn't have to be nearly as dense.

  Can I vectorize the search through the t vector?

Right now I have a for loop that goes through every value of tand computes the empirical distribution function in that way. It seems that there might be a way to vectorize this operation further and get rid of the for loop. Haven't figured out a good way to do it however.

And of course, any other suggestions are welcome!

My code is below:

function [OUT] = k_stest(x1,x2,p)

% Set default p-value
    if nargin == 2
        p = 0.05;
    end

% Compute lenghts
    N1 = length(x1);
    N2 = length(x2);

% Set vector t over which the empirical distribution function will be computed.
    t = linspace(min(min([x1 x2])),max(max([x1 x2])),2*max([N1 N2]));

% Initialize the statistic, negative values guarantees it will be overwritten by first call.
    D = -1;

% Set c from tabulated values
    if p == 0.10
        c = 1.22;
    elseif p == 0.05
        c = 1.36;
    elseif p == 0.025
        c = 1.48;
    elseif p == 0.01
        c = 1.63;
    elseif p == 0.005
        c = 1.73;
    elseif p == 0.001
        c = 1.95;
    else
        disp('Invalid p-value. Only p = 0.10 0.05 0.025 0.01 0.005 0.001 are supported')
        return
    end

% Search though the vector t, computing the empirical distribution function for each t,
% and overwriting the statistic if a higher value is found.
    for i = 1:length(t)
        F1 = sum(x1<=t(i))/N1;
        F2 = sum(x2<=t(i))/N2;
        if abs(F2-F1) > D
            D = abs(F2-F1);
        end
    end

% Compare the statistic to determine if the samples pass or fail.
    if D == -1;
       disp('Error, invalid input vectors')
       return
    elseif D > c*sqrt((N1+N2)/(N1*N2));
        OUT = 0;
    else
        OUT = 1;
    end

end

I'll take this in chronological order, from the top down. I think I have covered all aspects of the code =)

function [OUT] = k_stest(x1,x2,p)

This looks nice, but it would be better if you had spaces between the input arguments. Also, k_stest is not a very good name, as it's hard to understand what it does just be looking at the name. I read in another review that "You don't need the brackets around the output variable, but you should always include it". I don't necessarily agree,  I usually omit them, but that's just a matter of personal preferences.

% Set default p-value
    if nargin == 2
        p = 0.05;
    end

This is OK. Depending on how you call the function you might add some additional checks. Is p a scalar? Are x1 and x2 vectors? This is not necessary, but can be useful if you want to use this function a long time from now and don't remember the specifics. assert can be very useful here.

% Compute lenghts
    N1 = length(x1);
    N2 = length(x2);

numel (number of elements) is better than length. It's more robust and a lot faster for long vectors.

% Set vector t over which the empirical distribution function will be computed.
    t = linspace(min(min([x1 x2])),max(max([x1 x2])),2*max([N1 N2]));

This is dangerous, and a bit messy.

[x1 x2] assumes both x1 and x2 are row-vectors, i.e. horizontal vectors. A lot of functions in Matlab returns vectors as column-vectors i.e. vertical vectors by default, including the colon operator (:).

If you happen to call this function with two vertical vectors instead of two horizontal vectors, this would fail (due to [x1 x2]).

Instead, you should do:

x12 = [x1(:); x2(:)];
t = linspace(min(x12), max(x12), 2*max([N1, N2]));

This way you're converting x1 and x2 to vertical vectors, and concatenate them vertically, prior to the linspace call. Now you only need one call to max and min.

The way you're initializing D is OK!

% Set c from tabulated values
    if p == 0.10
        c = 1.22;
    elseif p == 0.05
        c = 1.36;
    ...

Comparing floating point values using == is dangerous. For instance 0.1 + 0.2 == 0.3 will return false. Instead, you should compare the values with the desired value, but with some tolerance (for instance eps).

    if abs(p - 0.10) < eps
        c = 1.22;
    elseif abs(p-0.05) < eps
        c = 1.36;

If two values are closer to each other than +/- eps then they are essentially equal.

% Search though the vector t, computing the empirical distribution function for each t,
% and overwriting the statistic if a higher value is found.
    for i = 1:length(t)
        F1 = sum(x1<=t(i))/N1;
        F2 = sum(x2<=t(i))/N2;
        if abs(F2-F1) > D
            D = abs(F2-F1);
        end
    end

You are right, vectorization is the way to go! Use bsxfun to create a logical matrix where each element that satisfy the condition x1 <= t(ii) are true and the others are false. Then sum the entire mask, and take the maximum difference.

mask1 = bsxfun(@le, x1(:), t);
mask2 = bsxfun(@le, x2(:), t);
s1 = sum(mask1) / N1;
s2 = sum(mask2) / N2;
D = max(abs(s1 - s2));

% Compare the statistic to determine if the samples pass or fail.
    if D == -1;
       disp('Error, invalid input vectors')
       return
    elseif D > c*sqrt((N1+N2)/(N1*N2));
        OUT = 0;
    else
        OUT = 1;
    end
end

The check for invalid input vectors should be in the start of the function. I can't see a reason why you want it in the end instead. And as far as I can tell it will never fail (if the input is invalid then you will get an error further up).

Instead of the if and elseif, you can simply do:

OUT = D <= c*sqrt((N1+N2)/(N1*N2));

To sum it all up, your function can be written like this. I've omitted the comment for simplicity. You should include them in your code.

function OUT = k_stest(x1, x2, p)

if nargin == 2
    p = 0.05;
end

% Add check for invalid input vectors!

N1 = numel(x1);
N2 = numel(x2);

x12 = [x1(:); x2(:)];
t = linspace(min(x12), max(x12), 2*max([N1, N2]));

if abs(p - 0.10) < eps
    c = 1.22;
elseif abs(p-0.05) < eps
    c = 1.36;
% Continue with the rest of the values
%
end

mask1 = bsxfun(@le, x1(:), t);
mask2 = bsxfun(@le, x2(:), t);
s1 = sum(mask1) / N1;
s2 = sum(mask2) / N2;
D = max(abs(s1 - s2));

OUT = D <= c*sqrt((N1+N2)/(N1*N2));

end


Starting with asynchronous JS and promises I've learn to code almost alone, so I really need some ideas/advice, as it started to get really difficult to advance.
Because this is mostly server side, I can't provide a running snippet, but yet it's a short snippet.
Code
When I press a UI button, it fetches a resource, and this should create a set of list items. Simple enough:
const btn = document.getElementById("btn")
const ul = document.getElementById("ul")
const putItems = (items)=>{items.forEach(item=>ul.innerHTML=`
<li>${item.msg}</li>`)}

btn.addEventListener("click", ()=>{
    fetch("https://localhost:3000/todos")
        .then(res=>res.json())
        .then(d=>putItems(d))
        .catch(e=>console.log(e))
})

On the server side, the main function and imports are:
const { MongoClient } = require("mongodb")
const cors = require('cors')
app.use(cors())

async function callDB(){   //get data from mongoDB
    const call = await MongoClient.connect( uri, {useNewUrlParser:true, useUnifiedTopology:true})
    let result
    try{
        result = await call
            .db("todos")
            .collection("items")
            .find({})
            .toArray()
    } catch(e){
        console.log(e)
    }

    return result;
}

app.get("/todos", cors(), (req, res)=>{
    const items = callDB().then(d=>res.json(d)).catch(e=>console.log(e))
});

This code imports dotenv to hide my sensible parts, and uses express I removed those imports just to be concise.
Mainly, I'd like to know if my asynchronous practice makes sense, or not. And I'm also getting a CORS error, that couldn't fix after an hour. That would help too.

There is a problem regarding error handling: in callDB() you catch all exceptions and log them in the console without passing them on to the caller. Either you should rethrow the exception (or a modified version that is more helpful for the caller) or not catch it at all. Since the MongoClient.connect() can also fail, I would move it into the try/catch as well:
async function getDataFromMongoDB() {  // renamed so you don't need the comment
    try{
        const connection = await MongoClient.connect( uri, {
            useNewUrlParser: true,
            useUnifiedTopology: true
        });
        return await connection
            .db("todos")
            .collection("items")
            .find({})
            .toArray();
    } catch(e) {
        console.log(e);
        throw e; // or throw something else
    }
}

This would be even more concise if you would extract the creation of the connection into its own function. However, I think you would like to write the whole function in one expression after the return, you would have to write it like this:
return await (await getConnection()).db("todos")...

The first await is required to keep error handling in this function.
The request handler can also make use of async/await:
app.get("/todos", cors(), async (req, res)=> {
    try {
        const response = await callDB();
        res.json(response);
    }
    catch (e) {
        console.log(e);
    }
});


Implementing Luhn's algorithm to validate a credit card number in Java I must implement Luhn's algorithm in a program to determine whether a given credit card number is valid or not, in addition I must print the name of the company that offers that credit card number. I believe this requires taking in the input as a string and converting each digit and testing if the number of digits in the input is in the valid range, which is 13 to 16.

I have paste my code below, would it be possible to have it reviewed and to receive feedback on whether or not my methods are correct and on issues such as how I may resolve my java.lang.NullPointerException. This would all help me continue in the right direction.

CreditCard.java

import java.util.Scanner;

public class CreditCard {

    private int evenSum;
    private int oddSum;
    private int sum;
    private String ccNumber;
    private String company;

    public boolean isDivisibleBy10() {
        return (getEvenSum() + getOddSum()) %10 == 0;
    }

    public boolean isValid() {
        return (validateNumber() && validateCompany());
    }

    public boolean validateCompany() {

        char[] ccArray = ccNumber.toCharArray();

        if(Character.getNumericValue(ccArray[0]) == 4) {
          company = "Visa";
          return true;
        }

        if(Character.getNumericValue(ccArray[0]) == 5) {
          company = "MasterCard";
          return true;
        }

        if(Character.getNumericValue(ccArray[0]) == 37) {
          company = "American Express";
          return true;
        }

        if(Character.getNumericValue(ccArray[0]) == 6) {
          company = "Discovery";
          return true;
        }
        return false;
      }

    public boolean validateLength() {
        String cardNumber = "";
        for (int  i = 0; i >= 16; i++){
          cardNumber += "1";
          while(i <= 16) {
            if (i >= 13) {
              System.out.println("valid length");
            } else {
              System.out.println("invalid length");
              break;
            }
          }
        }
        return false;
    }

    public boolean validateNumber() {
        // Luhn's algorithm
        long cardNumber = 0;
        long n = cardNumber;
        int sum = 0;
        int count = 0;

        while (n > 0)
        {
            int digit = (int)(n % 10);
            count++;
            if (count % 2 == 1){
                sum = sum + digit;
            }
            else if (digit<5){
                sum = sum + 2 * digit;
            }
            else{
                sum = sum + (2 * digit) - 9;
            }
            n = n/10;
        }
        return (sum % 10 == 0);
    }

    public boolean validateSums() {
        return isDivisibleBy10();
    }

    public CreditCard(String num) {
        Scanner input = new Scanner(System.in);
        System.out.println("Enter credit card number: ");
        num = input.nextLine();
        num = "0;";
    }

    public int getEvenSum() {
        return evenSum;
    }

    public int getOddSum() {
        return oddSum;
    }

    public int getSum() {
        sum = evenSum + oddSum;
        return sum;
    }

    public String getCcNumber() {
        Scanner input = new Scanner(System.in);
        System.out.println("Enter a number for a credit card number: ");
        ccNumber = input.nextLine();
        return ccNumber;
    }

    public String getCompany() {
        return company;
    }
}

tester.java

import java.util.Scanner;

        public class tester {

            public static void main(String[] args) {

                CreditCard cC = new CreditCard(null);

                System.out.print(cC.isValid());

            }
        }

Ordering

The ordering is a bit mixed. Try to an ordering such as: variables, constructor, public methods, private methods. There's no real convention for that, but try to tell the reader the story about your class. As for instance the member vars are the protagonists, the Constructor is the entry of a book, and the methods are the chapters.

Probable bugs

Even and odd sum are never assigned

simplify

In validateCompany, you can assign the char once and use a switch-statement.
In validateLength: String has a length() method.
In isDivisibleBy10: You call getEvenSum and getOddSum, add both values up, even though you have a getSum method.

concerns

The validateCompany method assigns the company. On one hand, the developer who uses your api would never figure that out. Also, you have to call validate, otherwise you wouldn't get the company
The constructor gets a credit card number passed. At the same time it reads from the console. And then ignores the input and assignes "0;" to the local variable. Only pass the number, that should be enough, the type shouldn't care about how the number is loaded.
getCcNumber: This method too should not read from the console. It should return the value which was passed to the constructor.

public methods

The only method you call in your tester class (which should start with a capitalized T), only calls the isValid() method. Reduce the visibility of all other methods. Depending on how the type is used, the user of the API will see too many methods, he only needs one. And beside that: It's a common principle to hide as much implementation as you can, also known as 'information hiding'

Hope this helps,
slowy


Refactoring digital clock - JavaScript ES5 Functional Programming I'm making a digital clock with ES5 ( no arrow stuff and so ). The location and temperature I'll do later.
How can I improve/refactor the current code that I have with functional programming?

CodePen: https://codepen.io/Aurelian/pen/opaxqx?editors=1010

This is the HTML:

United Kingdom, Manchester

NAN

This is the JS:

// Get te location
// Make the : blink

//var location = document.querySelector('.clock-location');

function abbrev(a,b) {
  return a.substr(0,b);
}

 function checkTime(i) {
      if (i < 10) {
        i = "0" + i
      };
      return i;
  }

function displayTime() {

  var clockTime = document.querySelector('.clock-time');
  var clockDate = document.querySelector('.clock-date');
  var clockDay = document.querySelector('.clock-day');
  var clockWeekdays = document.querySelector('.clock-weekdays');
  while(clockWeekdays.firstChild) clockWeekdays.removeChild(clockWeekdays.firstChild);

  var date = new Date();

  var weekday = new Array(7);
  weekday[0] = "Sunday";
  weekday[1] = "Monday";
  weekday[2] = "Tuesday";
  weekday[3] = "Wednesday";
  weekday[4] = "Thursday";
  weekday[5] = "Friday";
  weekday[6] = "Saturday";

  var monthNames = ["January", "February", "March", "April", "May", "June",
  "July", "August", "September", "October", "November", "December"
];

  var singleDay = [];

  for (var i = 0; i < weekday.length; i++) {
    var day = weekday[i];
    var li = document.createElement('li');
    li.textContent = abbrev(day,3);
    if (i === date.getDay()) {
        li.classList.add('active');
    }
    clockWeekdays.appendChild(li);
  }

  // Mechanics
  var todayDay = weekday[date.getDay()];
  var h = date.getHours();
  var m = checkTime(date.getMinutes());
  var s = checkTime(date.getSeconds());

  var year = date.getFullYear();
  var month = date.getMonth();

  // Display
  clockDay.textContent = abbrev(todayDay,3);
  clockTime.textContent = h + ":" + m + ":" + s;
  clockDate.textContent = monthNames[month] + " / " +  year;

}

window.setInterval(displayTime, 1000);

//displayTime();

// Get the time

//Loop Throw weekdays - show 3 letters

CodePen: https://codepen.io/Aurelian/pen/opaxqx?editors=1010

You have almost everything in a single function, displayTime.

I'd start by refactoring the date-to-string manipulations into separate functions and leaving only DOM manipulation code in displayTime:

// qs :: String -> Element
// formatTime :: Date -> String
// formatYearMonth :: Date -> String
function displayTime() {
  let date = new Date();
  // handle weekdays here
  qs('.clock-time').textContent = formatTime(date);
  qs('.clock-date').textContent = formatYearMonth(date);
}

formatTime would be more readable IMO if we named your checkTime function zeroPad and used mm/ss for double-digit strings:

function formatTime(date) {
  let zeroPad = (i) => ((i < 10) ? "0" : "") + i; // I know you said no arrow functions, but I can't resist

  let h = date.getHours();
  let mm = zeroPad(date.getMinutes());
  let ss = zeroPad(date.getSeconds());
  return `${h}:${mm}:${ss}`;
}

The weekday indicators can be created once and only get updated as necessary. This makes it obvious what part is dynamic:

let weekdayLIs = Array.from(qs('.clock-weekdays').children);
// I'm no fond of using the index here, can be changed to inspect the

function isCurrentWeekday(idx) { return idx === date.getDay(); }

weekdayLIs.forEach((li, idx) => {
  li.classList.toggle('active', isCurrentWeekday(idx));
});

I'm not sure what's the point of displaying the current weekday in two places, but if you insist:

qs('.clock-day').textContent = weekdayLIs.find((li,idx) => isCurrentWeekday(idx)).textContent;

As for creating the
s corresponding to the weekdays, you can simply list them in HTML; this has the benefit of having all the structure defined in one place:

Sun

Mon

Tue

Wed

Thu

Fri

Sat

https://codepen.io/anon/pen/zpyRjm?editors=1010


Returning a more specific class I would appreciate some feedback on my design of a few classes to query the Trakt.tv API.

public abstract class TraktQuery
{
    private const string baseUrl = "http://api.trakt.tv/";
    private const string apiKey = "[API key here]";

    protected abstract string QueryUrl { get; }
    protected abstract List<string> QueryParameters { get; }
    protected abstract TraktResponse ParseResponse(string response);

    public TraktResponse PerformQuery()
    {
        //exception handling removed for brevity
        WebClient webClient = new WebClient();
        string url = baseUrl + QueryUrl + "/" + apiKey + "/" + string.Join("/", QueryParameters);
        string response = webClient.DownloadString(url);
        return ParseResponse(response);
    }
}

public class TraktQueryEpisodeSummary : TraktQuery
{
    private string _showName;
    private int _season;
    private int _episodeNumber;

    public TraktQueryEpisodeSummary(string showName, int season, int episodeNumber)
    {
        _showName = showName;
        _season = season;
        _episodeNumber = episodeNumber;
    }

    protected override string QueryUrl
    {
        get { return "show/episode/summary.json"; }
    }

    protected override List<string> QueryParameters
    {
        get
        {
            List<string> paramters = new List<string>();
            paramters.Add(_showName);
            paramters.Add("" + _season);
            paramters.Add("" + _episodeNumber);
            return paramters;
        }
    }

    protected override TraktResponse ParseResponse(string response)
    {
        return JsonConvert.DeserializeObject<TraktResponseEpisodeSummary>(response);
    }

public class TraktResponseEpisodeSummary : TraktResponse
{
    public TraktShowInformation show { get; set; }
    public TraktEpisode episode { get; set; }
}

Currently TraktResponse is an empty abstract class but I'll probably add to it soon.

This code is then used as follows:

class Program
{
    static void Main(string[] args)
    {

        TraktQueryEpisodeSummary query = new TraktQueryEpisodeSummary("House", 1, 1);
        TraktResponseEpisodeSummary summary = (TraktResponseEpisodeSummary)query.PerformQuery();

        Console.WriteLine("Title: " + summary.show.title);
        Console.WriteLine("Poster image: " + summary.show.images.poster);
        Console.WriteLine("Genres: " + string.Join(", ", summary.show.genres));
        Console.WriteLine("Episode name: " + summary.episode.title);
        Console.WriteLine("Overview: " + summary.episode.overview);
    }
}

What makes me think my design isn't quite right is that I have to cast from TraktResponse to the more specific TraktResponseEpisodeSummary in order to access the information I need. What I would like to do is make TraktQueryEpisodeSummary.ParseResponse() return a TraktResponseEpisodeSummary instead of a TraktResponse however I'm aware that C# doesn't support return type covariance which I think is what this is.

Are my concerns well-founded or not? Is there a better design that avoids this casting or is it perfectly normal and acceptable?

Dreza has outlined the correct approach (the same used in IComparable<T>). You can make your class generic, with a type constraint:

public abstract class TraktQuery<TResponse> where TResponse : TraktResponse

And change every occurence of TraktResponse to TResponse:

protected abstract TResponse ParseResponse(string response);

and

public TResponse PerformQuery()
{
    //...
}

Then, your deriving class can be defined as

public class TraktQueryEpisodeSummary : TraktQuery<TraktResponseEpisodeSummary>
{
    //...
    protected override TraktResponseEpisodeSummary ParseResponse(string response)
    {
        //...
    }
    //...
}

Voila, no cast necessary:

TraktResponseEpisodeSummary summary = query.PerformQuery();

Further suggestions:

Use collection and object initializers, for instance in the overridden QueryParameters:

 return new List<string> {_showName, _season.ToString(), _episodeNumber.ToString()};

Or turn them into an IEnumerable<string> and use an iterator block:

protected override IEnumerable<string> QueryParameters
{
    get
    {
        yield return _showName;
        yield return _season.ToString();
        yield return _episodeNumber.ToString();
    }
}

Be careful when concatenating strings to form a URL like this:

string url = baseUrl + QueryUrl + "/" + apiKey + "/" + string.Join("/",QueryParameters);

It might be better to

Use the Uri and/or UriBuilder class
Put the first three parts into an sequence, then call IEnumerable<T>.Concat and finally string.Join
There's probably some even better options.


If she floats then she is not a witch like we thought Continuing my C++ saga, this is the third project for my CS1 class:

  Buoyancy is the ability of an object to float.  Archimedes' principle
  states that the buoyant force is equal to the weight of the fluid that
  is displaced by the submerged object.  The buoyant force can be
  computed by

  $$ F_b = V \cdot y $$

  where \$ F_b \$ is the buoyant force. \$ V \$ is the volume of the
  submerged object, and \$ y \$ is the specific weight of the fluid.  If
  \$ F_b \$ is greater than or equal to the weight of the object, then
  it will float, otherwise it will sink.

  Write a program that inputs the weight (in pounds) and radius (in
  feet) of a sphere and outputs whether the sphere will sink or float in
  water.  Use \$ y = 62.4 \dfrac{\text{lb}}{\text{ft}^3} \$ as the
  specific weight of water.  The volume of a sphere is computed by \$ \left( \dfrac{4}{3} \right) \pi r^3\$.

Additionally, I must have a loop in this program that allows the user to run it as many times as they want to. I have to ask the user each time whether they want to continue.

It is also supposed to be fully commented, which I think I have done within reason.

buoyancy.cpp:

/**
 * @file buoyancy.cpp
 * @brief Calculates if a sphere will sink or float in water given the weight and radius
 * @author syb0rg
 * @date 9/12/14
 */

#include <iostream>
#include <limits>
#include <math.h>

/**
 * Sphere
 * @var weight The weight of the sphere
 * @var radius The height of the sphere
 */
struct Sphere
{
    double weight = 0;
    double radius = 0;
};

/**
 * Resets the command line so we can input data again, and signals user to re-enter proper data
 */
void resetConsole()
{
    std::cin.clear(); // clear the error flag that was set so that future I/O operations will work correctly
    std::cin.ignore(std::numeric_limits<std::streamsize>::max(), '\n'); // skips to the next newline
    std::cout << "Invalid input.  Please enter a positive number: ";
}

int main()
{
    constexpr double waterBuoyancy = 62.4;
    Sphere obj;
    char again = '\0';
    do
    {
        // get input for height, re-read input if not a positive number
        std::cout << "Enter the weight in pounds: ";
        while(!(std::cin >> obj.weight) || obj.weight < 0) resetConsole();

        // get input for radius, re-read input if not a positive number
        std::cout << "Enter the radius in inches: ";
        while(!(std::cin >> obj.radius) || obj.radius < 0) resetConsole();

        double volume = ((4/3) * M_PI * pow(obj.radius, 3));
        double bouyantForce = volume * waterBuoyancy;

        if (obj.weight <= bouyantForce) std::cout << "The object will float." << std::endl;
        else std::cout << "The object will sink." << std::endl;

        std::cout << "Run the program again (y/N): ";
        std::cin.get();  // absorb newline character from previous input
        std::cin.get(again);
        again = tolower(again);
        if ('\n' == again) again = 'n';
    } while (again == 'y');
}

There are several small things that you can improve:

double weight = 0; works fine, but double weight = 0.0; would be more pedantic since weight is a double.
By the way, the following line is a good example to illustrate the benefits of literals pedantry:

double volume = ((4/3) * M_PI * pow(obj.radius, 3));

Here, 4/3 performs an integer division and not a floating point one; that expression will yield 1 instead of 1.333333333 (I doubt that you want it to yield 1). You should change it to 4.0/3.0 to get the desired result.
That said, there is another problem with the aforementioned line: M_PI is not standard C++. It is not standard C either. It is a standard POSIX addition to <math.h>. You should rewrite your own constants or use Boost ones for examples if you want your code to be portable.
Speaking of <math.h>... You are using C++, therefore you should use the C++ standard library headers too and not the C ones:

#include <cmath>

And you notably forgot to include the header <ctype.h> for std::tolower.
That said, it is good practice in C++ to fully qualify the names of the functions from the standard library: you should write std::tolower (and thus actually include <cctype> instead  of <ctype.h>). It's not really longer and it may help to prevent name clashes (I doubt that you will have problems with tolower, but it can be worse with some more common names).


Implementing simple and fast inheritance in JavaScript I have been using the John Resig JavaScript class implementation in my web apps, but the tests shows it is really slow. I really find it useful for the way of extending objects, and the benefits got from having a better code and less redundancy.

In some post was explained that it was slow because of the how the _super method is handled. Since super is Java style, and most of time I develop in PHP, I made my own version of Resig implementation using the parent:: style (used in PHP), with the aim to make this faster:

(function () {
  this.Class = function () {
  };
  Class.extend = function extend(prop) {
    var prototype = new this();
    prototype.parent = this.prototype;

    for (var name in prop) {
      prototype[name] = prop[name];
    }

    function Class() {
      this.construct.apply(this, arguments);
    }
    Class.prototype = prototype;
    Class.prototype.constructor = Class;
    Class.extend = extend;

    return Class;
  };
}) ();

Case of use:

var Person = Class.extend({
  construct: function (name) {
    this.name = name;

  },
  say: function () {
    console.log('I am person: '+this.name);
  },

});

var Student = Person.extend({
  construct: function (name, mark) {
    this.parent.construct.call(this, name);
    this.mark = 5;
  },
  say: function () {
    this.parent.say.call(this);

    console.log('And a student');
  },
  getMark: function(){
   console.log(this.mark);
  }
});

var me = new Student('Alban');
me.say();
me.getMark();
console.log(me instanceof Person);
console.log(me instanceof Student);

Any opinion about this? Is this  fast? What about correctness? Some test shows that is the fastest for the moment.

My next improvement will be to remove the scope .call(this):

this.parent.say.call(this);

to

this.parent.say();

For some strange unexplained (but explained) reasons this test is not working any more, raising a too much recursion error (but it worked some days ago!)

I updated the test and fix the issue with this test. This is leading to strange results. On Chrome 39 it reports to be the fast class definition, on Firefox 35 it seems to be slow.

(1) Any opinion about this?

Emulating language features/idioms which are not supported by the language itself (e.g. classes in JavaScript) does not seem to be a very good idea to me.
JavaScript is not an classical OOP-Language (yet), but there are Prototypes:

(function(){

    var Person = Object.create(null);
    Person.name = "";
    Person.say = function Person_say () {

        console.log("Person: " + this.name);

    };

    var Student = Object.create(Person);
    Student.say = function Student_say () {

        Person.say.call(this);
        console.log("Student: " + this.name);

    };

    var PsychicStudent = Object.create(Student);
    PsychicStudent.say = function PsychicStudent_say () {

        Student.say.call(this);
        console.log("PsychicStudent: " + this.name);

    };

    var ParanormalStudent = Object.create(PsychicStudent);
    ParanormalStudent.say = function ParanormalStudent_say () {

        PsychicStudent.say.call(this);
        console.log("ParanormalStudent: " + this.name);

    };

    var p = Object.create(Person);
    p.name = "A";
    p.say();

    var s = Object.create(Student);
    s.name = "B";
    s.say();

    var s1 = Object.create(PsychicStudent);
    s1.name = "C";
    s1.say();

    var s2 = Object.create(ParanormalStudent);
    s2.name = "D";
    s2.say();

}());

For insights i recommend:

http://javascript.crockford.com/prototypal.html
http://www.crockford.com/javascript/inheritance.html especially the conclusion at the end

(2) Is this fast?

Emulation is probably slower than using native methods - i suppose.
However, it depends on the underlying engine. V8 is faster than Gecko.

(3) What about correctness?

Depends on how you understand correctness. JavaScript is powerful enough to emulate classical inheritance to some extend.

(4) FYI

If you want a more Java-like access to the parent-prototype with "super"-sugar, you can do something like this:

(function(){

    Object.extend = function Object_extend (parent) {

       var child = Object.create(parent);

       child.super = function (name,ctx) {

            if (typeof parent[name] === "function") {

                parent[name].call(ctx);

            }

            return parent;

        };

        return child

    };

    var Person = Object.create(null);
    Person.name = "";
    Person.say = function Person_say () {

        console.log("Person: " + this.name);

    };

    var Student = Object.extend(Person);
    Student.say = function Student_say () {

        Student.super("say", this)
        console.log("Student: " + this.name);

    };

    var PsychicStudent = Object.extend(Student);
    PsychicStudent.say = function PsychicStudent_say () {

        PsychicStudent.super("say", this);
        console.log("PsychicStudent: " + this.name);

    };

    var ParanormalStudent = Object.extend(PsychicStudent);
    ParanormalStudent.say = function ParanormalStudent_say () {

        ParanormalStudent.super("say", this);
        console.log("ParanormalStudent: " + this.name);

    };

    var p = Object.create(Person);
    p.name = "A";
    p.say();

    var s = Object.create(Student);
    s.name = "B";
    s.say();

    var s1 = Object.create(PsychicStudent);
    s1.name = "C";
    s1.say();

    var s2 = Object.create(ParanormalStudent);
    s2.name = "D";
    s2.say();

}());

However, bear in mind, that "super" is also a foreign concept to JavaScript.
You cannot type foreign syntax and expect it to behave identically with comparable performance.
Additionally, you may emulate "super" as shown above, but you will have to use with great care, meaning 'ParanormalStudent.super' and 'this.super' may point to the same function, however, the latter will result in exceeding the call stack. This seems to me like a bad practice, since as a programmer you will have a tool which may either work or produce chaos.


Understand recursion I'm trying to improve my understanding of recursive and iterative
processes. I've started with SICP, but have branched off to try and
find a few exercises when I started having difficulty. Currently I've
been looking at a series of posts on the subject by Tom
Moertel.

Tom's code
defines a node object for binary search trees and a search function:

class BSTNode(object):
    """Binary search tree node."""
    def __init__(self, val, left=None, right=None):
        self.val = val
        self.left = left
        self.right = right

    def __repr__(self):
        return '(%s, %r, %r)' % (self.val, self.left, self.right)

def find_val_or_next_smallest(bst, x):
    """ Get the greatest value <= x in a binary search tree. Returns
    None if no such value can be found. """
    if bst is None:
        return None
    elif bst.val == x:
        return x
    elif bst.val > x:
        return find_val_or_next_smallest(bst.left, x)
    else:
        right_best = find_val_or_next_smallest(bst.right, x)
        if right_best is None:
            return bst.val
        return right_best

The exercise asks us to transform the search function into an
iterative process and gives a general
procedure
for doing so: refactor all recursive calls into tail calls using
accumulators and then create a loop structure.

I've refactored the else clause in the search function into:

    else:
        return work(find_val4(bst.right, x), bst)

def work(child_node, parent_node):
    if child_node is None:
        return parent_node.val
    return child_node

But I've been having trouble figuring out how to move the logic done
in the 'work' function into an accumulator. Any advice, hints, or ways
to think about this would be totally appreciated.

Ahhhh. Okay.

That the issue here is that if the process takes a right branch at node Foo, it needs to store a reference to Foo in the event that it fails to find a child node which is less-than or equal. That reference effectively lives in the stack-calls which occur in inside the calls to the work function.

There can be an arbitrary number of these calls, so we need an internal, private stack:

def find_val_iterative(parent_node, x):
    right_branch = []
    def _inner(parent_node, x):
        if parent_node is None:
            if right_branch:
                return right_branch.pop().val
            return None
        elif parent_node.val == x:
            return x
        elif parent_node.val > x:
            return _inner(parent_node.left, x)
        else:
            right_branch.append(parent_node)
            return _inner(parent_node.right, x)
    return _inner(parent_node, x)


Iterate over two collections of equal length What's the best way to iterate over two collections of equal length?

Currently I'm using this:

for(var index = 0; index < collection1.Count(); index++)
{
    var item1 = collection1[index];
    var item2 = collection2[index];
    Console.WriteLine(item1.ToString() + ", "+ item2.ToString());
}

But I'd love to be able to use a foreach instead, e.g.

foreach(item1, item2 in collection1, collection2)
{
    Console.WriteLine(item1.ToString() + ", " + item2.ToString();
}

Now, this would be sort of possible if I zipped my two collections into a dictionary, but is that the best way to go? Would that be a sufficient gain of readability while sacrificing the performance needed to build the dictionary?

Similarly, I'm not sure if a dictionary is idealistically correct here, as there may not necessarily be a link between the two collections (other than their order in the list), merely a guarantee that they both have the same length.

You can use LINQ's Zip method. As usual, it's a bit slower than manually written code, but unless this is in a hot spot it rarely matters. The cost of Console.WriteLine vastly exceeds the cost of LINQ.

foreach(var pair in collection1.Zip(collection2, Tuple.Create))
{
    Console.WriteLine("{0}, {1}", pair.Item1, pair.Item2);
}

You can replace Tuple.Create by ValueTuple.Create if you target .net 4.7 or you referenced the compatibility nuget package. This saves a heap allocation and is usually faster.

In C# 7 you could use named tuples, which enables meaningful property names. It's based on ValueTuple and thus avoid the heap allocation as well.

names.Zip(ages, (name, age) => (Name:name, Age:age))


Temperature converter app I'm learning Ruby and as an exercise I decided to build this little temperature converter app (C to F, F to C, F to K, etc). Looking back at it, it's pretty much just a bunch of boilerplate code. Any ideas on how I could shorten this up and make it a little more dynamic would really be appreciated!

#!/usr/bin/ruby
# My project: temperature converter (temp_conv.rb)
# start date: 10-26-2016
# finish date:
# A simple program to convert either from C to F, or F to C depending on the user's choice.

def c_to_f(c)
    ans = c*1.8+32
    puts "#{c}C = #{ans}F"
end

def f_to_c(f)
    ans = (f-32)/1.8
    puts "#{f}F = #{ans}C"
end

def f_to_k(f)
    ans = (f+459.65)*5/9
    puts "#{f}F = #{ans}K"
end

def k_to_f(k)
    ans = k * 9/5 - 459.67
    puts "#{k}K = #{ans}F"
end

def c_to_k(c)
    ans = c + 273.15
    puts "#{c}C = #{ans}K"
end

def k_to_c(k)
    ans = k - 273.15
    puts "#{k}K = #{ans}C"
end

again = nil
while again != "n"
    puts "welcome to Temp Converter 1.0!
          Please select an option from the following menu:
    1) Convert Celsius to Fahrenheit
    2) Convert Fahrenheit to Celsius
    3) Convert Fahrenheit to Kelvin
    4) Convert Kelvin to Fahrenheit
    5) Convert Celsius to Kelvin
    6) Convert Kelvin to Celsius
    0) Quit
    "
    m_choice = nil
    loop do
        print ":> "
        m_choice = gets.chomp.to_i
        break if m_choice >= 0 && m_choice <= 6
        puts "Invalid input. Please enter a choice between 0 and 6."
    end
    case m_choice
    when 1
        puts "Celsius to Fahrenheit"
        print "enter Temperature:> "
        cel = gets.chomp.to_i
        c_to_f(cel)
    when 2
        puts "Fahrenheit to Celsius"
        print "Enter Temperature:> "
        fah = gets.chomp.to_i
        f_to_c(fah)
    when 3
        puts "Fahrenheit to Kelvin"
        print "Enter Temperature:> "
        fah = gets.chomp.to_i
        f_to_k(fah)
    when 4
        puts "Kelvin to Fahrenheit"
        print "Enter Temperature:> "
        kel = gets.chomp.to_i
        k_to_f(kel)
    when 5
        puts "Celsius to Kelvin"
        print "Enter Temperature:> "
        cel = gets.chomp.to_i
        c_to_k(cel)
    when 6
        puts "Kelvin to Celsius"
        print "Enter Temperature:> "
        kel = gets.chomp.to_i
        k_to_c(kel)
    else
        puts "Thank you for using, Temperature Converter!"
        sleep(1)
        exit(0)
    end

    print "would you like to run again? (Y/N)"
    loop do
        print ":> "
        again = gets.chomp.downcase
        break if again == 'y' || again == 'n'
        puts "Invalid Input. Please choose either 'y' or 'n'"
    end
end

The good

Short, focused methods. Methods should be short and have a single responsibility. With your conversion related methods, you've achieved this - nice job. Ruby especially encourages small methods; it's common to see them be as short as a single line.
It works. Your program appears to correctly convert between temperatures.
It's readable. You followed reasonably good formatting and style guidelines. Note that conventional Ruby style uses 2 spaces rather then tabs for indentation - I recommend adopting this.

Suggestions

One way you might shorten things up and dynamicize is as follows:
rather than having 6 methods handling each conversion, you could cut it in half by writing functions such as this:

def convert_to_fahrenheit(start_unit, temp)
  if start_unit == 'C'
    return temp * 1.8 + 32
  elsif start_unit == 'K'
    return temp * 9 / 5 - 459.67
  else
    raise ArgumentError, "Unit: #{start_unit} is invalid. Unit should only be K or C"
  end
end

With this solution, the tradeoff is that our methods get bigger but they're still reasonably short and each still has a single responsibility.

Of course with this solution you have to change the existing 1-6 numbery way of gathering input. Instead you might have methods like:

def print_starting_unit_request
  puts "Enter the temperature unit you have (C, F, K):"
end

def print_resulting_unit_request
  puts "Enter the temperature unit you want (C, F, K):"
end

def get_temperature_unit_input
  gets.chomp.upcase
end

def valid_input_temperature?(input)
  ['K', 'C', 'F'].include? input
end

def units_match?(starting_unit, resulting_unit)
  starting_unit == resulting_unit
end

This would eliminate the long series of repetitive case statements. You might have something like:

def convert_temperature(start_unit, end_unit, temp)
  if end_unit == 'C'
    convert_to_celcius(start_unit, temp)
  elsif end_unit == 'K'
    convert_to_kelvin(start_unit, temp)
  elsif end_unit == 'F'
    convert_to_fahrenheit(start_unit, temp)
  else
    raise ArgumentError, "Unit: #{end_unit} is invalid. Valid (K, C, F)."
  end
end

Do note that this is not an ideal solution. It is preferable to use an object-oriented class based solution rather than conditionals; I get the sense that you might not have learned OOP yet. This would be a good exercise in OOP by refactoring once you get more comfortable.

These are rough ideas to get you going and of course you'd have to set up the logic appropriately (looping, etc). A key idea here is that you want to be using methods often. Methods are self documenting and make code more readable.

One thing I noticed when looking at:

**1) Convert Celsius to Fahrenheit**
2) Convert Fahrenheit to Celsius
3) Convert Fahrenheit to Kelvin
4) Convert Kelvin to Fahrenheit
**5) Convert Celsius to Kelvin**
6) Convert Kelvin to Celsius
0) Quit

Every other combination of starting temperature seems sensibly grouped together but 1 & 5 unexpectedly are split up. I would have expected 5 to come right after 1. This might not seem like a big deal and it really isn't here but I think it illustrates a couple of important ideas of writing clean code to be mindful of: consistency and don't be arbitrary.

Consistency is often mentioned related to various aspects of writing good code; it's especially important when many people are touching the same software. The essential idea is that when things are consistent, they are more easily read and understood; when they're not, things tend to stick out, distract us, and interrupt the flow of whatever we're trying to do.

Don't be arbitrary is an idea I recently read from Clean Code, a popular book about writing... clean code:

  Have a reason for the way you structure your code, and make sure that reason is communi- cated by the structure of the code. If a structure appears arbitrary, others will feel empowered to change it. (Martin, 303)

This may seem overkill for a post like this, but hey, I just finished reading the book and want to help reinforce the concepts. But I think it's a good thing to be aware of no matter what level the programmer. In this situation, we structure our input capturing into intuitive grouping that the user might expect - therefore, it's not arbitrary.

Style note:
Prefer spaces between operations (and be consistent). You have both:

ans = c*1.8+32

ans = k * 9/5 - 459.67

Prefer the later.


General Knowledge quiz I have made a General Knowledge quiz in python as a project from my school

import time

score=int()
print("Welcome to the General knowledge Quesions Quiz")
time.sleep(2)
print("Question 1")
time.sleep(2)
Question1 = input ("A vehicle with the national registration code ‘PK’ would originate from which country? ")
if Question1 == ("Pakistan") or Question1 == ("pakistan"):
    print("Correct answer")
    score = score + 1
else:
    print("Incorrect answer, the correct answer is Pakistan")
time.sleep(2)
print("Question 2")
time.sleep(2)
Question2 = input("In which English county is Blenheim Palace? ")
if Question2 == ("Oxfordshire") or Question2 == ("oxfordshire"):
    print("Correct answer")
    score = score + 1
else:
    print("Incorrect answer, the correct answer is Oxfordshire")
time.sleep(2)
print("Question 3")
time.sleep(2)
Question3 = input("How many hours are there in a leap year? ")
if Question3 == ("8784"):
    print("Correct answer")
    score = score + 1
else:
    print("Incorrect answer, the correct answer is 8784")
time.sleep(2)
print("Question 4")
time.sleep(2)
Question4 = input("In which sport could you win the Davis Cup? ")
if Question4 == ("Tennis") or Question4 == ("tennis"):
    print("Correct answer")
    score = score + 1
else:
    print("Incorrect answer, the correct answer is Tennis")
time.sleep(2)
print("Question 5")
time.sleep(2)
Question5 = input("What is the capital of Afghanistan? ")
if Question5 == ("Kabul") or Question5 == ("kabul"):
    print("Correct answer")
    score = score + 1
else:
    print("Incorrect answer, the correct answer is Kabul")
time.sleep(2)
print("Question 6")
time.sleep(2)
Question6 = input("How many golden stars feature on the flag of the European Union? ")
if Question6 == ("twelve") or Question6 == ("Twelve") or Question6 == ("12"):
    print("Correct answer")
    score = score + 1
else:
    print("Incorrect answer, the correct answer is twelve")
time.sleep(2)
print("Question 7")
time.sleep(2)
Question7 = input("In which year did Channel 4 begin transmission in the UK? ")
if Question7 == ("1982"):
    print("Correct answer")
    score = score + 1
else:
    print("Incorrect answer, the correct answer is 1982")
time.sleep(2)
print("Question 8")
time.sleep(2)
Question8=input("In which year did Theresa May become Prime Minister? ")
if Question8 == ("2016"):
    print("Correct answer")
    score = score + 1
else:
    print("Incorrect answer, the correct answer is 2016")
time.sleep(2)
print("Question 9")
time.sleep(2)
Question9 = input("What is six-fourteenths of 112? ")
if Question9 == ("48") or Question9  == ("forty eight"):
    print("Correct answer")
    score = score + 1
else:
    print("Incorrect asnwer, the correct answer is 48")
time.sleep(2)
print("Question 10")
time.sleep(2)
Question10 = input("Which type of angle is greater than 90 degrees but less than 180 degrees? ")
if Question10 == ("obtuse") or ("Obtuse"):
    print("Correct answer")
    score = score + 1
time.sleep(2)
print("This is the end of the quiz")
time.sleep(1)
print("You got",(score),"/ 10")
num1 = (score)
num2 = 10
print('{0:.2f}%'.format((num1 / num2 * 100)))

I have finished the quiz and if anyone has any suggestions on how I could have improved it, they will be greatly appreciated. So please suggest how I can improve it!

Thanks :)

You should generally avoid redundancy when programming.

I'd make a list and loop through it

QUESTIONS = [("A vehicle with the national registration code ‘PK’ would originate from which country?", "Pakistan"),
             ("In which English county is Blenheim Palace?", "Oxfordshire")] # Add more to this list

for n,(question, answer) in enumerate(QUESTIONS, 1):
    time.sleep(2)
    print("Question", n)
    time.sleep(2)
    user_answer = input(question)
    if user_answer.lower() == answer.lower():
        print("Correct answer")
        score += 1
    else:
        print("Incorrect answer, the correct answer is", answer)

And so on

Hope this helps :-)


C# Winforms: Is there any way to improve this simple custom Textbox code? I have been working on Custom TextBox control, which only allows alphanumeric characters. Note that I have not restricted characters from KeyPress, KeyUp event as I don't want to restrict copy/paste or any other operation that should be allowed generally. I have only trimmed non-alphanumeric character on paste operation. However, I am not sure whether the code I wrote is good or bad as I have very little experience of desktop application.

using System;
using System.Windows.Forms;
using System.Text.RegularExpressions;

namespace SaintThomas.UserControls
{
    class TextBoxForUserName : TextBox
    {
    protected override void OnTextChanged(EventArgs e)
    {
        base.OnTextChanged(e);

        this.SuspendLayout();
        int startPos = this.SelectionStart;
        if (Regex.IsMatch(this.Text, "[^0-9_A-Z]", RegexOptions.IgnoreCase))
        {
        int reduceStartPos = this.Text.Length;
        this.Text = Regex.Replace(this.Text, "[^0-9_A-Z]", "", RegexOptions.IgnoreCase);
        startPos = (startPos <= 0) ? 0 : startPos - (reduceStartPos - this.Text.Length);
        if (this.Text.Length < startPos)
        {
            startPos = this.Text.Length;
        }
        this.SelectionStart = startPos;
        }
        this.ResumeLayout();
    }
    }
}


Check whether string ends with string I wrote the following to check whether a string ends with a second string.

function isEndOf(origin, target) {
  return (origin.substr(target.length * -1, target.length) === target);
}

Usage:

isEndOf("A long time ago in a galaxy far, far away", "far away")

This takes a substring with the size of the target string counting from the end of the string and checks whether it's equal to the target string. Quite straightforward. However, I'm concerned about my usage of target.length * -1. This smells like using the wrong method altogether.

Is there a more idiomatic approach for writing such a basic function? Would slicing be more appropriate? The naming can probably be improved.

There's no need for the 2nd argument in substr. So you could just do:

origin.substr(target.length * -1) === target

or

origin.slice(target.length * -1) === target

Same deal.

Edit: Whoops. Actually it's not quite the same deal. As Sumurai8 commented on Simon's answer, MDN says that JScript does not handle negative offsets for substr. However, slice has no such caveat. So long story short: Use slice, not substr.

Instead of * -1, you could also use a unary minus:

origin.slice(-target.length) === target

One thing you might want, though, is to check target before you try accessing its length property. If target == undefined you'll get an error. Something simple like this should do:

if(!target) return false;

One gotcha though: It'll return false if target is an empty string, since empty strings are false'y. However, it doesn't really make much sense to check if string ends with an empty string, so perhaps that's fine? Otherwise, do a typeof check (like the one below) on target.

And you might want to check origin too. However, here you just want to check that it's a string - not if it's just false'y. You'll be calling slice on it, so it must be a string or you'll get an error, whereas the worst that can happen if target isn't a string (but is truth'y) is that target.length is undefined.

So to check origin do:

if(typeof origin !== 'string') return false;

By the way, without the target-is-false'y check, if target is a string but also empty, you'll get some tricky results:

Origin        Target         Result
==========================================
"foo"         ""             false
""            "foo"          false
""            ""             true

Results #2 makes sense, but results #1 and #3 are perhaps a little strange, because, again, what does it mean to have an string end with an empty string? The results are due to slice(-0) being the same as slice(0), i.e. returning the whole string. So in effect you're checking:

"foo".slice(0) == "" // => "foo" == "" (false)
"".slice(0) == ""    // => "" == "" (true)

Kinda confusing.

If you want an empty target string to always return true, you could do:

if(typeof origin !== 'string') return false;
if(typeof target !== 'string') return false;
if(!target) return true;

  I'm aware my code has a lot of white-space for JavaScript standards. I like to think it keeps code readable. The naming can probably be improved.

Whitespace seems just fine to me! Don't know what JS you're used to, but I see absolutely no reason to change anything about whitespace usage.

I would drop the outer parentheses though, like Simon suggested.

As for naming, I'd call it endsWith, or stringEndsWith.

You could also extend the String prototype with an endsWith method, but generally it's better to leave native prototypes alone. However, this is fairly harmless, so I'd be tempted to say:

if(!String.prototype.endsWith) {
  String.prototype.endsWith = function (ending) {
    if(typeof ending !== 'string') return false;
    if(!ending) return true;
    return this.slice(-ending.length) === ending;
  };
}

Here we can skip checking origin, because we're adding the method to String itself. So if it's called, it's called on a string.


OOP and non-OOP versions of layout and page animation code I am trying to learn how to write more object oriented javascript code, but I am having a hard time understanding the correct way to do it, and how it is useful for front end development. As you can see below, I tried to rewrite my old javascript code to be more object oriented, but it just doesn't seem necessary for what I am doing. I am not even sure if I am doing it correctly (would it be better to use a self executing anonymous function for this?). Could someone take a look at my code and tell me how I can improve it? Also, explain to me why it is important to write object oriented javascript for the development of a websites frontend?

Original Code

// Document and Windows Variables
var $document = $(document),
    $window = $(window);

// Equalize
function equal_heights() {
    var $row = $('.equal');

    if ($row.length > 0) {
        $row.each(function() {
            var $sections = $(this).children(),
                largest_height = 0;

            $sections.height('auto');

            $sections.each(function() {
                var h = $(this).height();
                if (largest_height < h) largest_height = h;
            });

            $sections.height(largest_height);
        });
    }
}

// Position Popups
function position_popup() {
    var $popup = $('.popup'),
        pop_height = $popup.outerHeight(true),
        pop_width = $popup.outerWidth(true),
        browser_height = $(window).height(),
        pop_top = 10,
        page_pos = document.documentElement.scrollTop || document.body.scrollTop;

    if (pop_height < browser_height) {
        pop_top = browser_height / 2 - pop_height / 2;
    }

    $popup.css({
        top: page_pos + pop_top
    });
}

// Page Animation Fade (On Scroll)
function page_animations() {
    $('.fade-me').each(function(i) {
        if ($window.scrollTop() > $(this).offset().top - ($window.height() / 1)) {

            $(this).addClass('is-showing');

        }
    });
}

page_animations();

/********************
Run Functions
********************/
$window.on({
  load: function() {
   equal_heights();
   position_popup();
  }
});

OOP

// Document and Windows Variables
var $reuseVars = {
    window: $(window),
    document: $(document)
}

//  Functions Object
function AllFunctions() {}

AllFunctions.prototype = {
    // Equal Heights
    equalHeights: function() {
        var $row = $('.equal');

        if ($row.length > 0) {
            $row.each(function() {
                var $sections = $(this).children(),
                    largest_height = 0;

                $sections.height('auto');

                $sections.each(function() {
                    var h = $(this).height();
                    if (largest_height < h) largest_height = h;
                });

                $sections.height(largest_height);
            });
        }
    },

    // Popup Position
    positionPopup: function() {
        var $popup = $('.popup'),
            pop_height = $popup.outerHeight(true),
            pop_width = $popup.outerWidth(true),
            browser_height = $(window).height(),
            pop_top = 10,
            page_pos = document.documentElement.scrollTop || document.body.scrollTop;

        if (pop_height < browser_height) {
            pop_top = browser_height / 2 - pop_height / 2;
        }

        $popup.css({
            top: page_pos + pop_top
        });
    },

    // Page Fade Animation - On Scroll
    pageAnimation: function() {
        $('.fade-me').each(function(i) {
            if ($reuseVars.window.scrollTop() > $(this).offset().top - ($reuseVars.window.height() / 1)) {

                $(this).addClass('is-showing');

            }
        });
    }
};

// Create All Functions Object
var allFunctions = new AllFunctions();

// Run Functions on doc ready
allFunctions.pageAnimation();

/********************
 Run Functions
********************/
$reuseVars.window.on({
   load: function() {
     allFunctions.equalHeights();
     allFunctions.positionPopup();
   }
});

I think you are over-engineering here. OOP is not an end in itself, so don't convert your code to "OOP style" just because.

The most desirable property of code (after correctness) is clarity. Clarity means obviousness, which means ease of maintenance, both of which mean reduced chance of bugs. To me, your second code sample is less clear than your first.

I would go into a different direction and just try to simplify your first code sample instead of slapping some fancy "OOP" stuff on.

Part of simplification is abstracting functionality into re-usable blocks. jQuery is very easy to extend with your own functions, so let's create two extensions maxHeight and positionCenter:

$.fn.extend({
    maxHeight: function () {
        var heights = this.map(function() { return $(this).height(); });
        return Math.max.apply(null, heights);
    },
    positionCenter: function () {
        return this.each(function () {
            var $this = $(this),
                height = $this.outerHeight(true),
                width = $this.outerWidth(true),
                browser_height = $(window).height(),
                top = 10,
                page_pos = document.documentElement.scrollTop || document.body.scrollTop;

            if (height < browser_height) {
                top = browser_height / 2 - height / 2;
            }
            $this.css({
              top: page_pos + top
              // "left" calculation is still missing
            });
        });
    }
});

This could end up in a script file where you collect all your custom extensions.

With some of the plumbing out of the way, the code that actually works on the page becomes a whole lot clearer:

$(window).scroll(function () {
    var $window = $(window);
    $('.fade-me').each(function() {
        var $this = $(this);
        if ($window.scrollTop() > $this.offset().top - $window.height() / 1) {
            $this.addClass('is-showing');
            // when is this class removed?
        }
    });
});

$(function () {
    $('.equal').each(function() {
        var $sections = $(this).children().height('auto');
        $sections.height( $sections.maxHeight() );
    });
    $('.popup').positionCenter();
});

(The above is of course untested, you might have to tweak it a little here and there.)

The next thing you could investigate is making your own custom jQuery selectors.

Imagine your code scroll handler would look like this:

$(window).scroll(function () {
    $('.fade-me:inview').addClass('is-showing');
});

Wouldn't that be even clearer?

Here is how to do that: http://www.sitepoint.com/make-your-own-custom-jquery-selector/

As a finger exercise, write a jQuery extension named equalizeHeight, such that you can make the following simplification:

$('.equal').each(function() {
    $(this).children().equalizeHeight();
});


Conditional Concatenation of a Pandas DataFrame I am concatenating columns of a Python Pandas Dataframe and want to improve the speed of my code.

My data has the following structure:

    Apple    Pear    Cherry
    1        2       3
    4        5       NaN
    7        8       9

I only want to concatenate the contents of the Cherry column if there is actually value in the respective row. If my code works correctly, the result of the example above should be:

    Result
    1 :: 2 :: 3
    4 :: 5
    7 :: 8 :: 9

My code so far is this:

a_dataframe[result] = a_dataframe.apply(lambda r:
          str(r.loc['apple']) + ' :: ' + str(r.loc['pear'])+' :: '+str(r.loc['cherry'])
          if pd.notnull(r.loc['cherry']) & (r.loc['cherry'] != "")
          # if cherry value is empty, do not add cherry into result
          else str(r.loc['apple']) + ' :: ' + str(r.loc['pear']),
          axis=1)

Any thoughts on how I can improve the speed of my code? Can I run this without an apply statement using only Pandas column operations?

Thanks up front for the help.

There's no need to create a lambda for this.

Let's suppose we have the following dataframe:

my_df = pd.DataFrame({
    'Apple':  ['1', '4', '7'],
    'Pear':   ['2', '5', '8'],
    'Cherry': ['3', np.nan, '9']})

Which is:

Apple Cherry Pear
   1      3    2
   4    NaN    5
   7      9    8

An easier way to achieve what you want without the apply() function is:

use iterrows() to parse each row one by one.
use Series() and str.cat() to do the merge.

You'll get this:

l = []
for _, row in my_df.iterrows():
    l.append(pd.Series(row).str.cat(sep='::'))

empty_df = pd.DataFrame(l, columns=['Result'])

Doing this, NaN will automatically be taken out, and will lead us to the desired result:

Result
1::3::2
   4::5
7::9::8

The entire program may look like:

import pandas as pd
import numpy as np

def merge_columns(my_df):
    l = []
    for _, row in my_df.iterrows():
        l.append(pd.Series(row).str.cat(sep='::'))
    empty_df = pd.DataFrame(l, columns=['Result'])

    return empty_df.to_string(index=False)

if __name__ == '__main__':
    my_df = pd.DataFrame({
        'Apple': ['1', '4', '7'],
        'Pear': ['2', '5', '8'],
        'Cherry': ['3', np.nan, '9']})
    print(merge_columns(my_df))

There are other things that I added to my answer as:

if __name__ == '__main__'
added the logic into its own function so that you can reuse it later

As  @MathiasEttinger suggested, you can also modify the above function to use list comprehension to get a slightly better performance:

def merge_columns_1(my_df):
    l = [pd.Series(row).str.cat(sep='::') for _, row in my_df.iterrows()]

    return pd.DataFrame(l, columns=['Result']).to_string(index=False)

I'll let the order of the columns as an exercise for OP.


CSV file reader in PHP that supports large files (>15k lines) I have written the following function in PHP to read a CSV file. It works correctly for small files.

However, if I try to read in files that are bigger than 15k lines, it takes between 1–2 minutes to process them. How can I optimize this code to make it run faster on large files?

Is there anything else that I should improve?

function read_csv($file){
        $return_waarde = array();
        if(!is_null($file) && !is_empty($file)){
            $header = str_getcsv(utf8_encode(array_shift($file)), ';');
            $header_trimmed = array();

            foreach($header as $value){
                $trim = trim($value);
                if(!in_array($value, $header_trimmed)){
                    $header_trimmed[] = $trim;
                } else {
                    $header_trimmed[] = $trim . "1";
                }
            }
            ini_set('memory_limit', '512M');
            ini_set('max_execution_time', '180');

            foreach($file as $record)
            {
                if(!in_array($record,$return_waarde)){
                    $return_waarde[] = array_combine($header_trimmed, str_getcsv(utf8_encode($record), ';'));
                }
            }
        } else {
            $return_waarde = "there is no file";
        }
        return $return_waarde;
    }

Performance

As performance is your main concern, let's face this first. To complete the example CSV-file with ~36k lines your original script needs around 139s*.

The main bottlenecks are in_array:

if (!in_array($record,$return_waarde)) {}

and array_combine:

$return_waarde[] = array_combine($header_trimmed, str_getcsv(utf8_encode($record), ';'));

As you want an associative array, we can't get rid of array_combine but we can improve the very expensive and slow test from in_array.

Idea

Instead of checking the fastly growing and complex result-array for existence of the newly created associative array, you can do this:

create a second array
create a hash of the current dataset/row
check this array's keys for the existence of the latest hash using isset, which is faster than in_array
only if the hash is not found, store it, run array_combine on the row and append the result as well

Result

while (false !== ($data = fgetcsv($handle, 1000, ','))) {
    $hash = md5(serialize($data));

    if (!isset($hashes[$hash])) {
        $hashes[$hash] = true;
        $values[] = array_combine($headerUnique, $data);
    }
}

With this improvement the script processes all 36k lines in ~0.5s now*. Seems a little faster. ;)

Unique entries in the result

Even though this is solved by using the hash now, let me point out a flaw in your logic:

if (!in_array($record, $return_waarde)){
    $return_waarde[] = array_combine($header_trimmed, str_getcsv());
}

This will never find any duplicates, because you check for existence of the indexed array $record but afterwards you insert a different associative array.

Unique header names

In the beginning you create unique names for duplicate entries in the header row:

if(!in_array($value, $header_trimmed)){
      $header_trimmed[] = $trim;
  } else {
      $header_trimmed[] = $trim . "1";
  }

If you have a column name more than two times, you'll end up with this, probably unintended, result:

['column', 'column1', 'column1']

You can create a function to make the names truly unique, e.g.:

function unique_columns(array $columns):array {
    $values = [];

    foreach ($columns as $value) {
        $count = 0;
        $value = $original = trim($value);

        while (in_array($value, $values)) {
            $value = $original . '-' . ++$count;
        }

        $values[] = $value;
    }

    return $values;
}

This will result in

['column', 'column-1', 'column-2']

Return value of read_cvs

Currently your function read_csv() does return either a string or an array. The function should always return an array. You can even make the parameter- and return-value-types more strict:

function read_csv(string $file): array {}

Also try to exit early, when something went wrong instead of nesting if-statements. If you actually want to do something, if an error occurs, throw an exception:

if (!$file) {
    throw new Exception('File not found: ' . file);
}

Final result

Finally let's make this function more versatile by adding the line length and delimiter as optional parameters.

function read_csv(string $file, int $length = 1000, string $delimiter = ','): array {
    $handle = fopen($file, 'r');
    $hashes = [];
    $values = [];
    $header = null;
    $headerUnique = null;

    if (!$handle) {
        return $values;
    }

    $header = fgetcsv($handle, $length, $delimiter);

    if (!$header) {
        return $values;
    }

    $headerUnique = unique_columns($header);

    while (false !== ($data = fgetcsv($handle, $length, delimiter))) {
        $hash = md5(serialize($data));

        if (!isset($hashes[$hash])) {
            $hashes[$hash] = true;
            $values[] = array_combine($headerUnique, $data);
        }
    }

    fclose($handle);

    return $values;
}

* For testing I used an example CSV-file with over 36.000 lines from the site SpatialKey. I duplicated a few column names and added at least one duplicate line. My environment is the latest MAMP running PHP 7.1.1. The time was measured using: $start = microtime(true); $x = read_csv('test.csv'); print microtime(true) - $start;.


Screen manager and its screens having access over the screen manager I've got this here screen manager system and while it works rather well, I do worry on how practical it all is.

The application creates a screen manager class then enters a message loop which calls the update, render and handle input methods of the screen manager.

The screen manager has a vector container of unique_ptr's.
The main container type is a virtual screen class. It looks like this.

I should mention that vgSreen has a forward declaration of screenManager.

std::vector<std::unique_ptr<vgScreen>> gameScreens;
this->AddScreen(std::unique_ptr<MainMenu>(new MainMenu()));

When the screen manager adds a screen it loads the screens content and initializes it, then adds it to the stack. When it initializes a screen it passes a pointer of itself like this. From here on I'm skeptical about the performance of the program.

gameScreenToAdd.Initialize(*this);

The game screen stores the screen manager parameter as a pointer:

bool MainMenu::Initialize(ScreenManager* sManager)
{
    //manager is a ScreenManager* pointer
    this->manager = sManager;
}

Then when a screen wants to add or remove itself or another screen it can do the following:

manager->AddScreen(std::unique_ptr<Gameplay>(new Gameplay()));

I believe the erase function may result in errors because when a unique_ptr is added to the vector, the vector size increases, but erasing the vector doesn't then resize the container.

bool ScreenManager::RemoveScreen(int index)
{
    unsigned int a = GetCount();

    gameScreen[index]->Shutdown();

    gameScreen[index].release();

    gameScreen.erase(gameScreen.begin() + index);

    return true;
}

If there's a better alternate than using a vector of unique pointers, or if I'm leaking horrendously by passing screen manager to each screen, let me know. Or if you have any thoughts on this in general.

I just happened to code my own StateMachine a week ago, which is similar to your ScreenManager, so here are my thoughts on the subject.

A vector of state can work, but I think using a stack instead saves you the index management. Also, the stack would resize itself during both pop and push operations.

A state (or a screen in your case) manages its own lifetime. As a consequence, it can remove itself from the stack during an update operation, and must survive until the next loop is processed. Thus, I choosed std::shared_ptr to work out this problem.

std::stack<std::shared_ptr<Screen>> _screens;

Since you're using C++11, the more you avoid manual dynamic allocations, the better. Using templates, you can use a safer way of creating your screen, and even provide parameters to them :

template<typename Screen, typename... Args>
void push_screen (Args && ... args)
{
    if (!_screens.empty())
        _screens.top()->pause(); // Pause the current state

    _screens.emplace(std::make_shared<Screen>(std::forward<Args>(args)...));
    // Or, if you still use std::unique_ptr
    // _screens.emplace(new Screen { std::forward<Args>(args)... }); // C++11
    // _screens.emplace(std::make_unique<Screen>(std::forward<Args>(args)...)); // C++14
    _screens.top()->enter(); // Start the new state
}

This way, you can pass a reference of your ScreenManager and avoid using a bare pointer.

push_screen<MainMenuScreen>(*this);


Extracting contents of dictionary contained in Pandas dataframe to make new dataframe columns I created a Pandas dataframe from a MongoDB query.

c = db.runs.find().limit(limit)
df = pd.DataFrame(list(c))

Right now one column of the dataframe corresponds to a document nested within the original MongoDB document, now typed as a dictionary. The dictionary is in the run_info column.

I would like to extract some of the dictionary's values to make new columns of the data frame. Is there a general way to do this? If not, what is the best brute force method specific to my data?

Here is my brute force approach, and it's horrendous. Even for a dataframe of only 200 rows, this is taking several minutes:

run_info = df['run_info']
# want to extra values from dictionary stored in column df['run_info']
# some values are within dictionaries nested within top dictionary
# these are 'author' and 'weather'
for i in range(len(df['run'])):
    g = run_info[i]
    if g.get('weather'):
        for name in weather_list:
            if g.get('weather').get(name):
                try:
                    df.set_value(i, name, g['weather'][name])
                except:
                    pass
    if g.get('author'):
        for name in author_list:
            if g.get('author').get(name):
                try:
                    df.set_value(i, name, g['author'][name])
                except:
                    pass
    for name in name_list:
        if g.get(name):
            try:
                if name is 'local_start_time':
                    df.set_value(i, name, g[name][0:19])
                else:
                    df.set_value(i, name, g[name])
            except:
                pass

I'd appreciate any and all suggestions for how to improve the speed here. Also I am using the try...except because once in a while the encoding on some funky character is causing a ValueError.

I suspect the try-except in the loops are the main cause of the slowness. It's not clear at which point you get ValueError due to "funky" data. It would be good to clean that up, get rid of the try-catch, and I think your solution will get noticeably faster.

Another small thing, that might not make a noticeable difference at all, you have some repeated lookups, like in this snippet:

if g.get('weather'):
    for name in weather_list:
        if g.get('weather').get(name):

Here, the g.get('weather') bit is executed twice. It would be better to save the result of the first call, and then reuse that. Apply this logic to other similar places. Although it might not make a practical difference, avoiding duplicated logic is a good practice.


Simple autoloader, but trying to make it psr-compliat I made this class, just to understand what PSR autloader is. I didn't want to download and use it without knowing how it works. So, the one I built is very simple, but I would like a review on how to improve it, and what it lacks to be 100% psr-1 compliant.

Here is the class loader.

namespace ClassLoader;

class Loader
{
    private $classDir;
    private $namespace;
    public $dirSeparatorSymbol = '\\';

    public function __construct($namespace, $classDir)
    {
        $this->classDir = $classDir;
        $this->namespace = $namespace;

    }

    private function load($class)
    {

        $include_path = str_replace($this->dirSeparatorSymbol, DIRECTORY_SEPARATOR, $this->classDir);

        $classFilename = substr($class, strrpos($class, '\\') + 1) . '.php';

        if(file_exists($class = $include_path.$classFilename)){
            require $class;

            return true;
        }

    }

    public function register()
    {
        spl_autoload_register([$this, 'load']);
    }
}

Also: You can see the constructor's first argument: __construct($namespace, ... has a namespace param. But I don't know how this is used. The class loads any functions just by doing:

$a = new ClassLoader\Loader('unused/param', 'path/to/bar');
$a->register();

new bar;

Well, your class is psr-1 compliant. But i think you mean psr-0.

To have a maximum compliance to the psr-0 autoloader. simply copy paste it...

Your method for instance doesn't work for Class_Name

if you call your autoload with \namespace\ClassName the first \ doesn't get trimmed. This will leed to problems. (php will load /path/to/lib//namespace/ClassName.php)

as mentioned in the comments, the use of the $this->dirSeparatorSymbol is weird. You don't need it. so don't use it.

Edit: 'unused' namespace

Often you will have multiple libraries at different places. Your own will probably reside under ROOT.'/lib/' but other libraries you downloaded should/will be under ROOT.'/vendor/AwesomeLibrary/src/'. to accomodate this. You will register 2 autoloaders:

$personalLoader = new ClassLoader\Loader('Personal','/lib/');
$personalLoader->register();

$awesomeLoader = new ClassLoader\Loader('AwesomeLibrary','/vendor/AwesomeLibrary/src/');
$awesomeLoader->register();

Now your autoloader should check or it is allowed to load the file. For instance if I use
AwesomeLibrary/MyClass; the personal loader shouldn't do anything since it knows nothing about AwesomeLibrary. the second awesomeLoader however knows how to load the AwesomeLibrary.


A function that replicates DLookup, and can use parameterized queries This function was built to replace DLookup so that i could pass in parameterized queries for a speed increase. I also wanted a simple way to get more than one value if needed. For example, if i want 4 or 5 fields out of a particular record or maybe 4 or 5 records with only one field, I could get that with this function.

EDIT: the whole filter string portion of the fuction is legacy functionality from when i initially built this function and had to leave in untill all of the places that use this piece of code have been updated.

Public Function Qlookup(ByVal argQuery As String, Optional ByRef argParameters As Variant = Null, Optional ByVal argIsFilterString As Boolean = False) As Variant
    'Parameters -
    'argQuery: the query definition name
    'argParameters: the list of parameters (can come in an array or not) that should be in the order and starting at 0
    'argIsFilterString: Allows you to use a 'Where' Clause instead of a parameter list
    'that the parameters in your query are.
    'Output -
    'Will output a single item, 1d array or 2d array depending on the query you feed it.
    'This comment line is here to fix the formatting messing up.
    If Not BasicInclude.DebugMode Then On Error GoTo Error_Handler
    Dim rs As DAO.Recordset
    Dim qry As QueryDef
    Dim u As Long
    Dim out() As Variant
    Dim i As Long
    Dim j As Long
    Qlookup = Null
    Set qry = dbLocal.QueryDefs(argQuery)
    If argIsFilterString Then
        Set rs = qry.OpenRecordset(dbOpenSnapshot)
        rs.filter = argParameters
        Set rs = rs.OpenRecordset(dbOpenSnapshot)
    Else
        If IsArray(argParameters) Then
            u = UBound(argParameters)
            If u = (qry.Parameters.count - 1) Then
                For i = 0 To u
                    qry.Parameters(i).value = argParameters(i)
                Next i
            Else
                Err.Raise vbObjectError, "Qlookup", "Number of Parameters in query(" & qry.Parameters.count & ") do not match the number of parameters passed in(" & u + 1 & ")"
            End If
        Else
            If Not (IsNull(argParameters)) And qry.Parameters.count = 1 Then
                qry.Parameters(0).value = argParameters
            ElseIf qry.Parameters.count = 0 And Not (IsNull(argParameters)) Then
                Err.Raise vbObjectError + 1, "Qlookup", "Number of Parameters in query(" & qry.Parameters.count & ") do not match the number of parameters passed in(1)"
            ElseIf qry.Parameters.count = 0 And (IsNull(argParameters)) Then
            End If
        End If
        Set rs = qry.OpenRecordset(dbOpenSnapshot)
    End If
    If rs.RecordCount Then
        rs.MoveFirst
        rs.MoveLast
        rs.MoveFirst
        If rs.RecordCount > 1 Then
            If rs.Fields.count > 1 Then
                ReDim out(rs.RecordCount - 1, rs.Fields.count - 1)
                For i = 0 To rs.RecordCount - 1
                    For j = 0 To rs.Fields.count - 1
                        out(i, j) = rs.Fields(j).value
                    Next
                    rs.MoveNext
                Next
            Else
                ReDim out(rs.RecordCount - 1)
                For i = 0 To rs.RecordCount - 1
                    out(i) = rs.Fields(0).value
                    rs.MoveNext
                Next
            End If
            Qlookup = out
        Else
            If rs.Fields.count > 1 Then
                ReDim out(rs.RecordCount - 1, rs.Fields.count - 1)
                For i = 0 To rs.RecordCount - 1
                    For j = 0 To rs.Fields.count - 1
                        out(i, j) = rs.Fields(j).value
                    Next
                    rs.MoveNext
                Next
                Qlookup = out
            Else
                Qlookup = rs.Fields(0).value
            End If
        End If
    End If
Error_Exit:
    Set rs = Nothing
    Set qry = Nothing
    Exit Function
Error_Handler:
    StandardErrorBox "Qlookup", Err, Errors
    Qlookup = Null
    Resume Error_Exit
End Function

I'm going to cover other points that aren't covered...

Unnecessary recordset movement

You have this:

If rs.RecordCount Then
    rs.MoveFirst
    rs.MoveLast
    rs.MoveFirst
    ...
End If

This is totally unnecessary and also problematic:

RecordCount is not the most idiomatic way of checking for an empty. EOF is more common and is usually effective. It also works in both DAO and ADO; I'm a big advocate of writing code that is portable and consistent; it means less thing to remembers and less surprises when one quirky thing that works in one library doesn't in other.
MoveLast in order to get a count. You already know that RecordCount doesn't return accurate count until you've accessed all records in the recordset. But the more important question to ask yourself is: Did you really need that? I assert that 99% of time you don't. You have two options at your disposal:

1) Use GetRows method to extract an array out of recordset.

2) Use either a VBA.Collection or Scripting.Dictionary and stop caring about how many records there are; just use a Do Until rs.EOF loop and insert it into the dynamic collection variable.

The point being, you do not need to know the count of the records to do what you need. Eliminate that from your procedure and it becomes much more simpler and also less error-prone.

Do loop instead of For loop

In similar vein you have this code:

For i = 0 To rs.RecordCount - 1
    out(i) = rs.Fields(0).Value
    rs.MoveNext
Next

You need to stop caring about how many iterations you need to do. This is much less verbose and easier to understand:

Do Until rs.EOF
    'do something
    rs.MoveNext
Loop

No local variables is needed just to iterate; nor do you need to care about count to iterate. You just keep iterating until.... you get to the end!

Unnecessary branching

Others already commented on the need to extract functionality into smaller functions and I fully agree. I want to call your attention to this:

    If rs.RecordCount > 1 Then
        If rs.Fields.count > 1 Then
            ReDim out(rs.RecordCount - 1, rs.Fields.count - 1)
            For i = 0 To rs.RecordCount - 1
                ...
            Next
        Else
            ReDim out(rs.RecordCount - 1)
            For i = 0 To rs.RecordCount - 1
                ...
            Next
        End If
        ...
    Else
        If rs.Fields.count > 1 Then
            ReDim out(rs.RecordCount - 1, rs.Fields.count - 1)
            For i = 0 To rs.RecordCount - 1
                ...
            Next
            ...
        Else
            ...
        End If
    End If

Note how all branch, except for very last one basically do the same thing. In fact, the last one is only special since it's only one column. But do we really care? I say no. All you really want to do is this:

Do Until rs.EOF
    For Each fld In rs.Fields
       ...
    Next
    rs.MoveNext
Loop

This does the same thing as the above branches would have done and does so in much less verbose manner and will work whether there's only one column or several, only one records or several records. Heck, it even works even if there's no records!

So in short, you've been a victim of being obliged to hold the compiler's hand and painstakingly tell it each individual step it must do instead of stepping back and see what you are really trying to do, and convey it to the compiler in most idiomatic manner. It's literally the compiler's job to make sense of the above instructions into machine operations which are mind-numbingly detailed well beyond the point of boredom. In fact, it's also the database engine's job to figure out what you want and how to get it to you in most efficient manner. So you want to step back and be a bit more "declarative"1 in your programming.

Legacy Support

You have this section:

If argIsFilterString Then
    Set rs = qry.OpenRecordset(dbOpenSnapshot)
    rs.filter = argParameters
    Set rs = rs.OpenRecordset(dbOpenSnapshot)
Else
    ...
End If

Which you already indicated is there for backward compatibility. But as others have alluded to, that only add noise and make it hard to understand your function. Note that even if you're the only one writing and reading this code, future you won't know what the hell past you were doing/thinking. So if you don't want your future you to go back and kill the past you, you owe it to your future you to make it easy to ready. One easy way to set up for legacy support would be to do something akin to this:

Public Function Everything(A As Foo, B As Bar) As Baz
  If B Then
    Everything = EverythingLegacy(A, B)
  Else
    Everything = EverythingCurrent(A)
  End If
End If

Private Function EverythingLegacy(A As Foo, B As Bar) As Baz
...
End Function

Private Function EverythingCurrent(A As Foo) As Baz
...
End Function

This gives you two big wins:

1) The code for Everything function is immediately much easier to read and make sense. The B parameter which is only used for legacy becomes more obvious.

2) It's now easy to test the individual functions without having to change the implementation and you know that changes you make won't break the legacy implementation when you tweak your current implementation. When you've finished removing all the legacy implementation, then it's easy to delete the legacy implementation and make EverythingCurrent the new Everything.

--

1 Technically speaking, that's inaccurate because VBA is an imperative language whereas SQL is a declarative language, so I use the term loosely in the sense that you need to focus on describing the problem rather than outlining the steps yourself and getting lost in the forest of details.


Efficiently matching IP addresses with a list of CIDRs I am using Python3 to extract some information from a dictionary that contains 10k of CIDRs, but the running time is pretty long.

Here is my purpose: I have a dictionary of CIDRs (10000) and a list of IPs (5000), then I want to go through each IP in the list and check if it belongs to one CIDR in the dictionary.

Could you please give me some trick to reduce the running time?

#python 3:
import os
import ipaddress

Dict = {127.0.0.0/8 : ABC, 169.128.0.0/16 : DEF}
List = {127.0.0.1, 126.1.1.1, 169.2.2.2, 150.0.22.2}

for IP in List:
  for CIDR in Dict.keys():
    if ipaddress.ip_address(IP) in ipaddress.ip_network(CIDR):
      print(CIDR)
      break

First of all, welcome to Code Review! This is a great first question. There are some things to learn about how to post a question in order to get the best response, so I'll cover these first and then cover the performance problems that you're seeing.

Readability

You will get more reviews if your code is readable. I find the layout of your code very clear and readable apart from two things.

Your indenting uses only 2 spaces instead of the recommended 4.
Your variable names Dict and List are not descriptive and start with an uppercase letter (which is generally reserved for classes)

I understand that these two variables are not part of the code you use in the real world setting and were just quickly added for the purposes of this question. However, it's worth using clear variable names and standard indenting because the reviewer won't have your familiarity with the code.

Note that your List is not a list, since you have used braces { and } instead of square brackets [ and ]. Braces denote a set, or a dict if key value pairs are used. In your case List is a set, which has different behaviour.

Running your code before you post it

You didn't run this code before you posted it, and it contains syntax errors as pointed out by 200_success.
You can't be expected to spot every minor typo in your code before you post it. However, you are expected to run the code before you post it. This would have picked up the problem with Dict and with List. It only takes a moment and means you will get more meaningful reviews, less confusion, and use up less of the reviewers time so they can help more people.

Since the site rules require working code, it would be perfectly reasonable for a reviewer to simply move on to the next question if this one has code that doesn't run. For this reason you are also likely to get more reviews and to see your first review sooner if you run your code before posting it.

For reference, the syntax errors on both lines are due to not enclosing strings in quotes, so that python tries to evaluate them as expressions.

Dict = {127.0.0.0/8 : ABC, 169.128.0.0/16 : DEF}
List = {127.0.0.1, 126.1.1.1, 169.2.2.2, 150.0.22.2}

should be:

Dict = {'127.0.0.0/8' : 'ABC', '169.128.0.0/16' : 'DEF'}
List = {'127.0.0.1', '126.1.1.1', '169.2.2.2', '150.0.22.2'}

Repeating work

You are repeating two things, each of which slows down your program.

You convert IP using ipaddress.ip_address(IP). This conversion always gives the same output for a given IP, so you can calculate it once at the start of the outer loop rather than recalculating the same thing every time you start the inner loop.
You convert CIDR using ipaddress.ip_network(CIDR). Each time through the inner loop you will be converting the same list of CIDRs. If you have sufficient memory you can calculate all of these once, and then use those precalculated values rather than recalculating.

You state that there are 10,000 CIDRs and 5,000 IPs.

The first problem will therefore repeat the IP conversion on average 5,000 times for cases where a match is found (because on average it will get half way through the 10,000 CIDRs before finding the match). Where no match is found it will repeat the IP conversion 10,000 times, once for each CIDR. Between 5,000 and 10,000 conversions for each of 5,000 IPs is between 25,000,000 and 50,000,000 conversions when only 5,000 are needed (once per IP).

The second problem will repeat all 10,000 CIDR conversions for any IP that does not find a match, and on average 5,000 CIDR conversions for any IP that does find a match. That's between 25,000,000 and 50,000,000 conversions when only 10,000 are needed (once per CIDR).

How to return an unconverted CIDR

I can see why you might be tempted to loop over unconverted CIDRs - you return an unconverted CIDR so working with a list of converted CIDRs would mean you'd have to find out which CIDR the converted CIDR came from before returning it.

You can return the unconverted CIDR by creating a dict at the start which has converted CIDR as the key and original CIDR as the value.

For example:

converted_CIDRs = {ipaddress.ip_network(CIDR):CIDR for CIDR in Dict.keys()}

Now you can work with converted CIDRs to avoid all that recalculation, and still return an unconverted CIDR at the end by doing a quick dictionary lookup using the converted CIDR as the key.


Extract location from HTTP socket I have the following HTTP reply saved in a local file called source.txt:

HTTP/1.1 301 Moved
Connection: close
Content-length: 111
Location: https://11.12.13.14:81/
Content-type: text/html; charset="utf-8"

<META HTTP-EQUIV="refresh" CONTENT="0;URL=https://11.12.13.14:81/">

Source code:

#include <stdio.h>
#include <stdlib.h>
#define MAXBUFLEN 1024

char* getLocation(char* source)
{
    const char *p1 = strstr(source, "Location:")+10;
    const char *p2 = strstr(p1, "\n");
    size_t len = p2-p1;
    char *res = (char*)malloc(sizeof(char)*(len+1));
    strncpy(res, p1, len);
    res[len] = '\0';
    return res;
}

char* getData(char* source)
{
    const char *p1 = strstr(source, "://")+3;
    const char *p2 = strstr(p1, "\n");
    size_t len = p2-p1;
    char *res = (char*)malloc(sizeof(char)*(len+1));
    strncpy(res, p1, len);
    res[len] = '\0';
    return res;
}

int main()
{
    char source[MAXBUFLEN];
    char host[100];
    int port;
    FILE *fp = fopen("source.txt", "r");
    if (fp != NULL) {
        size_t newLen = fread(source, sizeof(char), MAXBUFLEN, fp);
        if (newLen == 0) {
            fputs("Error reading file", stderr);
        } else {
            source[++newLen] = '\0';

//extraction code
            char* res = getLocation(source);
            printf("getLocation result: %s\n", res);
            if (strstr(res, "://"))
            {
                res = getData(source);
                printf("getData result: %s\n", res);
                if (strstr(res, ":"))
                {
                    sscanf(res, "%[^:]:%d[^/]", host, &port);
                    printf("host: %s | port: %d\n", host, port);
                }
                else
                    printf("delimiter not found\n");
            }
            else
                printf("no link\n");
//
        }
    }
    fclose(fp);
}

The program is working well, but it's very ugly.  Is there any way to improve the code to avoid doing so many operations?  I mean somehow merging the two functions, getLocation() and getData().

Your getLocation can be improved a little:

For a start, the call parameter should be const (although if you were happy
modifying the string you could avoid the memory allocation).

Moving on:

const char *p1 = strstr(source, "Location:")+10;

This is ok, except that p1 is cryptic and it is better to avoid embedded constants such as 10. Since this is the length of "Location:", we can extract that:

#define LOC "Location:"
const char *loc = strstr(source, LOC) - 1 + sizeof LOC;

Note the -1 for the \0 at the end of LOC. But it is now uglier still.  And of
course it might fail, returning NULL.  So we need to protect the next line:

const char *p2 = strstr(p1, "\n");

in which you are looking for just one character; strchr is more appropriate and eol is a more descriptive name:

if (loc) {
    const char *eol = strchr(loc, '\n');
}

So next you get the length

size_t len = p2-p1;

which needs a cast to avoid a sign-conversion warning:

size_t len = (size_t) (eol - loc);

You allocate memory next:

char *res = (char*)malloc(sizeof(char)*(len+1));

but you should omit the cast and note that sizeof(char) is 1 by definition,
so:

char *res = malloc(len + 1);

and of course allocation could also fail, so you must check before copying the
address into it.  You copy the address thus:

strncpy(res, p1, len);
res[len] = '\0';

but as you already know the length of the string, memcpy would be better, as
strncpy must check each char in the input string for \0

if (res) {
    memcpy(res, loc, len);
    res[len] = '\0';
}

Putting it all together, we get:

static char* getLocation(const char* source)
{
#define LOC "Location:"
    size_t len = 0;
    char *eol = NULL;
    const char *loc = strstr(source, LOC) - 1 + sizeof LOC;
    if (loc) {
        eol = strchr(loc, '\n');
    }
    if (eol && loc) {
        len = (size_t) (eol - loc);
    }
    if (len) {
        char *res = malloc(len + 1);
        if (res) {
            memcpy(res, loc, len);
            res[len] = '\0';
        }
        return res;
    }
    return NULL;
}

Yes, string handling in C is ugly.

My conclusion from this ugliness is that another approach is needed.  Why not
modify the input string by putting a \0 at eol and returning loc?  Then
you could extract the address and port directly from that:

static char* getLocation(char* source)
{
#define LOC "Location:"
    char *loc = strstr(source, LOC) - 1 + sizeof LOC;
    if (loc) {
        char *eol = strchr(loc, '\n');
        if (eol) {
            *eol = '\0';
        }
    }
    return loc;
}