Add color to terminal output I have the following header included in some of my projects so that I can add a little color to the terminal output.  Here is how it would be used:

fprintf(stdout, "Recognized text: %s\n", text ?: RED_TEXT("No text recognized."));

color.h:

/**
 * @file color.h
 * @brief Defines all of the ANSI terminal escape codes that modify the color of text.
 */

#ifndef COLOR_H
#define COLOR_H

#define BLACK_TEXT(x) "\033[30;1m" x "\033[0m"
#define RED_TEXT(x) "\033[31;1m" x "\033[0m"
#define GREEN_TEXT(x) "\033[32;1m" x "\033[0m"
#define YELLOW_TEXT(x) "\033[33;1m" x "\033[0m"
#define BLUE_TEXT(x) "\033[34;1m" x "\033[0m"
#define MAGENTA_TEXT(x) "\033[35;1m" x "\033[0m"
#define CYAN_TEXT(x) "\033[36;1m" x "\033[0m"
#define WHITE_TEXT(x) "\033[37;1m" x "\033[0m"

#define BOLD_BLACK_TEXT(x) "\033[1m\033[30m;1m" x "\033[0m"
#define BOLD_RED_TEXT(x) "\033[1m\033[31m;1m" x "\033[0m"
#define BOLD_GREEN_TEXT(x) "\033[1m\033[32m;1m" x "\033[0m"
#define BOLD_YELLOW_TEXT(x) "\033[1m\033[33m;1m" x "\033[0m"
#define BOLD_BLUE_TEXT(x) "\033[1m\033[34m;1m" x "\033[0m"
#define BOLD_MAGENTA_TEXT(x) "\033[1m\033[35m;1m" x "\033[0m"
#define BOLD_CYAN_TEXT(x) "\033[1m\033[36m;1m" x "\033[0m"
#define BOLD_WHITE_TEXT(x) "\033[1m\033[37m;1m" x "\033[0m"

#endif // COLOR_H

As @LokiAstari says, hard-coding these escape strings into your program is a bad idea, as the escape sequences vary according to the type of terminal.  There are libraries such as ncurses and terminfo that do just that.

These macros only work when their arguments are string literals, which may be surprising to callers.

There is also a lot of repetition, much of it caused by lack of orthogonality.

If you wanted to "keep it simple" and do this anyway, you would be better off with

#define COLOR_RESET  "\033[0m"
#define BOLD         "\033[1m"
#define BLACK_TEXT   "\033[30;1m"
#define RED_TEXT     "\033[31;1m"
#define GREEN_TEXT   "\033[32;1m"
#define YELLOW_TEXT  "\033[33;1m"
#define BLUE_TEXT    "\033[34;1m"
#define MAGENTA_TEXT "\033[35;1m"
#define CYAN_TEXT    "\033[36;1m"
#define WHITE_TEXT   "\033[37;1m"

… to be used like

fprintf(stdout, "Recognized text: %s\n", text ? text : (RED_TEXT "No text recognized." COLOR_RESET));

For colour reset use "\033[0;m" instead of "\033[0,"


Ruby bubble sort algorithm I'm just looking for constructive criticism of my Ruby implementation of a bubble sort algorithm.

class BubbleSorter < Object
  def bubble_sort(list)
    swaps = 0
    # parse the list until it is sorted
    until @sorted == true
      # run the comparison of adjacent elements
      for i in 0...(list.length - 1)
        # if the first is greater than the second, swap them with parallel assignment
        if list[i] > list[i+1]
          list[i], list[i+1] = list[i+1], list[i]
          # increase the number of swaps performed in this run by 1
          swaps += 1
        end
        # compare the next 2 elements
        i += 1
      end

      # uncomment the following line to see each iteration:
      # p list

    # If any swaps took place during the last run, the list is not yet sorted
    if swaps > 0
        @sorted = false
      # no swaps? Everything is in order
      else
        @sorted = true
      end
      # reset swap count for each run
      swaps = 0
    end
  end
end

Although you should comment your work, some of your comments seem unecessary. Let's look at some lines to see why:

# parse the list until it is sorted
until @sorted == true

The comment seems redundant. Your line of code until @sorted == true reads clear enough for me to understand that you want to keep doing something until the list is sorted.

# if the first is greater than the second, swap them with parallel assignment
if list[i] > list[i+1]
  list[i], list[i+1] = list[i+1], list[i]

This code is also pretty self explanatory. If you did have a comment you should be explaining why you do this. However, bubblesort is pretty well known so you probably don't even need a comment on this.

# increase the number of swaps performed in this run by 1
swaps += 1

Again why? I can read the code, it is obvious you're incrementing the number of swaps. I would omit.

# compare the next 2 elements
i += 1

Based of the if statement I would know that this the purpose of i. I would omit it, but it does explain why you increment i, so there is some grey zone here.

You write some good comments toward the end, let's analyze why they are good:

# If any swaps took place during the last run, the list is not yet sorted
if swaps > 0
    @sorted = false
# no swaps? Everything is in order
else
  @sorted = true

It explains why we need to check if swaps > 0 and how the sorting algorithm depends on this.


Minimum perfect squares needed to sum up to a target I'm trying to solve this problem

  Given a positive integer n, find the least number of perfect square
  numbers (for example, 1, 4, 9, 16, ...) which sum to n.

I've come up with a solution that works correctly but times out on large inputs:

from collections import defaultdict
class Solution:
    def numSquares(self, n: int) -> int:
        coins = []
        for i in range(1, n+1):
            if i**2>n:
                break
            coins.append(i**2)
        min_coins_to_make = defaultdict(lambda :float("inf"))
        min_coins_to_make[0] = 0
        for coin in coins:
            if coin > n:
                break
            for target in range(coin, n+1):
                min_coins_to_make[target] = min(min_coins_to_make[target], 1+min_coins_to_make[target-coin])
        if min_coins_to_make[target] == float("inf"):
            return 0
        return min_coins_to_make[target]

How do I optimize it in terms of time and space complexity?

https://leetcode.com/problems/perfect-squares/

I won't specifically address the execution time concerns to begin with; there are other issues:

Don't use a class

There's no reason for this to be a class. You have one method and you don't even reference self. In theory you could remove self and mark it a @staticmethod, but really it should just be a function without a class.

Reuse variables

Make this variable -

i2 = i**2

since it's used twice. The same goes for min_coins_to_make[target].

Loop limit

    for coin in coins:
        if coin > n:
            break

That termination condition will be true if coin > n, but coin == i**2. i**2 > n will never be true, because in the previous loop,

        if i**2>n:
            break

So can't you just write for coin in coins without an interior termination condition?


Image processing and plotting I wrote a program for analyzing pictures. The problem I'm having is very slow processing time for medium images like 800x800. I think the root of the problem for this is the for loop where I complete my NumPy array with values. The main task for the program is to count how many times x intensity shows in each color channel and then plot them in a histogram. For example, in the end we will be able to see how many times do we get intensity 200 in red channel and so on.

My code is probably very hard to read but I tried to add comments to make things easier.

from PIL import Image
import numpy as np
import matplotlib.pyplot as plt

#Load an image
fname = 'picture1.jpg'
image_grey = Image.open(fname).convert("L")
image_rgb = Image.open(fname).convert("RGB")

arrey_grey = np.asarray(image_grey)
arrey_rgb = np.asarray(image_rgb)

#Get image size
with Image.open('picture1.jpg') as img:
    width, height = img.size

size = width * height

#Plot the uploaded image both grey and rgb
plt.imshow(arrey_grey, cmap='gray')
plt.show()

plt.imshow(arrey_rgb)
plt.show()

#Define numpy arrey for each color channel
matrix_red = np.zeros((256, 4),dtype = object)
matrix_red[:,1] = int(0)
matrix_red[:,2:] = float(0)

matrix_green = np.zeros((256, 4),dtype = object)
matrix_green[:,1] = int(0)
matrix_green[:,2:] = float(0)

matrix_blue = np.zeros((256, 4),dtype = object)
matrix_blue[:,1] = int(0)
matrix_blue[:,2:] = float(0)

# Completing first column with 0-255
for i in range(256):
    matrix_red[i][0] = i
    matrix_green[i][0] = i
    matrix_blue[i][0] = i

# Counting intensity for each color channel
for i in range(width):
    Matrix_Width = arrey_rgb[i]
    for i in range(height):
        Matrix_Height = Matrix_Width[i]
        Red_Value = Matrix_Height[0]
        Green_Value = Matrix_Height[1]
        Blue_Value = Matrix_Height[2]
        for i in range(256):
            if (matrix_red[i][0] == Red_Value):
                matrix_red[i][1] = matrix_red[i][1] + 1
            if (matrix_green[i][0] == Green_Value):
                matrix_green[i][1] = matrix_green[i][1] + 1
            if (matrix_blue[i][0] == Blue_Value):
                matrix_blue[i][1] = matrix_blue[i][1] + 1

# Data for task ahead
Hx = 0
for i in range(256):
    matrix_red[i][2] = matrix_red[i][1] / size
    Hx = (matrix_red[i][2] + matrix_red[i][3]) + Hx
    matrix_red[i][3] = Hx

#Plotting results
Frequencie_Red = np.zeros((256, 1),dtype = object)
Frequencie_Red[:,0] = int(0)

Frequencie_Green = np.zeros((256, 1),dtype = object)
Frequencie_Green[:,0] = int(0)

Frequencie_Blue = np.zeros((256, 1),dtype = object)
Frequencie_Blue[:,0] = int(0)

Intensity = np.zeros((256, 1),dtype = object)
Intensity[:,0] = int(0)

for i in range(256):
    Frequencie_Red[i] = matrix_red[i][1]
    Frequencie_Green[i] = matrix_green[i][1]
    Frequencie_Blue[i] = matrix_blue[i][1]

for i in range(256):
    Intensity[i] = i

pos = Intensity
width = 1.0

ax = plt.axes()
ax.set_xticks(pos + (width / 2))
ax.set_xticklabels(Intensity)

plt.bar(pos, Frequencie_Red, width, color='r')
plt.show()

plt.bar(pos, Frequencie_Green, width, color='g')
plt.show()

plt.bar(pos, Frequencie_Blue, width, color='b')
plt.show()

As i got pointed out in comment section I added test image and capture with my results.

This is the image I'm using to do my calculations.

The result looks like this. Where Red histogram is red channel and so on:

*Small note, I have no idea why under the first histogram there are black line.

Usually, when using for loops and numpy together, you’re probably doing it wrong.

Some things you could replace in your code:

Use np.arange(256) instead of np.zeros + for i in range(256);
Use slicing instead of manually extracting values out of an array;
Use comparisons and .sum() instead of manually counting the number of pixels of a certain color intensity ((array_rgb == 42).sum(axis=1).sum(axis=0))

But all that is nothing compared to the use of numpy.histogram which does exactly what you want.

You also open the same image twice without closing it and yet, you use a with statement with your third open. Always use a with statement to better manage your resources.

I would use several plt.figure() and a single plt.show() to get all 5 plots show at once and be able to compare them. It will also make the whole thing more responsive.

Lastly, I would put all the code in a function that can be parametrized with the image name:

from PIL import Image
import numpy as np
import matplotlib.pyplot as plt

def histogram(image_name):
    with Image.open(image_name) as image_file:
        array_grey = np.asarray(image_file.convert('L'))
        array_rgb = np.asarray(image_file.convert('RGB'))

    plt.figure()
    plt.imshow(array_grey, cmap='gray')
    plt.figure()
    plt.imshow(array_rgb)

    for i, color in enumerate('rgb'):
        hist, bins = np.histogram(array_rgb[..., i], bins=256, range=(0, 256))
        plt.figure()
        plt.bar(bins[:-1], hist, color=color)
    plt.show()

if __name__ == '__main__':
    histogram('picture1.jpg')

(Using the ... syntax from this answer)

Or you could keep the 3 different histograms in variables for further processing:

from PIL import Image
import numpy as np
import matplotlib.pyplot as plt

def build_histograms(image_array):
    for i in range(image_array.shape[-1]):
        hist, _ = np.histogram(image_array[..., i], bins=256, range=(0, 256))
        yield hist

def histogram(image_name):
    with Image.open(image_name) as image_file:
        array_grey = np.asarray(image_file.convert('L'))
        array_rgb = np.asarray(image_file.convert('RGB'))

    plt.figure()
    plt.imshow(array_grey, cmap='gray')
    plt.figure()
    plt.imshow(array_rgb)

    red_frequencies, green_frequencies, blue_frequencies = build_histograms(array_rgb)
    grey_frequencies, = build_histograms(array_grey[..., np.newaxis])

    x_axis = np.arange(256)
    plt.figure()
    plt.bar(x_axis, red_frequencies, 1.0, color='r')
    plt.figure()
    plt.bar(x_axis, green_frequencies, 1.0, color='g')
    plt.figure()
    plt.bar(x_axis, blue_frequencies, 1.0, color='b')
    plt.figure()
    plt.bar(x_axis, grey_frequencies, 1.0, color='k')

    plt.show()

if __name__ == '__main__':
    histogram('picture1.jpg')


Generic multidimensional array in C++ #include <iostream>
#include <memory>
using std::cout;
using std::endl;

template<typename T, int dimension = 1>
class Array
{
private:
    std::unique_ptr<T[]> pointer;
    int size[dimension];
    int realSize;

public:
    Array()
    {
    }

    template<typename... Ns>
    Array(Ns... ns)
    : realSize(1)
    {
        create(1, ns...);
    }

private:
    template<typename... Ns>
    void create(int d, int n, Ns... ns)
    {
        realSize *= n;
        size[d - 1] = n;
        create(d + 1, ns...);
    }

    void create(int d)
    {
        pointer = std::unique_ptr<T[]>(new T[realSize]);
    }

    int computeSubSize(int d) const
    {
        if (d == dimension)
        {
            return 1;
        }
        return size[d] * computeSubSize(d + 1);
    }

    template<typename... Ns>
    int getIndex(int d, int n, Ns... ns) const
    {
        return n * computeSubSize(d) + getIndex(d + 1, ns...);
    }

    int getIndex(int d) const
    {
        return 0;
    }

public:
    template<typename... Ns>
    T& operator()(Ns... ns) const
    {
        return pointer[getIndex(1, ns...)];
    }

    int getSize(int d = 1) const
    {
        return size[d - 1];
    }
};

int main()
{
    constexpr int N = 10;
    Array<int> a(N);
    for (int i = 0; i < a.getSize(); ++i)
    {
        a(i) = i + 1;
        cout << a(i) << ' ';
    }
    cout << '\n' << endl;
    Array<int, 2> a2(N, N * 2);
    for (int i = 0; i < a2.getSize(1); ++i)
    {
        for (int j = 0; j < a2.getSize(2); ++j)
        {
            a2(i, j) = i + j + 2;
            if (a2(i, j) < 10)
            {
                cout << '0';
            }
            cout << a2(i, j) << ' ';
        }
        cout << endl;
    }
    return 0;
}

Output

1 2 3 4 5 6 7 8 9 10

02 03 04 05 06 07 08 09 10 11 12 13 14 15 16 17 18 19 20 21
03 04 05 06 07 08 09 10 11 12 13 14 15 16 17 18 19 20 21 22
04 05 06 07 08 09 10 11 12 13 14 15 16 17 18 19 20 21 22 23
05 06 07 08 09 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24
06 07 08 09 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25
07 08 09 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26
08 09 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27
09 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28
10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29
11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30

No need for an empty constructor in C++11:

Array()
{
}

You can just use a default constructor:

Array() = default;

It's a little confusing to have multiple public/private sections.  Here, you can just put all the public code under the same keyword.
You could make your structure more useful by providing iterators.  This will, for instance, allow you to use range based for-loops instead of plain ones for iterating through this structure.
You don't need std::endl if you just need newlines.  Just output "\n" instead for this.  More info about this can be found here.
It's unnecessary to have your own return 0 at the end of main() in C++.  The compiler will provide this return for you.


LINQ to SQL query filter, with the name match ignoring multiple punctuation characters I have the following LINQ to SQL query which works fine but looks ugly:

var filter = "filter";

query = query.Where(x =>
    x.Name.Replace("'", "").Replace("\"", "").Replace("#", "").Replace("/", "").Replace("-", "").Contains(filter) ||
    x.FullName.Replace("'", "").Replace("\"", "").Replace("#", "").Replace("/", "").Replace("-", "").Contains(filter));

It'd be nice to be able to do something similar to this (which isn't possible because LINQ to Entities won't recognize the method):

var filter = "filter";
var removals = new string[] { "'", "\"", "#", "/", "-" };

query = query.Where(x =>
  Replaces(x.Name, removals).Contains(filter) ||
  Replaces(x.Full, removals).Contains(filter));

... but I can't figure out how that could be written. I've written predicates that dealt with entire expressions, but not with just a single property.

This is a LINQ to SQL expression, so I can't just pull it out into its own method or I'll get an error like:

  Additional information: LINQ to Entities does not recognize the method
  'System.String RemoveAll

You are rather removing those parts instead of replacing them, so more appropriate name would be remove all.

You can make your own extension method like this :

public static class Extensions
{
    public static string RemoveAll(this string source, string[] charsToRemove)
    {
        return charsToRemove.Aggregate(source, (current, t) => current.Replace(t, string.Empty));
    }
}

If you really want to replace them with something you can do it like this :

public static string ReplaceAll(this string source, string[] charsToRemove, string[] charsToReplace)
{
    string result = source;
    for (var i = 0; i < charsToRemove.Length; i++)
    {
        result = result.Replace(charsToRemove[i], charsToReplace[i]);
    }
    return result;
}

Example usage :

var filter = "filter";
string[] itemsToRemove = {"'", @"""",};

query = query.Where(x =>
    x.Name.RemoveAll(itemsToRemove).Contains(filter) ||
    x.FullName.RemoveAll(itemsToRemove).Contains(filter));

UPDATE

LINQ to SQL would require you to call .AsEnumerable(), .ToList() or .ToArray() first before operating on strings, you might loose some performance from that but the other way is to write your custom query provider or stick with what you have.


Refactor C# code for creating XML to not use intermediate dictionary If have the following code that via an SQL query retrieves data from a database in the form:

Id      subschemaName  locusName
MLST    MLST           LMO0558
MLST    MLST           LMO0563
MLVST   MLVST          LMO1305
MLVST   MLVST          LMO1089

And I want to turn this into an XML like:

<ResponseSubschemas>
    <organism>LMO</organism>
    <Subschemas>
        <Subschema>
            <id>MLST</id>
            <name>MLST</name>
            <loci>
                <locus>LMO0558</locus>
                <locus>LMO0563</locus>
            </loci>
        </Subschema>
        <Subschema>
            <id>MLVST</id>
            <name>MLVST</name>
            <loci>
                <locus>LMO1305</locus>
                <locus>LMO1089</locus>
            </loci>
        </Subschema>
    </Subschemas>
</ResponseSubschemas>

I do this by using a dictionary but it is the first time I'm using C# and I'm sure there must be a more efficient way of doing this in one go?

string subschemaSQL =   "SELECT subschema.Id, subschema.name as subschemaName, locus.Name as locusName " +
                        "FROM subschema " +
                        "LEFT JOIN subschemamembers ON subschemamembers.SubSchemaID = subschema.PrimKey " +
                        "LEFT JOIN locus ON subschemamembers.LocusID = locus.ID " +
                        "WHERE  subschema.OrganismID = ? " +
                        "ORDER BY subschema.Id, locusName";

XElement rootNode = new XElement("ResponseSubschemas",
    new XElement("organism", organismID),
    new XElement("Subschemas"));
XElement subschemasNode = rootNode.Element("Subschemas");

DbDataReader subschemaReader = conn.Query(subschemaSQL, organismID);

Dictionary<string, List<string>> dictionary = new Dictionary<string, List<string>>();
while (subschemaReader.Read())
{
    string subschemaDbID = (string)subschemaReader["Id"];
    XElement subschemaNode = new XElement("Subschema",
        new XElement("id", subschemaDbID),
        new XElement("name", subschemaReader["subschemaName"])
    );
    subschemasNode.Add(subschemaNode);
    string locusName = (string)subschemaReader["locusName"];
    if (dictionary.ContainsKey(subschemaDbID))
    {
        dictionary[subschemaDbID].Add(locusName);
    }
    else
    {
        dictionary.Add(subschemaDbID, new List<string> { locusName });
    }
}

IEnumerable<XElement> subschemaNodes = from element in
                                       subschemasNode.Elements("Subschema")
                                       select element;
foreach (XElement subschemaNode in subschemaNodes)
{
    string subschemaID = subschemaNode.Element("id").Value;
    XElement lociNode = new XElement("loci");
    foreach (string locusName in dictionary[subschemaID])
    {
        XElement locusNode = new XElement("locus", locusName);
        lociNode.Add(locusNode);
    }
    subschemaNode.Add(lociNode);

}

responseXml = rootNode.ToString();

Since your data is already sorted you definitely don't need to use a dictionary. You can just accumulate loci and then whenever the id or subschemaId rolls over you write them all out. That wouldn't be a huge change from what you've got. But honestly, what you've got is a bit confusing because you're trying to do several things at once. Namely:

Read from the database and parse the data
Determine when you've read an entire set of data
Write it all to XML

It would be easier to figure out what's going on by separating those things out from each other. It might also make your life easier if you ever want to do anything more complex with this data than just dumping it to XML. So let's just do a little rewrite.

To start with, let's create a class to hold each of those Subschema elements. (Honestly Subschema seems like a confusing name to me, it seems like maybe OrganismLocations or OrganismSightings might be more descriptive, but let's just stick with Subschema.)

public class Subschema{
    public string Id {get;set;}
    public string Name {get;set;}
    public List<string> Loci {get;set;}

    public Subschema(){
        Loci = new List<string>();
    }
}

Now, let's write a function to parse the database records out into a list of Subschema objects. This is basically doing what I said earlier - tracking loci until we hit a new id or subschemaName. We're using C# iterators (that's the yield return syntax) to make this a little bit cleaner.

private IEnumerable<Subschema> ParseSubschemas(DbDataReader subschemaReader){
    if(!subschemaReader.HasRows){
        yield break;
    }

    Subschema subschema = null;

    while(subschemaReader.Read()){
        //If we've rolled over to a new ID or SubschemaName, start a new set of sightings
        if(subschema != null && (subschema.Id != (string)subschemaReader["id"] || subschema.Name != (string)subschemaReader["subschemaName"])){
            yield return subschema;
            subschema = null;
        }

        if(subschema == null){
            subschema = new Subschema(){
                Id = (string)subschemaReader["id"],
                Name = (string)subschemaReader["subschemaName"]
            };
        }

        subschema.Loci.Add((string)subschemaReader["locusName"]);
    }

    //yield the last subschema
    yield return subschema;
}

So now we have a nice, clean list of the data you need to output. Basically we've handled the first and second bullets from my list, so all that's left is turning that into XML. There's probably a cleaner way of doing this than using XElements directly, but let's just stick with it for the moment. But again, let's stick all the logic in a nice clean function.

private XElement SerializeSubSchemasToXml(string organismId, IEnumerable<Subschema> sightings){
    XElement rootNode = new XElement("ResponseSubschemas",
        new XElement("organism", organismId),
        new XElement("Subschemas"));
    XElement subschemasNode = rootNode.Element("Subschemas");

    //Turn each Sighting into an XElement, and append to the subschemas
    foreach(var sighting in sightings){
        var subschemaElem = new XElement("Subschema",
                                new XElement("id", sighting.Id),
                                new XElement("name", sighting.Name),
                                new XElement("Loci", sighting.Loci.Select(l => new XElement("locus", l)))
                            );

        subschemasNode.Add(subschemaElem);
    }

    return rootNode;
}

Pretty straightforward here. We could've written this more succinctly, probably even in a (really big) one liner, but this strikes a reasonable balance with clarity, I think.

So, now we just have to put those two together...

private string ReadSubschemasAsXml(){
    string organismId = "FOO";
    string subschemaSQL =   "SELECT subschema.Id, subschema.name as subschemaName, locus.Name as locusName " +
        "FROM subschema " +
        "LEFT JOIN subschemamembers ON subschemamembers.SubSchemaID = subschema.PrimKey " +
        "LEFT JOIN locus ON subschemamembers.LocusID = locus.ID " +
        "WHERE  subschema.OrganismID = ? " +
        "ORDER BY subschema.Id, locusName";

    using(DbDataReader subschemaReader = conn.Query(subschemaSQL, organismId)){
        IEnumerable<Subschema> sightings = ParseSubschemas(subschemaReader);
        XElement xelem = SerializeSubSchemasToXml(organismId, sightings);
        return xelem.ToString();
    }
}

That last function isn't really a thing of beauty either - it's got kind of a lot going on. But it's definitely better. I also added a using statement around your DbDataReader, which is just a handy way of making sure that it gets Dispose()d properly.

By the way, I put up a DotNetFiddle of the whole thing. It doesn't work because I don't have a database to use, but it might be easier to read there.


Decoupling MVC model methods in java I have a basic swing application that I am trying to develop using MVC structure.

The principal class in the model connects to a derby database and retrieves values from columns and stores them in arrays.

My methods in the model (Connect(), Retrieve()) are public void. In runApp(), which is a SwingUtilities runnable, I am calling my model methods like this:

public static void runApp() {
    Model model = new Model();
    model.Connect(); //Connects to database
    model.Retrieve(); //Retrieves database column values from tables, creates arrays
    View view = new View(model);
    Controller controller = new Controller(view, model);
    view.setCalculateListener(controller);
}

I am trying to write this program so that everything is as less tightly coupled as possible.  This code currently works as is, but I would like to do this better.

My question: How can I do this in a less coupled manner? Rather than having the methods being called here, is there a better way to do this?

For starters, consider making Connect() and Retrieve() non-public. runApp() doesn't seem to have a good reason to know about the operations that Model needs to perform to be usable here. Can the constructor (or, possibly more appropriately, a static factory method) hide those methods?

Introducing state to objects is an easy way to make things complicated. If a View or Controller were somehow able to be passed a Model that had been, say, Connect()ed but not Retrieve()d, what would happen? It's simpler to factor that possibility out when that is an option.


Finding consecutive days in an array of dates The problem:

How to get the largest number of consecutive days from an array of unknown number of Date objects, by taking into account days only like 4th of April, 5th of April and so on.

Example input data:

let calendar = Calendar.current
let x1 = Date()
let x2 = calendar.date(byAdding: .day, value: 1, to: x1)!
let x3 = calendar.date(byAdding: .day, value: 1, to: x2)!
let x4 = calendar.date(byAdding: .day, value: 1, to: x3)!
let x5 = calendar.date(byAdding: .day, value: 1, to: x4)!
let x6 = calendar.date(byAdding: .day, value: 1, to: x5)!

let x7 = calendar.date(byAdding: .hour, value: 1, to: x4)!
let x8 = calendar.date(byAdding: .hour, value: 2, to: x4)!
let x9 = calendar.date(byAdding: .hour, value: 3, to: x2)!
let x10 = calendar.date(byAdding: .hour, value: 4, to: x2)!

var dates = [Date]()
dates.append(x1)
dates.append(x5)
dates.append(x2)
dates.append(x3)
dates.append(x4)
dates.append(x6)

dates.append(x7)
dates.append(x8)
dates.append(x9)
dates.append(x10)

Expected result from the sample data: 6 consecutive days

How this solution could be improved?

Solution:

dates.sort(by: < )

// extract only unique days
var dateIndex = 0
var nextDateIndex = 1

repeat {
    let date = dates[dateIndex]
    let nextDate = dates[nextDateIndex]
    if calendar.isDate(nextDate, inSameDayAs: date) {
        dates.remove(at: nextDateIndex)
    } else {
        dateIndex += 1
        nextDateIndex += 1
    }
} while (dateIndex < dates.count - 1)

print(dates)

let aDayInSeconds: TimeInterval = 86_400
let twoDaysInSeconds: TimeInterval = 172_800

var consecutiveDaysCount = 1
dateIndex = 0
nextDateIndex = 1

// count consecutive days
repeat {
    guard (i + 1 < dates.count) else { break }
    let date = dates[dateIndex]
    let nextDate = dates[nextDateIndex]

    if nextDate.timeIntervalSince(date) >= aDayInSeconds && nextDate.timeIntervalSince(date) < twoDaysInSeconds {
        dateIndex += 1
        nextDateIndex += 1
        consecutiveDaysCount += 1
    } else {
        break
    }
} while (i < newDates.count - 1)

print(consecutiveDaysCount)

Never use 86,400 seconds as the duration of a day. "Most" days have 24 hours,
but in regions with daylight saving time, a day can have 23 or 25 hours (when the
clock is adjusted one hour forward or back).

Another problem is that you compare the difference in seconds between two days
in order to decide if they are on the same day. Then for example
"April 3, 10:00" and "April 4, 09:00" will be considered the same day.

There is also a bug in your final loop: The loop is exited if two dates are more
than one day apart, instead of continuing to search for a longer sequence of
consecutive days.

I would start by computing the difference in days to some reference date first:

dates.sort() // Short form for: dates.sort(by: < )
let referenceDate = calendar.startOfDay(for: dates.first!)

let dayDiffs = dates.map { (date) -> Int in
    calendar.dateComponents([.day], from: referenceDate, to: date).day!
}

Now we have an array of (non-decreasing) integers, in your example
that would be

print(dayDiffs)
// [0, 1, 1, 1, 2, 3, 3, 3, 4, 5]

It remains to find the longest
sequence of consecutive numbers (ignoring duplicates) in that array, i.e. no date/calendar calculations
are needed from this point on.

This should be done in a separate function, which could look like this:

/// Find maximal length of a subsequence of consecutive numbers in the array.
/// It is assumed that the array is sorted in non-decreasing order.
/// Consecutive equal elements are ignored.

func maximalConsecutiveNumbers(in array: [Int]) -> Int {
    var longest = 0 // length of longest subsequence of consecutive numbers
    var current = 1 // length of current subsequence of consecutive numbers

    for (prev, next) in zip(array, array.dropFirst()) {
        if next > prev + 1 {
            // Numbers are not consecutive, start a new subsequence.
            current = 1
        } else if next == prev + 1 {
            // Numbers are consecutive, increase current length
            current += 1
        }
        if current > longest {
            longest = current
        }
    }
    return longest
}

The approach is actually similar as in your last loop, only that

we only have to compare integers,
we continue to search for longer subsequences if one consecutive sequence ends,
consecutive equal numbers are simply ignored, so that it is no longer necessary
to remove duplicates in advance,
zip() + dropFirst() is used to iterate over all pairs of adjacent array elements, that saves a look of bookkeeping (dateIndex,
nextDateIndex).

You should add unit tests to verify that the function works correctly.
This is easier for an array of integers than for an array of dates.

With these preparations, the number of consecutive days is simply obtained as

let consecutiveDaysCount = maximalConsecutiveNumbers(in: dayDiffs)


Vigenere cipher C program I implemented a Vigenere cipher that preserves case and can also decrypt if you pass the -d argument. Usage is ./vigenere <plaintext|ciphertext> key [-d] and it works fine but I think there are probably some things I could do better. So, just looking for critique.
#include <stdio.h>
#include <stdlib.h>
#include <ctype.h>
#include <string.h>

#define ENCRYPT 1
#define DECRYPT -1

int mod(int x, int n)
{
    return (x % n + n) % n;
}

char *vigenere(char *input, char *keystream, int mode)
{
    if (strlen(keystream) != strlen(input))
        return NULL;

    int output_len = strlen(input), offset = 'a', wasupper = 0;
    char *output = malloc(output_len);

    for (int i = 0; i < output_len; ++i)
    {
        if (isupper(input[i]))
        {
            input[i] = tolower(input[i]);
            wasupper = 1;
        }
        else
            wasupper = 0;

        output[i] = mod((input[i] - offset) + (keystream[i] - offset) * mode, 26) + offset;

        if (wasupper)
            output[i] = toupper(output[i]);
    }

    return output;
}

char *gen_keystream(char *key, int keystream_len)
{
    int key_len = strlen(key), j = 0;
    char *keystream = malloc(keystream_len);

    for (int i = 0; i < keystream_len; j = ++i % key_len)
        keystream[i] = key[j];

    return keystream;
}

int main(int argc, char **argv)
{
    if (argc < 3)
        return EXIT_FAILURE;

    int mode = ENCRYPT;

    if (argc > 3 && strcmp(argv[3], "-d") == 0)
        mode = DECRYPT;

    int keystream_len = strlen(argv[1]);
    char *keystream = gen_keystream(argv[2], keystream_len);
    char *result = vigenere(argv[1], keystream, mode);

    printf("%s\n", result);

    free(keystream);
    free(result);

    return EXIT_SUCCESS;
}

Bug: no null character
printf("%s\n", result); fails as result does not point to a string.  That data lacks a null character and is short, by 1 of allocated memory needed.
// char *output = malloc(output_len);
char *output = malloc(output_len +1);
output[output_len] = '\0';

Similar problem with decryptor.
No protection against non-A-Z characters
I'd expect only encrypting/decrypting when the input[] is an A-Z.
I suspect code may generate output[i] == 0 for select inputs, rendering the string short.

Pedantic bug: Negative char
When input[i] < 0, isupper(input[i]) is UB.  Better to access the string via an unsigned char * for is...().
A negative char value also messes-up mod().  When x < 0, x % n code does not perform a Euclidean mod.


What is the best way to make this code dry? I have a script that makes a bunch of api calls (xml) and it's not adhering to DRY because I am repeating a lot of the code in the functions, what would be the best way to clean this up and reduce the amount of repeat code?
def get_api_key(hostname, username, password):
    url = 'https://' + hostname + '/api'
    values = {'type': 'keygen', 'user': username, 'password': password}
    response = requests.post(url, params=values, verify=False, proxies=proxies)
    return ET.fromstring(response.text).find('.//result/key').text

def add_template(hostname, api_key, template_name, name):
    url = 'https://' + hostname + '/api'
    values = {'type': 'config',
        'action': 'set',
        'xpath': f"entry[@name='{template_name}']/",
        'element': f"<tag>{name}</tag>",
        'key': api_key
        }
    requests.post(url, params=values, verify=False, proxies=proxies)
    return(template_name)

def add_template_stack(hostname, api_key, stack_name, template_name):
    url = 'https://' + hostname + '/api'
    values = {'type': 'config',
        'action': 'set',
        'xpath': f"entry[@name='{stack_name}']",
        'element': f"<tag>{template_name}</tag>",
        'key': api_key
        }
    requests.post(url, params=values, verify=False, proxies=proxies)
    return(stack_name)

def add_variable(template_name, var_name, netmask, hostname, api_key):
    url = 'https://' + hostname + '/api'
    values = {'type': 'config',
        'action': 'set',
        'xpath': f"entry[@name='{template_name}']/variable/entry[@name='{var_name}']",
        'element': f"<tag>{netmask}</tag>",
        'key': api_key
        }
    requests.post(url, params=values, verify=False, proxies=proxies)
    return(var_name)

I think a good way to reduce the WET code here is to consolidate and generalize the core functionality to a single function, and tweak the arguments going into it accordingly. I left the get_api_key function alone and focused on the other ones as they are the most similar. See below:
def add_template_2(hostname, api_key, template_name, name):
    return add(hostname, api_key, template_name, name)

def add_template_stack_2(hostname, api_key, stack_name, template_name):
    return add(hostname, api_key, stack_name, template_name)

def add_variable_2(template_name, var_name, netmask, hostname, api_key):
    return add(hostname, api_key, template_name, netmask, var_name)

def add(hostname, api_key, xpath_name, element, xpath_var=None):
    url = f'https://{hostname}/api'

    if xpath_var:
        xpath = f"entry[@name='{xpath_name}']/variable/entry[@name='{xpath_var}']"
        ret_val = xpath_var
    else:
        xpath = f"entry[@name='{xpath_name}']"
        ret_val = xpath_name

    values = {'type': 'config',
        'action': 'set',
        'xpath': xpath,
        'element': f"<tag>{element}</tag>",
        'key': api_key
        }
    requests.post(url, params=values, verify=False, proxies=proxies)
    return(ret_val)


Swift NSCache Singleton I want to wrap NSCache in a Singleton in order to use dependency injection in my code. This has resulted, rather unfortunately, in passing the type through a function parameter, and I even need two mocks for success and failure since it is a singleton instance.
I think there might be a better way, but any comments appreciated!
ImageCache and protocol
protocol ImageCacheProtocol {
    static var shared: Self { get }
    func getCache() -> NSCache<AnyObject, UIImage>
}

final class ImageCache: ImageCacheProtocol {
    var cache: NSCache<AnyObject, UIImage> = NSCache<AnyObject, UIImage>()

    public static let shared = ImageCache()
    private init() {}

    func getCache() -> NSCache<AnyObject, UIImage> {
        return cache
    }
}

MockImageCache & MockImageCacheFailure
final class MockImageCache: ImageCacheProtocol {
    var cache: NSCache<AnyObject, UIImage> = MockNSCache(shouldReturnImage: true)

    public static let shared = MockImageCache()
    private init() {}

    func getCache() -> NSCache<AnyObject, UIImage> {
        return cache
    }
}

final class MockImageCacheFailure: ImageCacheProtocol {
    var cache: NSCache<AnyObject, UIImage> = MockNSCache(shouldReturnImage: false)

    public static let shared = MockImageCacheFailure()
    private init() {}

    func getCache() -> NSCache<AnyObject, UIImage> {
        return cache
    }
}

Instantiating my view model (snippet)
class ViewModel<U: ImageCacheProtocol> {

    private var cache: U.Type
    init<T: NetworkManagerProtocol>(networkManager: T, data: DisplayContent, imageCache: U.Type) {
        self.networkManager = AnyNetworkManager(manager: networkManager)

        cache = imageCache

Yes, this isn't the canonical way. My solution is to not use Self and to instead focus on shared.
That is:
protocol ImageCacheProtocol {
    func getCache() -> NSCache<AnyObject, UIImage>
}

final class ImageCache: ImageCacheProtocol {
    var cache: NSCache<AnyObject, UIImage> = NSCache<AnyObject, UIImage>()
    static var shared = ImageCache()

    private init() {}

    func getCache() -> NSCache<AnyObject, UIImage> {
        return cache
    }
}

Then the mocks can be altered:
final class MockImageCache: ImageCacheProtocol {
    var cache: NSCache<AnyObject, UIImage> = MockNSCache(shouldReturnImage: true)
    public static var shared: ImageCacheProtocol! = MockImageCache()
    func getCache() -> NSCache<AnyObject, UIImage> {
        return cache
    }
}


Exception handling and general check in Java Are the catch statements ok or must I put in a println()? In general, is the code good-looking enough?

    public void music(String song) {
        try {
    File soundFile = new File(song);
    AudioInputStream sound = AudioSystem.getAudioInputStream(soundFile);

    // load the sound into memory (a Clip)
    DataLine.Info info = new DataLine.Info(Clip.class, sound.getFormat());
    Clip clip = (Clip) AudioSystem.getLine(info);
    clip.open(sound);

    // play the sound clip
    clip.start();
    }
    catch (IOException e) {
    }
    catch (LineUnavailableException e) {
    }
    catch (UnsupportedAudioFileException e) {
    }
}

Must I put in System.out.println(); so I can know which one it is?

That would be one approach, and it would certainly be better than your current approach, which is silently failing. In case something goes wrong, this will make it very difficult to find out the problem.

If you get an exception, you should first try to think of recovering from it right then and there.

If that is not possible, I would just trow it upwards, and let the calling method handle it. In your case for example by playing a default song, or skipping to the next song, etc.

If all else fails, you should report a helpful and detailed error message to the end user. For debugging purposes, you should also print the exact exception - including stacktrace - to the console.

Just swallowing the exception is pretty much never the correct approach.

Misc

// play the sound clip isn't really a helpful comment, it's pretty obvious from the code already
If you handle all exceptions the same way anyways (in your case ignoring them), you can just catch the generic Exception.
as barq said, your indentation is partly off.
music is a little generic as a method name. maybe playSong, or playClip


Removing whitespaces in a string I wrote this function to remove whitespaces in strings. Please help me improve it. I intended to use the function for a Big Integer ADT.

#include <iostream>
#include <string>
#include <string.h>

void rs(char* str){
    int i(0);
    int j(0);

    while((*(str + i) = *(str + j++)) != '\0')
        if(*(str + i) != ' ')
            i++;
    return;
}

int main()
{
    std::string str = "Hello World";
    char* result = strcpy((char*)malloc(str.length()+1), str.c_str());
    rs(result);

    std::cout << result << std::endl;

    return 0;
}

char* result = strcpy((char*)malloc(str.length()+1), str.c_str());

This seems more suited for a C program than a C++ one. In fact, with C++11, you don't need to write your own function.

Behold: std::remove_if

Note:

  std::remove_if

  Removes all elements satisfying specific criteria from the range
  [first, last) and returns a past-the-end iterator for the new end of
  the range.

  A call to remove is typically followed by a call to a container's
  erase method, which erases the unspecified values and reduces the
  physical size of the container to match its new logical size.

This is what you would do:

str.erase(remove_if(str.begin(), str.end(), ::isspace), str.end());

I am not sure if this is the safest or most efficient method, but I think it's definitely an improvement to what you have done.


Mutable default argument usage I have a function that returns a ChromeDriver instance and logs browser information only once in a session:

from selenium import webdriver
def init_chromedriver(path, timeout=30, _l=[]):
    driver = webdriver.Chrome(executable_path=path)
    driver.set_page_load_timeout(timeout)
    if not _l:
        _l.append(None)
        logger = logging.getLogger('driver')
        for k, v in driver.capabilities.items():
            logger.log(logging.INFO, f"{k}: {v}")
    return driver

Is it a good practice or should I use another method?

A global flag would be clear and simple:

_LOGGED_DRIVER = False

def init_chromedriver(path, timeout=30):
    driver = webdriver.Chrome(executable_path=path)
    driver.set_page_load_timeout(timeout)
    global _LOGGED_DRIVER
    if not _LOGGED_DRIVER:
        _LOGGED_DRIVER = True
        logger = logging.getLogger('driver')
        for k, v in driver.capabilities.items():
            logger.log(logging.INFO, f"{k}: {v}")
    return driver

Alternatively, you could replace the function after it has been called for the first time:

def _init_chromedriver(path, timeout=30):
    driver = webdriver.Chrome(executable_path=path)
    driver.set_page_load_timeout(timeout)
    return driver

def init_chromedriver(path, timeout=30):
    driver = _init_chromedriver(path, timeout)
    logger = logging.getLogger('driver')
    for k, v in driver.capabilities.items():
        logger.log(logging.INFO, f"{k}: {v}")
    global init_chromedriver
    init_chromedriver = _init_chromedriver
    return driver

But I think the global flag would be clearer.


Convert decimal to string I wrote a C# program that converts decimal to string. It is not the best but it works. I'm looking for inputs from others on how can it be improved? One aspect that baffled me was how to find out total number of digits after decimal point? Right now I've to pass it as a separate parameter. It is in C#, but I'm open to any programming language. credit goes to all authors that I found online and little bit to me

    static void Main(string[] args)
    {
        string s = decToStr(123456.789M,3);
        Console.WriteLine(s);
        Console.ReadLine();
    }

    static string decToStr(decimal d,double totaldec)
    {
        if (d == 0)
            return "0";
        int i = (int)d;
        decimal fracPart = d - i;
        int fracPartInt = 0;

        StringBuilder strResult = new StringBuilder();
        StringBuilder fracStrResult = new StringBuilder();

        strResult = ProcessInt(strResult,i);
        //not sure how to find total number of digts after decimal point
        fracPartInt = (int)(fracPart * (decimal)Math.Pow(10, totaldec));

        fracStrResult = ProcessInt(fracStrResult, fracPartInt);

        strResult.Append(".").Append(fracStrResult);

        return strResult.ToString();
    }

    static StringBuilder ProcessInt(StringBuilder sb, int i)
    {
        string digits = "0123456789";
        while (i > 0)
        {
            sb.Insert(0, digits[i % 10]);
            i = i / 10;
        }

        return sb;

    }

You don't really need to know the length of the fractional part.  Simply separate the fractional from the integral and handle each one separately.

static string DecToStr(decimal value)
{
    int integral = 0;
    decimal fractional = 0;
    SplitDecimal(value, out integral, out fractional);
    StringBuilder sb = new StringBuilder();
    while(integral > 0)
    {
        sb.Insert(0,integral % 10);
        integral /= 10;
    }
    sb.Append('.');
    while(fractional != 0)
    {
        int temp = 0;
        SplitDecimal(fractional * 10, out temp, out fractional);
        sb.Append(temp);
    }
    return sb.ToString();
}
static void SplitDecimal(decimal value,out int integral,out decimal fractional)
{
    integral = (int)value;
    fractional = value - integral;
}

EDIT  added check for negative

static string DecToStr(decimal value)
{
    int integral = 0;
    decimal fractional = 0;
    StringBuilder sb = new StringBuilder();
    if(value < 0)
    {
        value *= -1;
        sb.Append('-');
    }
    SplitDecimal(value, out integral, out fractional);
    while(integral > 0)
    {
        sb.Insert(0,integral % 10);
        integral /= 10;
    }
    sb.Append('.');
    while(fractional != 0)
    {
        int temp = 0;
        SplitDecimal(fractional * 10, out temp, out fractional);
        sb.Append(temp);
    }
    return sb.ToString();
}
static void SplitDecimal(decimal value,out int integral,out decimal fractional)
{
    integral = (int)value;
    fractional = value - integral;
}


Getting distinct array of objects I would like to get a distinct array of objects. The code below is the first thing that came to mind.

Do you see any room for improvements?

function distinct(objectArray){

    var distinctResult = [];

    $.each(objectArray, function(i, currentObject){
        if(!exists(distinctResult, currentObject))
            distinctResult.push(currentObject);
    });

    return distinctResult;
}

function exists(arr, object){
    var compareToJson = JSON.stringify(object),
        result = false;
    $.each(arr, function(i, existingObject){
        if(JSON.stringify(existingObject) === compareToJson) {
            result = true;
            return false; // break
        }
    });

    return result;
}

var testList = [{ foo: 1 }, { foo: 1 }, { bar: 1 }, { bar: 2, baz: 3 }];

console.log(distinct(testList));

Your code looks OK and will work but I have a few issues with it.

jQuery

You're using $.each in your distinct() function - this is fine, but it means I can now not use your function unless I support jQuery. jQuery is not really known as a general purpose utility library, so if I'm using Angular or Ember, I'm probably not going to want to use your function.

$.each is designed primarily for iterating the DOM rather than general arrays.

Reusability

Your distinct function is not very re-usable. You don't take a function as a parameter for matching, which means I have to rely on your method of determining equality. This might work for your problem space, but it does not for mine - Let us say I want to determine equality between base 64 strings. I cannot use your equality function there, and so if I have an array of base 64 strings I'm buggered.

Comparing JSON strings.

An interesting idea, I will give you that, but this is a bit... weird, and incurs a performance overhead. It certainly threw me off. While there is nothing intrinsically wrong with this approach, it feels dirty.

Behold lodash/uniq!

With all this said and done, there is already a utility function to perform this exact purpose in lodash (arguably the best general-purpose utility library in JavaScript). Granted, it only works on arrays, but so does your example. It also comes with the ability to specify your own matcher, and performs optimizations on sorted arrays.

var testList = [{ foo: 1 }, { foo: 1 }, { bar: 1 }, { bar: 2, baz: 3 }];
console.log(_.uniqBy(testList, function(obj) { return JSON.stringify(obj); });


Django query to find all items without a FK relationship except for excluded list I have a Book model and an Author model that has a FK relationship to Book (I realize that it should be the other way around, but I had to change the names of the real items to Book and Author for proprietary reasons.)  Related name on this relationship is authors.

I am trying to find all Books that do not have an Author or Authors, except for an exclude list of tuples called BOOKS_WITH_NO_AUTHORS that is about 25 items long and unlikely to grow much.  The pair of items in the tuples are unique together.  I can change this list of tuples to a dictionary or other format if that is helpful.

Everything works but it is very slow.  The database of books and authors is large.

BOOKS_WITH_NO_AUTHORS = [(1979, "ABC123"), (1928, "DEF456")]

def missing_author(request):

    excluded_books = [Book.objects.get(publish_year=book[0], book_num=book[1]).book_id
                      for book in BOOKS_WITH_NO_AUTHORS]

    books_missing_authors = Book.objects. \
        filter(authors=None). \
        exclude(book_id__in=excluded_books). \
        order_by('publish_year')

    return render(request,
                  'missing_author.html',
                  {"data": books_missing_authors})

One of the inefficiencies in your code is when you're  collecting the list of ids of excluded_books. If you have 25 BOOKS_WITH_NO_AUTHORS, you're hitting the database 25 times.

I would recommend using the Q objects.
https://docs.djangoproject.com/en/1.11/topics/db/queries/#complex-lookups-with-q-objects

This should substantially speed it up:

from django.db.models import Q

BOOKS_WITH_NO_AUTHORS = [(1979, "ABC123"), (1928, "DEF456")]

def missing_author(request):

    # Turn list of BOOKS_WITH_NO_AUTHORS into list of Q objects
    queries = [Q(publish_year=publish_year, book_num=book_num) for publish_year, book_num in BOOKS_WITH_NO_AUTHORS]

    # Take one Q object from the list
    query = queries.pop()

    # Or the Q object with the ones remaining in the list
    for item in queries:
        query |= item

    books_missing_authors = Book.objects. \
        filter(authors=None). \
        exclude(query). \
        order_by('publish_year')

    return render(request,
                  'missing_author.html',
                  {"data": books_missing_authors})


Implementations of unzip in Racket I've been practicing my folds and tail-call recursive programming, and I wanted to work on unzip for the general case as opposed to just pairs. This was very easy to do with standard tail call recursion. However, I was also told that any time you can use tail call recursion on a list, you could also use a fold. I was eventually able to figure out an algorithm that only uses folds, but it wasn't nearly as simple as, or even exactly a translation of, the tail recursive version. Are there any improvements or changes I could make to either of these algorithms?

Tail-call recursive

; Unzip a list of equally lengthed lists
; Takes car of every list and conses it to acc, then reverses acc at the end
; (unzip '((1 4 7 10) (2 5 8 11) (3 6 9 12))) => '((1 2 3) (4 5 6) (7 8 9) (10 11 12)
(define (unzip lst [acc '()])
  (if (or (empty? lst) (empty? (car lst)))
      (reverse acc)
      (unzip (map cdr lst) (cons (map car lst) acc))))

Folds only:

; Takes the last element and turns it into a list of singletons as a seed
; For each remaining element, it conses each element onto its respective list
(define (unzip lst)
  (define (list->singletons lst)
    (foldl (λ (x acc) (cons `(,x) acc)) '() (reverse lst)))
  (define (cons* heads tails)
    (foldr (λ (hd tl acc) (cons (cons hd tl) acc)) '() heads tails))
  (if (empty? lst)
      lst
      (foldl (λ (x acc) (cons* x acc))
             (list->singletons (car (reverse lst))) (cdr (reverse lst)))))

Doing it with folds pretty much begs you to create a custom accumulator and recursive function or fold within a fold like that.

A couple notes. I don't think foldr is tail-recursive. Also (λ (x acc) (cons* x acc)) is redundant. You can just use cons*

You're also using (reverse lst) in a few different places. You could create a separate definition here, or refactor.

(define (unzip lst)
  (define (template lst) ;;a list of null lists
    (if (null? lst)      ;;of the proper length
        '()
        (foldl (λ (x acc) (cons '() acc)) '() (car lst))))
  (define (cons* heads tails)
    (reverse (foldl (λ (hd tl acc)
                       (cons (cons hd tl) acc))
                    '() heads tails)))
  (reverse
      (foldl cons* template lst)))

Not much chage, but perhaps a bit easier to see what's going on. If your wanting to do it with a single fold, thats a bit tricker.

(define (unzip L)
 (define (mangle glob)
   (let loop ((news '()) (acc glob))
      (if (or (null? (car acc)) (pair? (car acc)))
          (cons (reverse news) (car acc))
          (loop (cons (car acc) news) (cdr acc)))))
  (reverse (apply foldl (lambda z (mangle z))'() L)))

Notice the lambda is just grabbing a list off all arguments, and we are applying foldl to the lists of lists, as if we are folding multiple lists. However arity is unknown so I've got to take all the arguments as a list, and figure out what is new and what was the accumulator on each round. As is this will choke on a list of lists of lists, but maybe an interesting approach.


Admin page select function I am using this script to get the admin pages via this query string ?page= once the admin is logged in.

Just needing a review, opinions

Function:

// GET PAGE FROM QUERY STRING
public function page_select($page) {
    $ext = '.php';
    $file = 'pages/'.$page.$ext;
    if(file_exists($file)) {
        return $file;
    }
    else {
        $file = 'pages/404'.$ext;
        return $file;
    }
}

Index.php

if(isset($_COOKIE["Cckiuas"]) && isset($_SESSION["loggedin"])) {
    if(isset($_GET['page'])) { $page = trim(htmlentities($_GET['page'])); }
    else { $page = 'dashboard'; }
    include($backend->get_base_url().INCLUDES."header.php");
    include($backend->page_select($page));
    include($backend->get_base_url().INCLUDES."footer.php");
}

Let's start with the critical issues:

You are vulnerable to directory attacks, I can get you to include any file by using the ../ to traverse up the directory structure, revealing details on your system:

example.com/index.php?page=../../../../../../../etc/passwd%00

The %00 represents the null byte character (See Null Byte Injection), which will terminate the string (thus ignoring your .php suffix). If I input that, I got all the users and passwords on the server (assuming the PHP user has read access, which it shouldn't, but most likely has).
The $ext variable is redundant, are you by any chance going to stop using .php extension even though you won't stop using PHP? (Because, you know, the code is PHP)
htmlentities is pointless if you aren't placing the target string in HTML. (And even if you were, htmlspecialchars is probably better)
INCLUDES is magic. Immutable global variables (like constants) are still global!
Consistent spacing and line-breaks - If you started with multiline in functions and control structures, stick with it.


Flagging employees whose spending is close to the budgeted amount I am working with an SQLite database. Is this code written the best way it can be and correct? The code:

Selects from the db adding a column for the percentage difference between two columns
Loops through the values of the columns
If the date is in the past
If the price is greater than 500 and the percentage difference is less than 1st argument set flag to 1
Else if the price is less than 500 and the percentage difference is less than 2nd argument set flag to 1
Otherwise keep the flag as 0

from datetime import datetime as dt

def calculateEmployeeSpend(read_cursor, flag_higher_amount, flag_lower_budget):

    read_cursor.execute("SELECT distinct b.employee_id, b.amount, "
                "s.spend, b.date, b.amount - s.spend as spend_left,  "
                "100.0*(b.amount - s.spend) / b.amount As PercentDiff FROM employee_budget_upload "
                "As b JOIN employee_budget_spent As s ON  b.employee_id = s.employee_id where b.amount != 0")

    for employee_id, amount, spend, date, spend_left, percent_diff in read_cursor:
        flag=0
        date_of_amount = dt.strptime(date, "%d/%m/%Y")
        if date_of_amount <= dt.now():
            if amount > 500 and percent_diff < int(flag_higher_amount) or amount < 500 and percent_diff < int(flag_lower_budget):
                flag=1

        write_cursor.execute('INSERT OR REPLACE INTO employee_overall_spend VALUES (?,?,?,?,?,?,?,?)',
                         (employee_id, amount, spend, date, spend_left, percent_diff, dt.now().strftime("%d/%m/%Y"), flag))

There's two ways you can go about this. Stick with your current way, or use SQL more.

Current way

I'd change your for loop to assign to one value.
I'd make a function that tells you if the values have the flag.
I'd use *value to tuple unpack into the write_cursor tuple.

And so would get something like:

from datetime import datetime as dt

def get_flag(employee_spend, higher, lower):
    _, amount, _, date, _, percent_diff = employee_spend
    return int(
        dt.strptime(date, "%d/%m/%Y") <= dt.now()
        and (
            amount > 500
            and percent_diff < int(higher)
            or amount < 500
            and percent_diff < int(lower)
        )
    )

def calculateEmployeeSpend(read_cursor, flag_higher_amount, flag_lower_budget):
    higher, lower = int(flag_higher_amount), int(flag_lower_budget)

    read_cursor.execute("SELECT distinct b.employee_id, b.amount, "
                "s.spend, b.date, b.amount - s.spend as spend_left,  "
                "100.0*(b.amount - s.spend) / b.amount As PercentDiff FROM employee_budget_upload "
                "As b JOIN employee_budget_spent As s ON  b.employee_id = s.employee_id where b.amount != 0")

    for employee_spend in read_cursor:
        flag = get_flag(employee_spend, higher, lower)
        write_cursor.execute(
            'INSERT OR REPLACE INTO employee_overall_spend VALUES (?,?,?,?,?,?,?,?)',
            (*employee_spend, dt.now().strftime("%d/%m/%Y"), flag)
        )

Use SQL

If you chose to use SQL I'd change it so that you pass higher and lower to the SQL query.
I'd move the flag logic into the SQL query.
I'd move the current time logic into the SQL query. I found how to get the current time from here.
I'd look into moving the SQL commands into their own files, so that they're easier to maintain.

I'm don't know SQL, but you could use something like:

CALCULATE_EMPLOYEE_SPEND_READ = '''\
DECLARE @curr_date DATETIME;
SET @curr_date = GETDATE();

SELECT
    e.employee_id,
    e.amount,
    e.spend,
    e.date,
    e.spend_left,
    e.percent_diff,
    @curr_date as curr_date,
    case when
        e.date <= @curr_date
        and (
            e.amount > 500
            and e.percent_diff < ?
            or e.amount < 500
            and e.percent_diff < ?
        )
        then 1
        else 0
    end
    as flag
FROM (
    SELECT distinct
        b.employee_id,
        b.amount,
        s.spend,
        b.date,
        b.amount - s.spend as spend_left,
        100.0 - 100.0*s.spend / b.amount as percent_diff
    FROM employee_budget_upload As b
    JOIN employee_budget_spent As s
    ON  b.employee_id = s.employee_id
    WHERE b.amount != 0
) as e
'''
CALCULATE_EMPLOYEE_SPEND_WRITE = 'INSERT OR REPLACE INTO employee_overall_spend VALUES (?,?,?,?,?,?,?,?)'

def calculateEmployeeSpend(read_cursor, higher, lower):
    read_cursor.execute(CALCULATE_EMPLOYEE_SPEND_READ, (int(higher), int(lower)))
    for employee_spend in read_cursor:
        write_cursor.execute(CALCULATE_EMPLOYEE_SPEND_WRITE, employee_spend)


Maintaining encapsulation seems to promote redundant classes I have two classes that are extremely similar and it seems a bit redundant but I want to maintain encapsulation.

First I have a servicestack route that accepts the following Request class:

public class CreateManualShipmentRequest
{
    public Address Sender { get; set; }
    public Address Recipient { get; set; }
    public PackageSizeAndWeight Package { get; set; }
    public Payor Payor{ get; set; }

    public List<string> SpecialServices { get; set; }
    public decimal InsuredValue { get; set; }
    public string ServiceType { get; set; }
    public string ServiceProvider { get; set; }

    public string LabelImageFormat { get; set; }
    public bool TestMode { get; set; }
}

So I need to take this information and pass it to a function that creates a specific request based on the provider. For example:

FedexHelpers.BuildFedexCreateShipmentRequest();

The easiest way to pass the information I need would be to make the function accept the request object as a parameter. These seems to violate encapsulation however because the function doesn't need to know it's a request, what provider has been selected. This lead me to create a new class:

public class ShipmentRequestInfo
{
    public Address Sender { get; set; }
    public Address Recipient { get; set; }
    public PackageSizeAndWeight Package { get; set; }
    public Payor Payor { get; set; }

    public List<string> SpecialServices { get; set; }
    public decimal InsuredValue { get; set; }
    public string ServiceType { get; set; }
}

So now I can pass the builder function this object without sharing any information about the request object at all. I even created a constructor that accepts the request object:

public ShipmentRequestInfo(CreateManualShipmentRequest request)
{
    Sender = request.Sender;
    Recipient = request.Recipient;
    Package = request.Package;
    Payor = request.Payor;

    SpecialServices = request.SpecialServices;
    InsuredValue = request.InsuredValue;
    ServiceType = request.ServiceType;

    LabelImageFormat = request.LabelImageFormat;
}

I feel I have achieved encapsulation this way as I can create the ShipmentRequestInfo wherever I need but it seems redundant to have such a similar class.

I was thinking about other possibilities and came up with a few:

Have the request class contain a ShipmentRequestInfo object. This will make it harder to read, you'd have to dig into the ShipmentRequestInfo class to read the object. It would also require the JSON sent to the API route to be nested one layer further.
Overload the builder function to accept a request object or a generic object. This might cut down on a bit of code, I wouldn't need the constructor in the ShipmentRequestInfo object, and I wouldn't have to create it to pass it to the builder function. What I don't want to end up is with a function that has too many overloads though.
Have CreateManualShipmentRequest inherit ShipmentRequestInfo. I'm not sure if this is a better option than having ShipmentRequestInfo as a property of CreateManualShipmentRequest, it seems pretty similar except that it couples the classes together more tightly?

I'm pretty new to OOP and want to make sure I'm considering all my options and designing classes properly.

What I see here is a great scenario for composition!

What you say is that you're annoyed because CreateManualShipmentRequest already has the shipment informations. Alright so, why doesn't CreateManualShipmentRequest contains ShipmentRequestInfo instead of repeating all the information?

public class ShipmentRequestInfo
{
    public Address Sender { get; set; }
    public Address Recipient { get; set; }
    public PackageSizeAndWeight Package { get; set; }
    public Payor Payor { get; set; }

    public List<string> SpecialServices { get; set; }
    public decimal InsuredValue { get; set; }
    public string ServiceType { get; set; }
}

public class CreateManualShipmentRequest
{
    public ShipmentRequestInfo ShipmentRequestInfo { get; set; }

    public string ServiceProvider { get; set; }

    public string LabelImageFormat { get; set; }
    public bool TestMode { get; set; }
}

You're right that having redundant code is a code smell, but creating another class to keep encapsulation is most of the time a pretty good thing to do.

I wanted to address the comment stating that you could use interfaces. I feel like it's not a bad solution, but I don't think it's the right one.

Implementing an interface is kind of a "is a" relationship.

But, could you say : "CreateManualShipmentRequest is a ShipmentRequestInfo? I don't think so. Though, saying "CreateManualShipmentRequest contains ShipmentRequestInfo" seems pretty logical to me!

Sure, readability and serialization are good points. For the serialization, well.. APIs are good, you'll have to write a little bit more code to deal with that. Readability itself isn't that affected. But are those points valuable than :

Not having redundant code
Passing the wrong object to a method
Implementing an interface that is misleading

? I feel like you should use composition.


Correct modularization and array usage I am trying to brush up on Java after not programming for a few years. The idea behind this code is simply to get used to object oriented programming, modularization techniques, and basic arrays. Any comments and critiques are obviously welcome!

Here is my code:

/*
 * This code is meant to ask the user for ten sentences,
 * store them in an array, and print that array out one sentence at a time.
 * The goal is to work on modularization and general OOP principles as well as basic arrays and loops.
 */

import java.util.Scanner;

/*
 * ArrayBuilder is a public class that creates an array of ten strings
 * and gives the ability to print them out with the printMessages
 * method.
 */

public class ArrayBuilder{
    //Initialize an array
    public String[] s;

    /*
     * The build method generates ten sentences based on user input
     * and stores them in an array of strings.
     */
    public String[] build(){
        //Create the 10 index array
        s = new String[10];
        //Initialize a scanner object to receive user input
        Scanner scanner = new Scanner(System.in);
        //Loop 10 times asking the user for a new sentence each time
        for (int i = 0; i < 10; i++){
            System.out.println("Enter a new sentence: ");
            //Store the input in the 's' array
            s[i] = scanner.nextLine();
        }
        //Return the 's' array to make it available to the printMessages method
        return s;
    }

    /*
     * The printMessages method loops through a given array and prints the
     * string at each index.
     */
    public void printMessages(String[] s){
        //Loop as many times as there are strings in the list, printing out each index
        for (int i = 0; i < s.length + 1; i++){
            System.out.println(s[i]);
        }
    }

    /*
     * Main method that instantiates an ArrayBuilder object, then calls the
     * build and printMessages methods to create and display an array of user input.
     */
    public static void main(String[] args){
        //Create a new ArrayBuilder object
        ArrayBuilder AB = new ArrayBuilder();
        //Invoke the build method to build up an array
        AB.build();
        //Invoke the printMessages method to display what is stored in the array
        AB.ptrintMessaages(AB.s);
    }
}

thanks for sharing your code.

here are my thoughts about it:

Naming

Finding good names is the hardest part in programming, so always take your time to think about the names of your identifiers.

avoid single character names / abbreviations

Since the number of characters is quite limited in most languages you will soon run out of names. This means that you either have to choose another character which is not so obviously connected to the purpose of the variable. And/or you have to "reuse" variable names in different contexts. Both makes your code hard to read and understand for other persons. (keep in mind that you are  that other person yourself if you look at your code in a few month!) The same is true for (uncommon) abbreviations. You might find them being obvious today, while you're actively dealing with the problem, but You might have to "relearn" them if you worked on something else for a while.

On the other hand in Java the length of identifier names names is virtually unlimited. There is no penalty in any way for long identifier names. So don't be stingy with letters when choosing names.

Choose you names from the problem domain, not from the technical solution.

Since you provided an "academic" example there is not much of a "problem domaine" here tough. I only want to mention it for the records...

Magic numbers

your code has some magic numbers. This are literal values with a special meaning like here:

    s = new String[10];
    //Loop 10 times asking the user for a new sentence each time
    for (int i = 0; i < 10; i++){

You should introduce constants with meaningful names:

    s = new String[ARRAY_SIZE];
    //Loop 10 times asking the user for a new sentence each time
    for (int i = 0; i < ARRAY_SIZE; i++){

Single Responsibility/Separation of Concerns

In your
excample
you mix business logic with user interaction like this:

public String[] build(){
    s = new String[10];
    Scanner scanner = new Scanner(System.in);
    for (int i = 0; i < 10; i++){
         // begin user interaction
        System.out.println("Enter a new sentence: ");
        s[i] = scanner.nextLine();
         // end user interaction
    }
    return s;
}

You should separate that into specialized methods. This would give you the option to move that UI-methods in classes of their own moving your architection foreward to the Model Vew Controller (MVC) Pattern.

dependency injection

Your class has the Scanenr class as a dependency. You instantiate this dependency within a method that represents your business logic.

You'd better pass in the Scanner instance as a constructor parameter allowing you to exchange the implementation later (maybe with a mock for unittesting your code)

reduce visibility (Interface segaration principle)

All your methods are public but (i guess) non of them is ment to be called by users. By default make all your methods private unless you explicitly want them to be either accessed by other code (public) or be overridden by subclasses (protected).

useless comments

You program has lots of comments. This used to be a desired feature 20 ears ago.

You should not add comments just for formal reasons. Such comments are useless at best but usually they turn into lies as the code develops because the first coder changing the content might forget to change the comment too and the next coder refuses to change the comment because she does not know if the comment is wrong or the code...

  Could you give an example of how to accomplish what you are talking about under the section "Single Responsibility/Separation of Concerns"? – J. Tate

You could intrioduce a UI interface for your "application" like this:

interface UserInterface{
    String requestElementFromUser();
}

and an implementation for use with the commandline (most likely in its own *.java file):

class CommandLineuserInterface implements UserInterface{
    private final Scanner commandlineInput;
    private final PrintWriter commandlineOutput;
    CommandLineuserInterface(Scanner commandlineInput, PrintWriter commandlineOutput){
      this.commandlineInput= commandlineInput;
      this.commandlineOutput= commandlineOutput;
    }
    @Override
    public String requestElementFromUser(){
        commandlineOutput.println("Enter a new sentence: ");
        return commandlineInput .nextLine();
    }
}

In your code you would inject an instance of this to your class instanc:

public static void main(String[] args){
    ArrayBuilder AB = new ArrayBuilder(new CommandlineUserInterface(new Scanner(System.in), System.out));

Store that in you app:

public class ArrayBuilder{
    //Initialize an array
    public String[] s;
    private final UserInterface userInterface;
    public ArrayBuilder(UserInterface userInterface){
       this.userInterface= userInterface;
    }

and use that in your code:

   for (int i = 0; i < 10; i++){
        s[i] = userInterface.requestElementFromUser();
   }

The benefit here is that you easily can change the implementation of UserInterface to get the elements from a database, a web service or any even a graphical dialog without ever touching this code again.
The only existing code to change is your main method. But tha't OK since the major responsibility of main is constructing the applications object tree.


Querying a data structure that contains various triples I wrote code that queries a data structure that contains various triples - 3-item tuples in the form subject-predicate-object. I wrote this code based on an exercise from Programming the Semantic Web textbook.

def query(clauses):
    bindings = None
    for clause in clauses:
        bpos = {}
        qc = []
        for pos, x in enumerate(clause):
            if x.startswith('?'):
                qc.append(None)
                bpos[x] = pos
            else:
                qc.append(x)
        rows = list(triples(tuple(qc)))
        if bindings == None:
            # 1st pass
            bindings = [{var: row[pos] for var, pos in bpos.items()}
                        for row in rows]
        else:
            # >2 pass, eliminate wrong bindings
            for binding in bindings:
                for row in rows:
                    for var, pos in bpos.items():
                        if var in binding:
                            if binding[var] != row[pos]:
                                bindings.remove(binding)
                                continue
                        else:
                            binding[var] = row[pos]
    return bindings

The function invocation looks like:

bg.query([('?person','lives','Chiapas'),
          ('?person','advocates','Zapatism')])

The function triples inside it accepts 3-tuples and returns list of 3-tuples. It can be found here.

The function query loops over each clause, tracks variables (strings that start with '?'), replaces them with None, invokes triples, receives rows, tries to fit values to existing bindings.

How can I simplify this code? How can I make it more functional-style, without nested for-loops, continue keyword and so on?

Your code would be helped by splitting parts of it out into functions.
Your continue statement does nothing
Rather then having a special case for the first time through, initialize bindings to [{}] for the same effect.

My version:

def clause_to_query(clause):
    return tuple(None if term.startswith('?') else term for term in clause)

def compatible_bindings(clause, triples):
     for triple in triples:
        yield { clause_term : fact_term
            for clause_term, fact_term in zip(clause, row) if clause_term.startswith('?')
        }

def merge_bindings(left, right):
    shared_keys = set(left.keys()).intersection(right.keys)

    if all(left[key] == right[key] for key in shared_keys):
        bindings = left.copy()
        bindings.update(right)
        return bindings
    else:
        return None # indicates that a binding cannot be merged

def merge_binding_lists(bindings_left, bindings_right):
    new_bindings = []
    for left, right in itertools.product(bindings_left, bindings_right):
        merged_binding = merge_bindings(left, right)
        if merged_binding is not None:
            new_bindings.append( merged_binding )

    return new_bindings

def query(clauses):
    bindings = [{}]
    for clause in clauses:
        query = clause_to_query(clause)
        new_bindings = compatible_bindings(clause, triples(query))
        bindings = merge_binding_lists(bindings, new_bindings)

    return bindings


Present table of data, acquired from an endpoint, with option to sort by different values I have built a Vue app which fetches data from an endpoint and presents that data in a table with sortable columns.

The app is based on this React challenge which is to create a table of users and their properties, from the data provided in the endpoint.

Here are the user stories fulfilled:

  I can see a table of the Free Code Camp campers who've earned the most brownie points in the past 30 days.
  I can see how many brownie points they've earned in the past 30 days, and how many they've earned total.
  I can toggle between sorting the list by how many brownie points they've earned in the past 30 days and by how many brownie points
  they've earned total.

Here is my code (working example):

const endpoint = 'https://fcctop100.herokuapp.com/api/fccusers/top/recent'

const store = {
  state: {
    sortingBy: ''
  }
}

new Vue({
  el: '#app',
  data() {
    return {
      data: [],
      state: store.state
    }
  },
  mounted() {
    Vue.http.get(endpoint).then(response => {
      this.data = response.body
    }, response => {
      console.warn('There was a data error:', response)
    })
  },
  methods: {
    sortByRecent() {
      this.data = _.sortBy(this.data, 'recent').reverse()
      store.state.sortingBy = 'recentDescending'
    },
    sortByAllTime() {
      this.data = _.sortBy(this.data, 'alltime').reverse()
      store.state.sortingBy = 'allTimeDescending'
    },
    getActiveClass(string) {
      return {active: store.state.sortingBy.includes(string)}
    }
  }
})
#app {
  display: flex;
  flex-direction: column;
  align-items: center;
}
.active {
  font-weight: bold;
}
button {
  font-size: 20px;
}
img {
  border-radius: 50%;
  height: 30px;
  width: 30px;
}

  <button @click="sortByRecent">Sort by Recent Points</button>
  <button @click="sortByAllTime">Sort by All Time Points</button>

        #

        Username
        Recent Points
        Total Points

      {{index + 1}}

      {{user.username}}
      {{user.recent}}
      {{user.alltime}}

Here is a repo with all the code (bootstrapped with vue-loader webpack).

My questions are:

I have placed a method for computing a CSS classname in methods - getActiveClass. In the documentation, it specifies that such methods can live inside the computed object:

  We can also bind to a computed property that returns an object.

However when doing so, I get the following console error in the browser:

  Uncaught TypeError: _vm.getActiveClass is not a function

Why does placing this method inside a computed object render this error?
My project was bootstrapped with vue-loader's webpack preset. In order to make the API call to my endpoint, I had to import Vue so that I could use vue-resource. Should the API call be made in main.js and then passed in as a prop to App.vue to avoid importing Vue in multiple places?
The data is received from the API call in any of the lifecycle methods beforeCreate, created, beforeMount and mounted. Which is the correct one for it to go?
Does it make sense for this code to all live in one single file component, or should it be split up into separate smaller components?

I'm new to Vue myself so this may not be 100% accurate, just my own opinions. Kudos for learning and reaching out for feedback! You seem to have grokked a lot so I'm impressed.

Computed properties are used the same as data properties. You do not call them like methods, and you can't pass parameters to computed properties. Instead of :class="getActiveClass('recent')" you'd have to use something like :class="{active: isSortedByRecentPoints}" or :class="recentPointsHeaderClasses". But what you have works just fine IMO.
There is no penalty to importing Vue in multiple places, and it is actually good practice. Think of it like Java imports, you are declaring all of our external dependencies in your header so you know what kinds of operations this code is likely to perform without reading every single line.
The correct hook is determined by your use case. For this example any one of the hooks mentioned would work fine. For more general advise, I tend to prefer the mounted hook, but that's just me and I don't know what I don't know. Will the component get mounted multiple times? Will you have all of the data you need to make the call at created? I'm not familiar enough with the details of Vue to give more in depth advise.
For this example one Vue instance works just fine. If you want to experiment you can extract a true component from your code and create an API: The table data would be an input prop, and any actions on that data would be output events. I wouldn't consider sorting to be a reasonable output event. How about a view details event on the user?

If you're looking for extra credit I'd recommend looking into Vuex for your store. Currently you're mutating the store directly, which kind of goes against the whole point of using a store. Also a store is overkill for this example, but I'd recommend it for learning purposes. I'll give you a hint: Instead of using state in your data object you will probably want to bind directly to sortingBy and then use $store.commit to alter it in your methods. Commit will trigger a mutation to change the state, which will in turn update your component's data. You can do something similar for the table data - put it in a store and dispatch a fetchdata action to GET your data, then commit it to the state. This may seem like overkill at first (and it probably is for simple apps) until you start playing around with the time travelling debugger in the Vue devtools, hot module replacement, and the like. Super cool stuff.


Scraping required fields exhausting a full website I've written some script in python using lxml library which is capable of dealing with a webpage with complicatedly layed-out next page links. My scraper is able to parse all the next page links without going to the next page and hardcoding any number to the lastmost link and scrape the required fields flawlessly. Although I tried to do the whole thing specklessly, any improvement on this will be highly appreciable. Here is what I've tried with:

import requests
from lxml import html

main_link = "https://www.yify-torrent.org/search/1080p/"
base_link = "https://www.yify-torrent.org"

def get_links(item_link):
    response = requests.get(item_link).text
    tree = html.fromstring(response)
    last_page = tree.cssselect('div.pager a:contains("Last")')[0].attrib["href"].split("/")[-2].replace('t-','')
    links = [item_link +"t-{0}/".format(page) for page in range(int(last_page) +1)]
    for link in links:
        process_docs(link)

def process_docs(nextpage_link):
    response = requests.get(nextpage_link).text
    tree = html.fromstring(response)
    for items in tree.cssselect('div.mv'):
        name = items.cssselect('h3 a')[0].text
        try:
            genre = items.xpath('.//div[@class="mdif"]//li[b="Genre:"]/text()')[0] # I don't know If css selector could do this
        except IndexError:
            genre = ""

        try:
            imd = items.xpath('.//div[@class="mdif"]//li[b="Rating:"]/text()')[0] # Used xpath in lieu of css selector
        except IndexError:
            imd = ""

        print(name, genre, imd)

get_links(main_link)

This is quite clean and understandable.

I would pinpoint only a couple of things:

to avoid code repetition and to generalize getting search result properties, what if you would collect all the properties (genre, quality, size etc) into a dictionary:

properties = {
    label.text: label.xpath("following-sibling::text()")[0]
    for label in items.xpath(".//div[@class="mdif"]//li/b")
}

As a bonus, you would not need the try/except part at all.
as usual, you may reuse a session to boost your HTTP requests part performance

Some other minor notes:

it looks like that you make an extra request to the page number 0, adjust your range if this is the case
items variable name is probably a bit misleading, at the very least it should be item since you are iterating over search result items one by one
put get_links() call to under the if __name__ == '__main__' to avoid the code being executed if the module is ever imported
I would also return or yield from the get_links and process_docs functions instead of printing things out


Project Euler 14: Longest Collatz Sequence I've brute forced solve Project Euler 14. This is my first attempt at trying to solve it, by caching already solved answers, and skipping them as I run it, I am finding the program take's almost twice as long as the simple brute force method of checking each number.

It works, but I could also use some insight on how to stop calculating, once I run into an integer we have already solved, and how to better write the entire problem into a class.

/*Project Euler 14.
The following iterative sequence is defined for the set of positive integers
n → n/2 (n is even)
n → 3n + 1 (n is odd)
Using the rule above and starting with 13, we generate the following sequence:
13 → 40 → 20 → 10 → 5 → 16 → 8 → 4 → 2 → 1
It can be seen that this sequence (starting at 13 and finishing at 1) contains 10 terms. Although it has not been proved yet (Collatz Problem), it is thought that all starting numbers finish at 1.
Which starting number, under one million, produces the longest chain?
NOTE: Once the chain starts the terms are allowed to go above one million
*/

#include <iostream>
#include <vector>

//I use this class to mark if a number has been solved.

class Collatz{
public:
    bool solved = false;
    unsigned terms = 0;
};

int main (){

    const unsigned maxnum = 1000000;
    std::vector<Collatz>cz(maxnum);
    unsigned larger = 0; //will be used to find largest sequence.
    unsigned largest = 0;

//This loop checks to see if the num has been solved yet.
//If not it solves its sequence, and marks all numbers in the sequence.

    for(unsigned a = 2; a < maxnum; a++){
        if(cz[a].solved == false){
            std::vector<unsigned long long>cache;
            cache.push_back(a);
            for(unsigned long long n = a; n != 1;){
                if(n % 2 == 0)
                    n = n/2;
                else
                    n = 3*n+1;
                cache.push_back(n);
            }
            //caching all already sovlved numbers
            for(unsigned c = 0; c < cache.size(); c++){
                if(cache[c] <= maxnum){
                    cz[cache[c]].solved = true;
                    cz[cache[c]].terms = cache.size() - c;
                }
            }
        }
        //Storing largest term.
        if(larger < cz[a].terms){
            larger = cz[a].terms;
            largest = a;
        }
    }

    std::cout << largest << std::endl;
}

An obvious improvement is to check n for being already solved on each iteration of for n loop. As coded, you still go all the way to 1, which might be quite long. Something along the lines of

for ( a = 2; a < maxnum; ++a) {
    std::vector<unsigned long long>cache;
    for (n = a; n != 1 && !cz[n].solved; ) {
        cache.push_back(n);
        n = (n%2 == 0)? n/2: 3*n+1;
    }
    // make cached data solved
    ...
}

If cz[1].solved is preset to true, the loop condition can be simplified to

for (n = a; !cz[n].solved; )


Optimization of average calculation I have a list of dictionaries, with keys 'a', 'n', 'o', 'u'.
Is there a way to speed up this calculation, for instance with NumPy? There are tens of thousands of items in the list.

The data is drawn from a database, so I must live with that it's in the form of a list of dictionaries originally.

x = n = o = u = 0
for entry in indata:
    x += (entry['a']) * entry['n']  # n - number of data points
    n += entry['n']
    o += entry['o']
    u += entry['u']

    loops += 1

average = int(round(x / n)), n, o, u

I'm not sure if there really is a better way to do this. The best I could come up with is:

import itertools
from collections import Counter

def convDict(inDict):
    inDict['a'] = inDict['a'] * inDict['n']
    return Counter(inDict)

average = sum(itertools.imap(convDict, inData), Counter())
average['a'] = average['a'] / average['n']

But I'm still not sure if that is better than what you originally had.

Counter is a subclass of dict. You can get items from them the same way you get items from a normal dict. One of the most important differences is that the Counter will not raise an Exception if you try to select a non-existant item, it will instead return 0.


DAO recordset effiency I have a form that consists of about 60 fields that all update when choosing a different customer from a drop down. I feel like my current way is not the best way as it takes about 30 seconds to update when changing customers.  Currently I have:

Private Sub ClientSelection_AfterUpdate()

On Error GoTo errhandler

Dim dbTemp As DAO.Database
Dim rsTemp As DAO.Recordset
Dim strSQL As String

DoCmd.Hourglass True

sYear = Me.YearSelection.Value
    Me.CustomerID = Me.CustomerSelection.Column(0)
    CustomerID = Mid(Me.CustomerID.Value, 2, 36)

Set dbTemp = CurrentDb()
Set rsTemp = dbTemp.OpenRecordset("SELECT Field1 FROM Table where CustomerID = '" & CustomerID & "' and MonthYear = #01/01/" & sYear & "#")
Set rsTemp2 = dbTemp.OpenRecordset("SELECT Field1 FROM Table where CustomerID = '" & CustomerID & "' and MonthYear = #02/01/" & sYear & "#")

Me.JanCharges.Value = rsTemp("Charges")
Me.FebCharges.Value = rsTemp2("Charges")

rsTemp.Close
rsTemp2.Close
Set rsTemp = Nothing
Set rsTemp2 = Nothing
Set dbTemp = Nothing

Exit_ClientSelection_AfterUpdate:
Exit Sub

errhandler:
 MsgBox "Error " & Err.Number & ": " & Err.Description & " in " & _
   VBE.ActiveCodePane.CodeModule, vbOKOnly, "Error"
    Resume Exit_ClientSelection_AfterUpdate

End Sub

Both of these commands basically happen 60 times (5 categories consisting of 12 fields(months)).  Then even further down I close all the recordsets and set them to Nothing.  All of this works just takes a little longer than I want.  What is a more efficient to accomplish this?

I'm going to leave the ms-access specifics to more proficient ms-access reviewers, as I'm not all that familiar with DAO and Access VBA.
If possible, switch to SQL Server ADODB.

DEADC0DE
Unless I'm missing something, you're inlining the SQL statements and Dim strSQL As String is an unused declaration.
These can be hard to spot without an add-in though. MZ-Tools has a review source code feature that can find unused variables and procedures; Rubberduck has a code inspections feature that can find that too, and a number of other code issues (disclaimer: I own and maintain rubberduck, see tag info).

UX Issue?
DoCmd.Hourglass True

You're turning the hourglass on, but you never turn it back off. I would expect this:
Exit_ClientSelection_AfterUpdate:
    DoCmd.Hourglass False
    Exit Sub

The error handler is problematic:
MsgBox "Error " & Err.Number & ": " & Err.Description & " in " & _
    VBE.ActiveCodePane.CodeModule, vbOKOnly, "Error"

This cannot reliably be reporting the source of the error, unless there's only 1 module in your VBA project. Proof:
Module1
Public Sub Boom()
    MsgBox VBE.ActiveCodePane.CodeModule
End Sub

Module2
Public Sub DoSomething()
    Module1.Boom
End Sub

If Module2 is the opened module when you run DoSomething, the message will say Module2, not Module1 where the "error" would have happened.
And it requires adding a reference to the VBIDE extensibility library in your VBAProject, which requires special security handling. Not ideal.


Converting a JSON object to XML to be passed to an ASMX webservice What I have is a scenario where we need to be able to call any ASMX web service and pass any data structure to it that has been constructed on the JavaScript side.

Until now, I have been creating service references for ASMX web services in order to call them, but in this scenario, I can no longer do that. This is my first go at passing XML to service references manually.

What I am not too sure of is the level of complexity required to get a JSON string into basically an XML string to be passed to the webservice.

 string func_name = "//some asmx web service function"
 string json_string = "//some json encoded string here";
 XmlDocument doc = (XmlDocument)JsonConvert.DeserializeXmlNode(variables, "root");
 StringWriter stringWriter = new StringWriter();
 XmlTextWriter xmlTextWriter = new XmlTextWriter(stringWriter);
 doc.WriteTo(xmlTextWriter);
 string xml_parameters = stringWriter.ToString();

 //remove <root> and </root> from the string
 xml_parameters = xml_parameters.Replace("<root>", null).Replace("</root>", null);

string request_xml= "<?xml version=\"1.0\" encoding=\"utf-8\"?>" +
                            "<soap:Envelope xmlns:soap=\"http://schemas.xmlsoap.org/soap/envelope/\" xmlns:xsi=\"http://www.w3.org/2001/XMLSchema-instance\" xmlns:xsd=\"http://www.w3.org/2001/XMLSchema\">" +
                               "<soap:Body>" +
                                   "<" + func_name + " xmlns=\"http://tempurl.org\" >" +
                                     xml_parameters+
                                   "</" + func_name +" >" +
                               "</soap:Body>" +
                           "</soap:Envelope>";

HttpResponseMessage response = client.PostAsync(uri, new StringContent(xml, Encoding.UTF8, request_header_type)).Result;

An awful lot of objects have been instantiated here in order to get my JSON converted into XML to be passed to the web service. Can anyone suggest any improvements or potential problems?

Use camelCase to name local variables instead of words separated by underscores. Although there are no guidelines for them it is common practice among C# programmers.

You should always call XmlWriter.Create to get XmlWriter instead of creating XmlTextWriter.

StringWriter, XmlWriter and any other writer implement IDisposable and thus should be used within using section:

using (var stringWriter = new StringWriter())
using (var xmlTextWriter = XmlWriter.Create(stringWriter))
{
    // ... do your stuff that requires stringWriter and xmlTextWriter
}

You should try to avoid .Result for async operation. Try to mark your method with async and use await for client.PostAsync.

Creation of XML looks awful... I suggest to extract your request XML template into XML document with kind of placeholders like:

<?xml version="1.0" encoding="utf-8"?>
<soap:Envelope xmlns:soap=\"http://schemas.xmlsoap.org/soap/envelope/"
               xmlns:xsi=\"http://www.w3.org/2001/XMLSchema-instance"
               xmlns:xsd=\"http://www.w3.org/2001/XMLSchema\">
    <soap:Body>
        <FUNC_NAME xmlns="http://tempurl.org">
            XML_PARAMETERS
        </FUNC_NAME>
    </soap:Body>
</soap:Envelope>

Then in your code just read this file to string with File.ReadAllText and replace FUNC_NAME and XML_PARAMETERS with string.Replace with real values.