Lexical Analysis Program For my computer science class, I was required to write a lexical analysis program that would perform several functions on a std::string. The assignment required a function for each of the following:

count number of a certain substring
count number of words excluding numbers
count number of unique words (excludes repeated words)
count number of sentences (by end punctuation)
average words per sentence
lexical density as percent (unique word count / word count * 100)

My code is fairly long, but here it is:

#include<iostream>
#include<string>
#include<sstream>
#include<algorithm>
#include<iterator>
#include<vector>

using namespace std;

int substringCount(const string&, const string&);
int wordCount(const string&);
int uniqueWordCount(const string&);
int sentenceCount(const string&);
double wordsPerSentence(const string&);
double lexicalDensity(const string&); // different words / total words * 100

int main() {
    string source = ("This is the source text for this program.");
    cout << "\"is\" count: " << substringCount(source," is ") << endl;
    cout << "Word count: " << wordCount(source) << endl;
    cout << "Sentence count: " << sentenceCount(source) << endl;
    cout << "Words per sentence: " << wordsPerSentence(source) << endl;
    cout << "Lexical density: " << lexicalDensity(source) << endl;
    return 0;
}

vector<int> substringIndices(const string& str, const string& sub) { // Find indices of substrings
    vector<int> indices = {};
    for (unsigned int i = 0; i < str.size(); i++) {
        unsigned int t = i;
        for (unsigned int j = 0; j < sub.size(); t++, j++) {
            if (t >= str.size()) {
                break;
            }
            if (str[t] == sub[j]) {
                if (j + 1 == sub.size()) indices.push_back(i);
                continue;
            } else {
                break;
            }
        }
    }
    return indices;
}

vector<string> splitByWhitespace(const string& str) { // split by whitespace
    vector<string> tokens;
    istringstream iss(str); // create istringstream
    copy(istream_iterator<string>(iss),istream_iterator<string>(),back_inserter(tokens)); // copy into tokens
    return tokens;
}

vector<string> deleteNumbers(const vector<string>& data) {
    vector<string> tr = {};
    for (unsigned int i = 0; i < data.size(); i++) {
        for (char c: data[i]) {
            if (string("0123456789").find(c) == -1) continue;
            else goto mainLoop;
        }
        tr.push_back(data[i]);
        mainLoop: continue;
    }
    return tr;
}

int substringCount(const string& str, const string& sub) { // get number of substrings
    return substringIndices(str,sub).size();
}

int wordCount(const string& str) { // wordcount (based on whitespace)
    return deleteNumbers(splitByWhitespace(str)).size();
}

int uniqueWordCount(const string& str) { // unique word count
    vector<string> words = deleteNumbers(splitByWhitespace(str));
    sort(words.begin(),words.end()); // sort
    words.erase(unique(words.begin(),words.end()),words.end()); // delete extra non-unique words
    return words.size();
}

int sentenceCount(const string& str) { // get number of sentences (based on end punctuation)
    return substringCount(str,".") + substringCount(str,"?") + substringCount(str,"!");
}

double wordsPerSentence(const string& str) {
    return double(wordCount(str)) / double(sentenceCount(str));
}

double lexicalDensity(const string& str) {
    return double(uniqueWordCount(str) * 100) / double(wordCount(str));
}

I am relatively new to C++, so if there are any common practices that I am missing it would be great to point them out.

Name Spaces
A most common practice is to preface types and functions supplied by the
Standard Template Library, sometimes known as the Standard Library with
std:: rather than ignoring names spaces by the statement

using namespace std;

This will become quite helpful as the problems and code become more complex
and you need to include other name spaces. Some languages such as C don't
support namespaces, however, C++ does and it is quite a useful feature.
More complex programs may use multiple name spaces and each one of these
name spaces can contain a definition for functions such as sort() or
find_if(), or overrides on operators such as <<. You may even need to
write your own sort() or find(). Using std::sort allows you not to
create your own name space when you write your own sort() with the same
arguments.

Initialization of Empty Containers
There is no need for the empty braces on this line:

    vector<int> indices = {};

The vector container class has a constructor that will initialize an empty
vector properly.

    vector<int> indices;

Use the Features of the Container Classes and the Standard Template Library
You may find this website useful for learning all of the features a
particular container class or the standard library.

There are definitely some functions you could be using, such as
std::find(), std::find_if(), std::count_if() and std::string::substr() that could definitely shorten the code.

It might be wise to investigate std::map as well for counting words.

GOTO
There are almost always ways to avoid using goto in C++. In rare cases
goto may be appropriate for error handling, more so in C than in
C++ because C++ has try{}/catch{} and exceptions.

In the following code there is really no reason to use a goto:

vector<string> deleteNumbers(const vector<string>& data) {
    vector<string> tr = {};
    for (unsigned int i = 0; i < data.size(); i++) {
        for (char c: data[i]) {
            if (string("0123456789").find(c) == -1) continue;
            else goto mainLoop;
        }
        tr.push_back(data[i]);
        mainLoop: continue;
    }
    return tr;
}


Function for setting a streak of strings in a Pandas I have a df with artist names in one column, I want to find how many times the artists repeats through the column.

Example:

df = pd.DataFrame({"Artist":["Tim Maia","Jorge Ben","Tim Maia",
                             "Tim Maia","Jorge Ben","Roberto Carlos",
                             "Roberto Carlos","Roberto Carlos",
                             "Roberto Carlos",
                             "Dire Straits"]})

I've already had some success using itertuples(), so that was my starting point for this function:

def artist_streak(df):
    # created to check if artists repeats, not returned
    artists = []
    # this will be returned
    streaks = []
    # since each observation counts, makes sense to start at 1
    count=1

    for row in df.itertuples():
        # artists[-1] in an empty list will raise IndexError:
        try:
            # Checking if current artist is the same as the last appended
            if artists[-1] == row.Artist:
                    count += 1
                    artists.append(row.Artist)
                    streaks.append(count)
            else:
                # The artist does not repeated
                count = 1
                artists.append(row.Artist)
                streaks.append(count)
        # Here we set things right for the first iteration
        except IndexError:
            artists.append(row.Artist)
            streaks.append(count)

    return streaks

Usage:

df["Result"] = artist_streak(df)

My concerns about this code are:

1. The artists list [ ]:

It is the way I could think of checking the current iterator item vs. the previous one, I don't know if it is a good way.

2. The try/except part:

As it addresses the specific case of list artists being empty, I don't know if it hurts performance (i.e. there is a faster way to do it).

Any other suggestions on how to make it faster, cleaner, or even pythonic will be welcome.

For starter, your implementation is too tightly tied to your data, making it not reusable for other kind of dataframe where you would want to count streaks. If you provided a column (pd.Series) to the function instead, it would be much more versatile; allowing calls such as count_streak(df.Artist), count_streak(pd.Series([1, 2, 2, 1, 1, 3]) or even count_streak('abcdccbdddbbbbcaaacbddba').

Doing so, you would remove the need for itertuple and favor regular iteration. This would ease the use of enumerate to check for the previous element:

def count_streak(sequence):
    streaks = []

    for index, element in enumerate(sequence):
        if not index or sequence[index - 1] != element:
            count = 1
        else:
            count += 1
        streaks.append(count)

    return streaks

Much simpler. But this is all without taking into account the wonderful itertools module and its groupby function. It will make group of consecutive data that evaluate equals, yielding both the value, and the entire group. The other advantage being that any iterable will be accepted:

def count_streak(iterable):
    return [
        i for _, g in itertools.groupby(iterable)
        for i, _ in enumerate(g, start=1)
    ]

Since you wish to stay in pandas' world, you also have a groupby method available on DataFrames and Series. But contrary to itertools.groupby that stops at each boundaries, pandas' groupby return all occurences of similar data even if some are discontinuous. But you can make use of cumsum and cumcount to get an equivalent and vectorize these operation instead of a slow python loop. This would require a pandas.Series as parameter, though, not any iterable:

def count_streak(serie):
    return serie.groupby((serie != serie.shift()).cumsum()).cumcount() + 1

usage being

df['Streak'] = count_streak(df.Artist)

And since you commented on performances, here is a small benchmark comparing the approaches. I’ve thrown in a couple extra approaches, out of curiosity. The code to produce the results is:

import pandas as pd
import numpy as np
import timeit

def python_streaks(sequence):
  streaks = []
  for index, element in enumerate(sequence):
    if not index or sequence[index - 1] != element:
      count = 1
    else:
      count += 1
    streaks.append(count)
  return streaks

def python2_streaks(sequence):
  streaks = [1] * len(sequence)
  for index, element in enumerate(sequence):
    if index and sequence[index - 1] == element:
      streaks[index] += streaks[index - 1]
  return streaks

def numpy_streaks(sequence):
  streaks = pd.Series(np.ones(len(sequence), dtype=int))
  for index, element in enumerate(sequence):
    if index and sequence[index - 1] == element:
      streaks[index] += streaks[index - 1]
  return streaks

def itertools_streak(iterable):
  return [
    i for _, g in itertools.groupby(iterable)
    for i, _ in enumerate(g, start=1)
  ]

def pandas_streaks(serie):
  return serie.groupby((serie != serie.shift()).cumsum()).cumcount() + 1

for size in range(8):
  size = 10**size
  for name in ('python', 'python2', 'numpy', 'pandas', 'itertools'):
    setup = 'import pandas as pd;from random import choice;from __main__ import {}_streaks as streaks; df = pd.DataFrame({{"Artist": [choice("abcde") for _ in range({})]}})'.format(name, size)
    elapsed = timeit.timeit('df["Streaks"] = streaks(df.Artist)', setup=setup, number=1000)
    print(name, size, elapsed)

And the results are (I stopped pure Python loops as they were taking too much time):

or, without using log scale on the Y-axis:

So depending on the amount of data you wish to process, either itertools (up to 10000ish elements) or pandas version is better.


Format a number to include commas This is my attempt to format a number and put commas where appropriate. I was wondering if this could be done in a simpler way.

It's used by calling format_string_number("123456789") and prints 123,456,789.

def format_string_number(st):
    inputVal = list(st)
    inputVal.reverse()
    returnVal = []

    for index, value in enumerate(inputVal):
        returnVal.insert(0,value)
        if (index+1) %3 == 0:
            returnVal.insert(0,",")

    if returnVal[0] == ",":
        returnVal.pop(0)

    print("".join(returnVal))

A few comments:

It seems odd to write a function for formatting a number that takes a string as an argument;
Your function doesn't currently deal with float numbers correctly (and, as it takes a string argument, you can't easily check isinstance(arg, int));
Your function prints the output rather than returning it, severely limiting its usefulness; and
You aren't compliant with the style guide.

Also, as pointed out in the comments, this is handled by format already, and can be used in str.format too:

>>> "Number: {:,}".format(123456.789)
'Number: 123,456.789'


Way to check if a byte array contains another byte Array I'm checking if an byte array contains a another Byte Array with this Code:

private int IndexOf(int index, byte[] AllBytes, byte[] searchByteArray)
{
    for (int i = index; i <= AllBytes.Length - 1 - searchByteArray.Length - 1; i++)
    {

        for (int j = 0; j <= searchByteArray.Length - 1; j++)
        {
            if (AllBytes[i + j] == searchByteArray[j])
            {
                if (j + 1 == searchByteArray.Length)
                    return i;
            }
            else
                break;
        }
    }
    return -1;
}

That works perfect and I get the index of the first byte back.

But the problem is I want to check very large Data.

My "big" Array contains around 900000000 Bytes and my searchArray about 10-20 Bytes. In that way my function is very very slow. Is there a way to make a better performance?

I need the Index of every found match so I do it into a loop:

for (int i = (IndexOf(0, allData, suchBytes) - 1); i < allData.Length; i++)
{
    Debug.WriteLine(i);
    tmpIndex = IndexOf(i, allData, suchBytes);

    if (tmpIndex > -1)
    {

        Byte1_Index.Add(tmpIndex + suchBytes.Length);
        Debug.WriteLine("Counter: " + Byte_Index_Counter);
        i = tmpIndex;
        Byte_Index_Counter++;

    }
}

And it takes a way too much time...

The biggest improvement would be to keep track of potential matches while you are finding if the current match is valid.  As Raymond mentioned, in his comment, you could look at KMP or Boyer Moore algorithms but you trade performance for complexity.

Searching though 900000000 Byte will just take some time.  There is no magic answer for that but I did a couple of test one with just searching while keeping track of potential matches and one using producer/consumer pattern so I could use the TPL and not block the main threads.  Neither one of these was actually faster than the other - in the end it was a wash and both ran anywhere between 7.5 seconds to 8 seconds to run on my machine.  The async version was a little better as it didn't just "hang" the system while it ran but I don't know if you can use Task in your solution.   I did try to bench your code on my machine but it had index out of bound issues.  I fixed them with what I thought would be correct but never got a bench mark because I got tired of waiting for the result.

Here is a example of searching that keeps track of potential matches.  There is more optimization that could happen.  Like not checking rest of source if no matches found and it is past the length of the search.  Also could do a pattern match but I'm just checking the first character.

Word of warning I did not do extensive testing on this

public class ByteSearch
{
    public static IEnumerable<int> Search(byte[] source, byte[] searchByteArray)
    {
        if (source == null)
        {
            throw new ArgumentNullException(nameof(source));
        }
        if (searchByteArray == null)
        {
            throw new ArgumentNullException(nameof(searchByteArray));
        }

        var searchLength = searchByteArray.Length;
        var sourceLength = source.Length;
        if (searchLength > sourceLength || searchLength == 0)
        {
            yield break;
        }

        var matches = new List<ByteSearchStore>(searchLength);
        for (var i = 0; i < sourceLength - 1; i++)
        {
            var currentByte = source[i];

            // found a potential match
            if (currentByte == searchByteArray[0])
            {
                if (searchByteArray.Length == 1)
                {
                    yield return i;
                }
                else
                {
                    matches.Add(new ByteSearchStore
                    {
                        StartingIndex = i,
                        Index = -1
                    });
                }
            }

            // check matches
            var count = matches.Count;
            var m = 0;
            while (m < count)
            {
                var potentalMatch = matches[m];
                if (searchByteArray[potentalMatch.Index + 1] == currentByte)
                {
                    potentalMatch.Index++;
                    if (potentalMatch.Index == searchLength - 1)
                    {
                        yield return potentalMatch.StartingIndex;
                        count = 0;
                        matches = new List<ByteSearchStore>(searchLength);
                    }
                    else
                    {
                        m++;
                    }
                }
                else
                {
                    matches.RemoveAt(m);
                    count--;
                }
            }
        }
    }

    private class ByteSearchStore
    {
        public int Index { get; set; }
        public int StartingIndex { get; set; }
    }
}

The program builds up a list of matches each time it hits the first character of the search string then ejects them once they stop matching the search string.  We only check the top match to see if it's the search string because it would be the longest and if it matches we wipe the list as we don't want matches inside our current match.

Example how I ran the program

var random = new Random();
var source = Enumerable.Range(0, 900000000).Select(_ => (byte)random.Next(byte.MinValue, byte.MaxValue)).ToArray();
var search = Enumerable.Range(0, 9000).Select(_ => (byte)random.Next(byte.MinValue, byte.MaxValue)).ToArray();
var test = ByteSearch.Search(source, search).ToArray();

If you want the TPL Producer/Consumer one I can post that but again it will be As Is


Determining if a string is a palindrome of a permutation I wrote code to determine if a string is a palindrome of a permutation. This exercise was taken from Cracking the Coding Interview. I'm looking for any tips on improving it.

#include <iostream>
#include <string>

/*
*   Determine if a string is a palindrome of a permutation.
*   For simplicity, assume each string contains ONLY alphabetical characters.
*/

//strip string of non-alphabetical characters
std::string strip_junk(std::string s){
    std::string output("");
    for(char& c : s)
        if(isalpha(c)){
            c = tolower(c);
            output += c;
        }
    return output;
}

//determine how many times each character appears in the string
int *get_char_counts(std::string input){
    static const int ascii_sub = 97;
    static int counters[26]{0};
    for(char c : input){
        counters[c - ascii_sub]++; //if c='a', c-ascii_sub = 0. if c='z', c-ascii_sub = 25
    }
    return counters;
}

//check if more than one character appears an odd number of times
bool check_chars(std::string input){
    int *char_counter = get_char_counts(input);
    bool flag = false;
    for(int i = 0; i < 26; ++i){
        if(*(char_counter + i) % 2 != 0){ //if number is odd, check if it appeared twice
            if(flag)
                return false; //it appeared twice, return false
            flag = true;
        }
    }
    return true;
}

//check if string is a palindrome of a permutation
bool palindromePermutation(std::string input){
    input = strip_junk(input);
    return check_chars(input);
}

int main(void){
    std::string input;
    std::getline(std::cin, input);
    std::cout << (palindromePermutation(input) ? "True" : "False") << std::endl;
}

I'd change a number of things in this code.

std::string output("");

No need for the empty initializer--it'll be initialized to an empty string by default (i.e., std::string output; is fine).

for(char& c : s)
    if(isalpha(c)){
        c = tolower(c);
        output += c;
    }

I don't see where you gain anything by modifying the input string here, instead of just writing the modified character to the output:

for (char c : s)
    if (isalpha(c))
        output += tolower(c);

Unfortunately, things aren't quite that simple. In most implementations, a plain char is signed, and passing a negative number to isalpha or tolower gives undefined behavior. Anytime you call either, you really need to convert to unsigned char first:

for (unsigned char c : s)
    if (isalpha(c))
       output += tolower(c);

Now when somebody tests with (for example) an accented character, it won't crash and burn.

static const int ascii_sub = 97;

This strikes me as wrong in at least a couple of different ways. First, the name strikes me as poor (ASCII, as such, is mostly obsolete, and even before that, it was unnecessarily narrow). I guess you mean the sub to refer to subtraction (somehow or other) but it's not immediately apparent how the two combine meaningfully.

Worse, it appears that your real intent is to just get the value of a. If you want that, it's almost always better to just use 'a'.

static int counters[26]{0};

Here I'd rather use something like static size_t counters['z'-'a']; Since it's static, it'll be initialized to zeros automatically. Using 'z' - 'a' makes it apparent what that 26 is intended to signify--and as a bonus, even makes it work with systems that use EBCDIC, where the distance from a to z is more than 26.

And, of course, the standard type for counting things where you can't ever have negative values is size_t (though a signed int like you've used will typically be perfectly fine).

Rather than using a static array, I'd probably use a std::vector (not static, and return it by value rather than returning a pointer to it).

Looking at check_chars, I think I'd do things a bit differently.  I'd probably use std::find_if to do the searching, something like this:

auto first = find_if(counts.begin(), counts.end(), [](size_t s) { return s % 2 != 0); });

auto second = find_if(first, counts.end(), [](size_t s) { return s % 2 != 0; });

return second != counts.end();

The first tries to find an odd value. The second tries to find a second odd value between the first, and the end. If the first failed, it'll return counts.end(). The second will try to search between the end and the end, and obviously conclude no such thing exists (very quickly), so it'll return counts.end() as well.

Finally this:

std::cout << (palindromePermutation(input) ? "True" : "False") << std::endl;

...can be simplified a bit as well:

std::cout << std::boolalpha << palindromePermutation(input) << "\n";

std::boolalpha is not only a bit shorter, but also uses the names defined in the locale, so when using (for example) a German locale, it could print out falsch and wahr (and with a Chinese locale, if Google Translate is to be trusted, 假 and 真) for "false" and "true", respectively.


Optimization of region of interest (ROI) drawing in the image I'm drawing rectangular ROI region by darkening the area outside of it as in this image:

But image.MakeTransparent takes too much time. What is the best way to increase drawing speed?

void DrawRoi(Bitmap Image, RectangleF rect)
{
        Rectangle roi = new Rectangle();

        roi.X = (int)((float)Image.Width * rect.X);
        roi.Y = (int)((float)Image.Height * rect.Y);
        roi.Width = (int)((float)Image.Width * rect.Width);
        roi.Height = (int)((float)Image.Height * rect.Height);

        Stopwatch timer = new Stopwatch();
        timer.Start();
        // graphics manipulation takes about 240ms on 1080p image
        using (Bitmap roiMaskImage = CreateRoiMaskImage(ImageWithRoi.Width, ImageWithRoi.Height, roi))
        {
                using (Graphics g = Graphics.FromImage(ImageWithRoi))
                {
                        g.DrawImage(Image, 0, 0);
                        g.DrawImage(roiMaskImage, 0, 0);
                        Pen borderPen = CreateRoiBorderPen(ImageWithRoi);
                        g.DrawRectangle(borderPen, roi);
                }
        }
        Debug.WriteLine("roi graphics: {0}ms", timer.ElapsedMilliseconds);
        this.imagePictureBox.Image = ImageWithRoi;
}

Bitmap CreateRoiMaskImage(int width, int height, Rectangle roi)
{
        Bitmap image = new Bitmap(width, height, PixelFormat.Format32bppArgb);
        using (Graphics g = Graphics.FromImage(image))
        {
                SolidBrush dimBrush = new SolidBrush(Color.FromArgb(64, 0, 0, 0));
                g.FillRectangle(dimBrush, 0, 0, width, height);
                SolidBrush roiBrush = new SolidBrush(Color.Red);
                g.FillRectangle(roiBrush, roi);
                image.MakeTransparent(Color.Red);
                return image;
        }
}

Pen CreateRoiBorderPen(Bitmap image)
{
        float width = ((float)(image.Width + image.Height) * 2.5f) / (float)(640 + 480);
        if (width < 1.0f)
                width = 1.0f;
        Pen pen = new Pen(Color.FromArgb(255, 0, 255, 0), width);
        return pen;
}

Well, you could try literally darkening the outside of the image by 25%, not by creating a 25%-opaque black area but just by calculating the pixel color values of the same color 25% darker, without the need to create a new 32bbp bitmap.

This might help: Formula to determine brightness of RGB color (StackOverflow). Based on this, let's say you choose Y = 0.2126 R + 0.7152 G + 0.0722 B as your luminance formula and 0.75 and the new brightness of the outside part of the image -- each pixel should be changed to:

Y = 0.75 * (0.2126 * R + 0.7152 * G + 0.0722 * B)
(R, G, B)' = (R - Y * 0.2126, G - Y * 0.7152, B - Y * 0.0722)

(Twice I realized that the formula I put here was quite wrong... this time I'm more confident, but it might be that this makes no sense when painted, I haven't tested it.)

This would require iterating through each individual pixel outside the central square, so not sure if it will be faster in the end... But as will all things performance, only measurements will tell!


Converting the weight of a potato into a letter grade I've just started learning C# using Rob Miles' C# Programming Yellow Book and some related lab exercises from his website. I did one of them and produced a solution that works. In Miles' book, he says that nesting if and else brackets is clumsy and introduces the switch. But switch apparently doesn't work with Boolean operators, e.g. case x < 201, so I can't use it. I could just have independent, non-nested if statements, though their conditions would have to be longer. Any other ideas?

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading.Tasks;

class Program
{
    static int GetIntNumber(int min, int max, string prompt, string inputPrompt)
    {
        int number;

        Console.Write(prompt);
        do  //loops as long as input is not between min and max
        {
            while (!int.TryParse(Console.ReadLine(), out number)) //loops as long as the input is not an integer
            {
                Console.Write("\nYou must enter an integer number!: ");
            }
            if (number < min || number > max) Console.Write(inputPrompt); //<--| write input prompt if number out of range
        } while (number < min || number > max);

        return number;
    }

    static void printGrade(string grade)
    {
        Console.WriteLine("\nYour potato is grade " + grade);
    }

    static void Main()
    {

        do
        {
            int weight = GetIntNumber(0, 1500, "\nEnter the weight of the potato in grams: ", "\nEnter an integer between 0 and 1500: ");

            if (weight < 201)
            {
                printGrade("X");
            }
            else
            {
                if (weight < 401)
                {
                    printGrade("A");
                }
                else
                {
                    if (weight < 801)
                    {
                        printGrade("B");
                    }
                    else
                        printGrade("Z");
                }
            }
            Console.WriteLine("\nPress any key to check the grade of another potato.");
        } while (Console.ReadKey(true).Key != ConsoleKey.Escape );

        Console.Read();

    }
}

You are very much correct that switch statements cannot take boolean operators like that: switch statements are used to build jump tables, and can only use constants to do so. However, you can easily come up with a fairly robust solution to your problem using a Dictionary and some LINQ.

Since you tagged your question beginner I'll be gentle with the explanation.

The first thing we'll do is make a Dictionary<int, string> which represents the lowest value associated with each grade.

var gradeMap = new Dictionary<int, string>
{
    [0] = "X",
    [201] = "A",
    [401] = "B",
    [801] = "Z",
};

So, this means if the value is >= 801, then it's Z, and so on.

Next, we'll simply one-line this entire operation in LINQ:

gradeMap.OrderByDescending(x => x.Key).FirstOrDefault(x => x.Key <= testWeight).Value

What's happening here? LINQ is Language Integrated Query, if you've ever worked with SQL you might recognize the Query bit, essentially, LINQ allows you to build code which looks and act's like a query against an IEnumerable object (which Dictionary is).

So if we take this step-by-step, we can start with our dictionary as follows:

0: X
201: A
401: B
801: Z

The gradeMap.OrderByDescending(x => x.Key) bit will first take the entire gradeMap dictionary, and order it by the key value in highest-to-lowest order:

801: Z
401: B
201: A
0: X

Next, we do a FirstOrDefault(x => x.Key <= testWeight), which will loop through all the elements (in the backwards order we have them) that are in the dictionary, and find the first one that matches our expression (x.Key <= testWeight). The x.Key is the reference to the key of the dictionary (our int values):

801: Z (Skipped)
401: B (Skipped)
201: A (Matched)

So, now we have the 201: A item from the dictionary, the last part is to get .Value on it which returns the string portion of it:

A

And viola, we have our value. :)

We can combine this together and get:

var testWeight = 205;
var grade = gradeMap.OrderByDescending(x => x.Key).FirstOrDefault(x => x.Key <= testWeight).Value;

So, the final thing we have to do is combine it with your original programme:

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading.Tasks;

class Program
{
    static int GetIntNumber(int min, int max, string prompt, string inputPrompt)
    {
        int number;

        Console.Write(prompt);
        do  //loops as long as input is not between min and max
        {
            while (!int.TryParse(Console.ReadLine(), out number)) //loops as long as the input is not an integer
            {
                Console.Write("\nYou must enter an integer number!: ");
            }
            if (number < min || number > max) Console.Write(inputPrompt); //<--| write input prompt if number out of range
        } while (number < min || number > max);

        return number;
    }

    static void printGrade(string grade)
    {
        Console.WriteLine("\nYour potato is grade " + grade);
    }

    static void Main()
    {
        do
        {
            var gradeMap = new Dictionary<int, string>
            {
                [0] = "X",
                [201] = "A",
                [401] = "B",
                [801] = "Z",
            };

            int weight = GetIntNumber(0, 1500, "\nEnter the weight of the potato in grams: ", "\nEnter an integer between 0 and 1500: ");

            printGrade(gradeMap.OrderByDescending(x => x.Key).FirstOrDefault(x => x.Key <= weight).Value);

            Console.WriteLine("\nPress any key to check the grade of another potato.");
        } while (Console.ReadKey(true).Key != ConsoleKey.Escape );

        Console.Read();
    }
}

The next best thing would be to extract the LINQ logic into a new method:

public string GetGrade(int weight)
{
    var gradeMap = new Dictionary<int, string>
    {
        [0] = "X",
        [201] = "A",
        [401] = "B",
        [801] = "Z",
    };

    return gradeMap.OrderByDescending(x => x.Key).FirstOrDefault(x => x.Key <= weight).Value;
}

Then our Main method becomes:

static void Main()
{
    do
    {
        int weight = GetIntNumber(0, 1500, "\nEnter the weight of the potato in grams: ", "\nEnter an integer between 0 and 1500: ");

        printGrade(GetGrade(weight));

        Console.WriteLine("\nPress any key to check the grade of another potato.");
    } while (Console.ReadKey(true).Key != ConsoleKey.Escape );

    Console.Read();
}

Now if you need to add a new grade, for whatever reason, it's trivial.

Also: if you're not using C#6.0, you will have to replace var gradeMap with the following:

var gradeMap = new Dictionary<int, string>
{
    {0, "X"},
    {201, "A"},
    {401, "B"},
    {801, "Z"}
};

Good work so far, hopefully you continue improving and learn more regarding C# and the paradigms it holds. :)

The answer by t3chb0t is also very good, I recommend reading it thoroughly as well (especially the bit about method overloading).


SQL query to count loans issued during each week Calculate the number of loans issued by the Regional offices for each business week in the period from 38 to 40 business week of 2011 inclusive. Sort by name of the regional unit, business week.

dataset and snippet here - http://www.sqlfiddle.com/#!18/53151/4

Are the joins correct in this query?

DDL:

create table territory_type(
    id int primary key identity (1,1),
    name nvarchar(50) not null,
);

create table territory(
  id int primary key identity (1,1),
  parent_id int null,
  name nvarchar(50) not null,
  territory_type_id int not null,
  foreign key(territory_type_id) references territory_type(id),
  constraint fk_tr_parent foreign key (parent_id) references territory(id) on delete no action
);

create table deal(
  dl_id int primary key identity (1,1),
  dl_valutation_date datetime not null,
  dl_sum decimal not null,
  dl_territory_id int not null,
  foreign key(dl_territory_id) references territory(id)
);

create table business_calendar(
  id int primary key identity (1,1),
  bc_year int not null,
  week int not null,
  date_begin date not null
);

QUERY:

select
        trd.name as "Regional office",
        bc.bc_year as "Year",
        bc.week as "Week",
        sum(d.dl_sum) as "Sum of credits",
        count(d.dl_id) as "Count of credits"
    from deal as d
    join business_calendar as bc
        on (bc.date_begin < d.dl_valutation_date
        or bc.date_begin = d.dl_valutation_date)
        and (dateadd(day, 6, bc.date_begin) > d.dl_valutation_date
        or dateadd(day, 6, bc.date_begin) = d.dl_valutation_date)
    join territory as t on t.id = d.dl_territory_id
    join territory as trd on t.parent_id = trd.id
    where bc.week in (38,39,40)
    group by trd.name,bc.bc_year, bc.week
    order by trd.name, bc.week asc;

Your query could be simplified.

use inner join to avoid ambiguity when reading the query
temporal interval join date between start and end (inclusive end)

join business_calendar as bc
        on (bc.date_begin < d.dl_valutation_date
        or bc.date_begin = d.dl_valutation_date)
        and (dateadd(day, 6, bc.date_begin) > d.dl_valutation_date
        or dateadd(day, 6, bc.date_begin) = d.dl_valutation_date)

 inner join business_calendar as bc
        on d.dl_valutation_date between bc.date_begin and dateadd(day, 6, bc.date_begin)

snippet:

select
    trd.name as "Региональное подразделение",
    bc.bc_year as "Год",
    bc.week as "Неделя",
    sum(d.dl_sum) as "Сумма выданных займов",
    count(d.dl_id) as "Кол-во займов"
from deal as d
inner join business_calendar as bc
    on d.dl_valutation_date between bc.date_begin and dateadd(day, 6, bc.date_begin)
inner join territory as t on t.id = d.dl_territory_id
inner join territory as trd on t.parent_id = trd.id
where bc.week in (38,39,40)
group by trd.name,bc.bc_year, bc.week
order by trd.name, bc.week asc


Returning largest product of longest subsequence between min and max This function returns the largest product of all elements of a list between the leftmost and rightmost min and max elements. Is there more readable and maybe compact way to rewrite the elif chain in this code:
from operator import mul
from functools import reduce

prod = lambda iterable: reduce(mul, iterable, 1)

def prod_between_min_and_max(arr):
    l_min, r_max = arr.index(min(arr)), len(arr)-1 - arr[::-1].index(max(arr))
    l_max, r_min = arr.index(max(arr)), len(arr)-1 - arr[::-1].index(min(arr))
    if len(range(l_min, r_max+1)) > len(range(l_max, r_min+1)):
        sl = slice(l_min, r_max+1)
    elif len(range(l_min, r_max+1)) < len(range(l_max, r_min+1)):
        sl = slice(l_max, r_min+1)
    elif prod(arr[l_min:r_max+1]) >= prod(arr[l_max:r_min+1]):
        sl = slice(l_min, r_max+1)
    else:
        sl = slice(l_max, r_min+1)
    return prod(arr[sl])

than this:
from operator import mul
from functools import reduce

cmp = lambda a, b: (a > b) - (a < b)
prod = lambda iterable: reduce(mul, iterable, 1)

def prod_between_min_and_max(arr):
    l_min, r_max = arr.index(min(arr)), len(arr)-1 - arr[::-1].index(max(arr))
    l_max, r_min = arr.index(max(arr)), len(arr)-1 - arr[::-1].index(min(arr))
    sl = [
        [
            slice(l_max, r_min+1),
            slice(l_min, r_max+1)
        ][
            prod(arr[l_min:r_max+1]) >= prod(arr[l_max:r_min+1])
        ],
        slice(l_min, r_max+1),
        slice(l_max, r_min+1)
    ][
        cmp(len(range(l_min, r_max+1)), len(range(l_max, r_min+1)))
    ]
    return prod(arr[sl])

Note: The code works in both Python 2 and Python 3.

The second example is terrible, don't use it.

You assign sl to either (l_min, r_max) or (l_max, r_min), and so you just need to change your ifs to become one if and an else.

First off len(range(l_min, r_max+1)) is the same as r_max+1 - l_min, and there's no need for the +1 if both sides have it. So your first if could become: r_max - l_min > r_min - l_max.

To merge the first and third, you should check it's the first, or explicitly check it's not the second and is the third. And so you can get:

def prod_between_min_and_max(arr):
    l_min = arr.index(min(arr))
    l_max = arr.index(max(arr))
    r_max = len(arr)-1 - arr[::-1].index(max(arr))
    r_min = len(arr)-1 - arr[::-1].index(min(arr))
    if (r_max - l_min > r_min - l_max
      or (r_max - l_min == r_min - l_max
          and prod(arr[l_min:r_max+1]) >= prod(arr[l_max:r_min+1]))):
        sl = (l_min, r_max)
    else:
        sl = (l_max, r_min)
    return prod(arr[slice(sl[0], sl[1]+1)])

This reminds me of FizzBuzz, as there doesn't seem to be a 'nice' solution.


Creating names from URLs I am creating a NAME column in a DataFrame and set its value based on substrings contained in another column.
Is there a more efficient way to do this?
import pandas as pd
df = pd.DataFrame([['www.pandas.org','low'], ['www.python.org','high']],
     columns=['URL','speed'])
print(df.head())

df['Name'] = df['URL']
print(df.head())

#set Name based on substring in URL
df.loc[df['Name'].str.contains("pandas", na=False), 'Name'] = "PANDAS"
df.loc[df['Name'].str.contains("python|pitone", na=False), 'Name'] = "PYTHON"
print(df.head())

Yes. Try to automate converting a URL to a name, instead of hardcoding the mapping. With only two URLs, your approach is doable, but as soon as you have to handle lots of different URLs, it will quickly become very painful.
Also, avoid first copying a column and then replacing every item in it. Rather, construct the column with the right contents directly. Here is an example:
df['Name'] = [url.split('.')[-2].upper() for url in df['URL']]

This will fail for proper URLs like https://www.pandas.org/index.html. If you want to handle that automatically, you'll have to properly parse them, for example using urllib.parse.


Random Distribution to fit an Average Python This is from my own question on Stack Overflow.

I want to get average numbers from a range such that they fit an intended average.  I expect something bell-curve-like, but asymmetrical unless the average is the mean of the range.  The function should allow for any degree of deviation.  In my Overflow question, I wanted it to pick from a list, but that's an easy modification if my idea works.

This is what I've come up with: (imports: numpy and random)

def randlist(minn, maxn, goaln, countn):
    nlist = []
    for i in range(0, countn):
        if len(nlist) > 10:
            ave = numpy.mean(nlist)
            if ave > goaln:
                a = random.uniform(minn, goaln)
            else:
                a = random.uniform(goaln, maxn)
        else:
            a = random.uniform(minn, maxn)
        nlist.append(a)
    return nlist

Trying it out:

b = randlist(1, 10, 7, 1000)
print len(b)
>> 1000
print numpy.mean(b)
>> 6.99951157861

I'm no mathematician; is this as functional as it appears to me to be?

If you want to express the field or distribution in a wave form, you should consider doing a Fourier Transformation on the function. Now, I must admit that I haven't understood exactly the sort of distribution function you are looking for. Numpy already has implementations for Gaussian distribution and such in the random module.

You can easily do so by:

numpy.random.normal(loc=7, scale=1.0, size=None)

This will produce a normal distribution with μ=7 and σ2=1.0

Now, about your code

def randlist(minn, maxn, goaln, countn):
    nlist = []

Consider renaming this function to the distribution's name that you are looking for. Such as normal for a normal distribution. Rename minn and maxn to min_ or max_. Since you are trying to avoid clashes with the keywords, you could also try using synonyms such as low and high.

for i in range(0, countn):

range(countn) is enough to start the list from 0.

if len(nlist) > 10:
    ave = numpy.mean(nlist)
    if ave > goaln:
        a = random.uniform(minn, goaln)
    else:
        a = random.uniform(goaln, maxn)
else:
    a = random.uniform(minn, maxn)

Since you are adding the first 10 numbers randomly to the list, you can do so inside the nlist itself in the form of a list comprehension (see sample code below).

def weighted_distribution(low, high, ev, count, start=10):
    result = [random.uniform(low, high) for i in range(start)]
    for i in range(count-start):
        if numpy.mean(result)> ev:
            result.append(random.uniform(low, ev))
        else:
            result.append(random.uniform(ev, high))
    return result

ev is the expected value for this distribution (mean).


'Tis the season for gift-wrapping When reviewing a Graham scan convex hull algorithm implementation, I wrote the following function in an answer:

giftWrap :: [Vector2] -> [Vector2]
giftWrap vs = result
  where
    -- Decides whether to keep or reject vertex b.
    -- Vertex b is provisionally kept if a left turn occurs there; the result
    -- is finalized only when a closed left-turn-only tour is completed.
    giftWrap' (a:b:c:vs)
      | vs == [] && dir == TurnLeft  = (True, [b, c])
      | vs == []                     = (False, [c])
      | dir == TurnLeft && final     = (True, (b:gw))
      | dir == TurnLeft              = giftWrap' (a:gw)
      | otherwise                    = (False, (a : c : vs))
      where
        dir = turn a b c
        (final, gw) = giftWrap' (b:c:vs)
    (_, result) = giftWrap' (vs ++ [head vs])

The purpose of this function is to keep only the vertices in the input sequence at which a left turn occurs.  However, if a right turn is encountered, we must drop the right-turn vertex from consideration, backtrack, and try again.

To achieve backtracking, I've designed the inner function to return a status flag in addition to the result.  I'm not satisfied with it though, as it looks ugly and is hard to follow.

Is there a better way to implement recursion with possible backtracking?

Is recursion the appropriate technique to use in the first place?

The simplest implementation that comes to my mind would keep the list split into two parts: One the already traversed points in reverse order, so that we can easily backtrack, and rest of unvisited points:

giftWrap :: [Vector2] -> [Vector2]
giftWrap (a:rest) = gw [] a rest
  where
    gw [] b (c:xs) = gw [b] c xs         -- beginning - just add the first point
    gw as@(a:as') b cs@(c:cs')
        | TurnRight <- turn a b c   = gw as' a cs      -- backtrack
        | otherwise                 = gw (b:as) c cs'  -- advance
    gw as b [] = reverse (b : as)

At the end, we need to reverse the result (unless we don't mind having the points reversed).

There is no need to check the case of the first point in the list, as it's always chosen so that it's a left turn.

Using pattern guards simplifies the call to turn.

I also added a small snippet that uses Test.QuickCheck to test the algorithm:

vectors :: Gen [Vector2]
vectors = nub <$> (listOf1 $ Vector2 <$> r <*> r)
  where
    r = fromIntegral <$> choose (-2 :: Int, 2 :: Int)

gwconvex_property :: Property
gwconvex_property = forAllShrink vectors (shrinkList shrinkNothing) $ \xs ->
    let rotate (x:xs) = xs ++ [x]
        vs = hull xs
        vs' = rotate vs
        vs'' = rotate vs'
    in counterexample ("Input: " ++ show vs) $
       counterexample ("Chained: " ++ show (chain vs)) $
       conjoin $ zipWith3 (\a b c -> turn a b c /= TurnRight) vs vs' vs''

main = quickCheck gwconvex_property

Note that the original implementation of giftWrap fails the test, either with

  Non-exhaustive patterns in function giftWrap'

or producing non-convex result, but I didn't have time to explore the details.


Parsing a Conditional statement with Python Recently I built a Usenet newsreader in Python and for that I built a keyword search which supports AND and OR functionalities (e.g., python AND django should bring up articles containing both. I defined a find function for this which takes a keyword query and a string and returns a bool signifying whether the string qualifies (i.e., for the above example, the string qualifies if it contains both 'python' and 'django').

Here's the snippet:

search = 'python AND django OR ruby AND rails'
query = search.split(' OR ')
query = [phrase.split(' AND ') for phrase in query]

def find(query, string):
    is_there = False
    for chunk in query: #OR
        chunk_qualified = False
        for word in chunk: #AND
            if word not in string:
                break
        else:
            chunk_qualified = True

        if chunk_qualified:
            is_there = True
            break

    return is_there

It works, but can I do it in a better manner (less code, more efficient)?

You can use the builtin function all() here to check phrases in a subquery are part of the string passed.

def get_sub_queries(query):
    for subquery in query.split(' OR '):
        yield subquery

def get_phrases(subquery):
    for phrase in subquery.split(' AND '):
        yield phrase

def find(query, string):
    for subquery in get_sub_queries(query):
        if all(phrase in string for phrase in get_phrases(subquery)):
            return True
    return False

Try to use better variable names(I know it's hard though :-)).

Apart from these I have broken your code into 3 functions now.

Contains logic to split queries to subqueries
Contains logic to split subquery to get phrases
Actual find() functionality

The reason behind this is that if the logic related to anyone of them changes later on then it won't complicate your search functionality and you can write separate tests for each of the three functionalities.

If you're on Python 3.3+ then use could also use yield from.


Program to compute final grade for weighted assignments I wrote a "gradebook" program.  This takes in grades for various assignments. The assignments types are weighted differently and final grade is computed at the end.  The code works but I can't help but think I am being ham handed in my approach. Should I be using a dictionary instead of storing things like hw_final, lab_final, and exam_final as global variables?

hw_tally = 0
hw_weight_count = 0
hw_final = 0
lab_tally = 0
lab_weight_count = 0
lab_final = 0
exam_tally = 0
exam_weight_count = 0
exam_final = 0

def assignment(kind, grade, weight=1):
    global hw_tally, hw_weight_count, hw_final, lab_tally, lab_weight_count, lab_final, exam_tally, exam_weight_count, exam_final
    if kind == 'hw':
        hw_weight_count += weight
        hw_temporary = (grade * weight)
        hw_tally += hw_temporary
        hw_final = hw_tally/hw_weight_count
    elif kind == 'lab':
        lab_weight_count += weight
        lab_temporary = (grade * weight)
        lab_tally += lab_temporary
        lab_final = lab_tally/lab_weight_count
    elif kind == 'exam':
        exam_weight_count += weight
        exam_temporary = (grade * weight)
        exam_tally += exam_temporary
        exam_final = exam_tally/exam_weight_count

assignment('hw', 81)
assignment('exam', 90, 4)
assignment('hw', 100, 2)
assignment('lab', 84, 3)
assignment('lab', 90, 2)
assignment('exam', 78)
assignment('exam', 85)
assignment('lab', 88)
assignment('exam', 75)

hw_weight_percent = 0.4
lab_weight_percent = 0.1
exam_weight_percent = 0.5

final_grade = ((hw_final * hw_weight_percent) + (lab_final *
lab_weight_percent) + (exam_final * exam_weight_percent))
print ("Final grade = ", final_grade)

Split assignment into three functions. homework, exam and labwork. Take:

def homework(grade, weight=1):
    global hw_tally, hw_weight_count, hw_final
    hw_weight_count += weight
    hw_temporary = (grade * weight)
    hw_tally += hw_temporary
    hw_final = hw_tally/hw_weight_count

Change homework into a class.

class Scores:
    def __init__(self):
        self.tally = 0
        self.weight_count = 0

    @property
    def final(self):
        return self.tally / self.weight_count

    def grade(grade, weight=1):
        self.weight_count += weight
        self.tally += grade * weight

Now you can use the above class to total all the separate things. And then perform your total on Scores.final.
Group the exam types into another class, say StudentScores.
Expand Scores to also take the percentage. And make final return the percentage.
Move your final calculation into StudentScores.

This can make:

class Scores:
    def __init__(self, percentage=1):
        self.tally = 0
        self.weight_count = 0
        self.percentage = percentage

    @property
    def final(self):
        return self.tally / self.weight_count * self.percentage

    def grade(self, grade, weight=1):
        self.weight_count += weight
        self.tally += grade * weight

class StudentScores:
    def __init__(self, scores):
        total = sum(scores.values())
        self._scores = {
            key: Scores(score / total)
            for key, score in scores.items()
        }

    def __getattr__(self, name):
        return self._scores[name]

    @property
    def grade(self):
        return sum(scores.final for scores in self._scores.values())

student = StudentScores({
    'homework': 0.4,
    'labwork': 0.1,
    'exam': 0.5
})

student.homework.grade(81)
student.exam.grade(90, 4)
student.homework.grade(100, 2)
student.labwork.grade(84, 3)
student.labwork.grade(90, 2)
student.exam.grade(78)
student.exam.grade(85)
student.labwork.grade(88)
student.exam.grade(75)

print("Final grade = ", student.grade)

This uses a lot of classes, and isn't really the best way if you're taking data from a file. And so if you store your results as a JSON file, and read it into Python however you want. Then you can default the weights into the JSON data. And then perform the rest of the code in a couple of comprehensions, wrapped in a sum:

results = {
    'homework': [
        0.4,
        [
            [81],
            [100, 2]
        ]
    ],
    'labwork': [
        0.1,
        [
            [84, 3],
            [90, 2],
            [88]
        ]
    ],
    'exam': [
        0.5,
        [
            [90, 4],
            [78],
            [85],
            [75]
        ]
    ]
}

def default_weights(results):
    return [
        [percentage, [(score + [1])[:2] for score in scores]]
        for percentage, scores in results.values()
    ]

def grade(results):
    return sum(
        percentage * sum(s*w for s, w in scores) / sum(w for _, w in scores)
        for percentage, scores in results
    )

print("Final grade = ", grade(default_weights(results)))


Constructing a tail cmd I wrote this without much thought:

(defn- tail-cmd
  "Construct a tail cmd"
  [file n & ignore-patterns]
  (clojure.string/join
   " | "
   (flatten [(format "tail -n %s -f %s" (or n 200) file)
             (map #(format "grep --line-buffered -v \"%s\"" %)
                  ignore-patterns)])))

On further thought, I'm not sure if it can simplified further. Can it be?

You may or may not consider this simpler:

(defn tail-cmd-2
  "Construct a tail cmd"
  [file n & ignore-patterns]
  (str "cat "
       file
       " | "
       (apply str (map #(format "grep -v \"%s\" | " %) ignore-patterns))
       "tail -f "
       (if n (str "-n " n))))

Or if you're willing to use the non-standard clojure.core.strint library, you can do the following.

(defn tail-cmd-5
  "Construct a tail cmd"
  [file n & ignore-patterns]
  (let [greps   (apply str (for [p ignore-patterns] (format "grep -v \"%s\" | " p)))
        minus-n (if n (str "-n " n))]
    (<< "cat ~{file} | ~{greps} tail -f ~{minus-n}")))

Here's the library source:
https://github.com/clojure/core.incubator


Monotone cubic interpolation http://en.wikipedia.org/wiki/Monotone_cubic_interpolation

We have implemented it using the formula from Wikipedia :

public class MonotoneCubicSplineInterpolation
    {
        public static double[] Calc(double[] xs, double[] ys, double[] x_interp)
        {
            var length = xs.Length;

            // Deal with length issues
            if (length != ys.Length)
            {
                IPDevLoggerWrapper.Error("Need an equal count of xs and ys");
                throw new Exception("Need an equal count of xs and ys");
            }
            if (length == 0)
            {
                return null;
            }
            if (length == 1)
            {
                return new double[] {ys[0]};
            }

           // Get consecutive differences and slopes
            var delta = new double[length - 1];
            var m = new double[length];

            for (int i = 0; i < length - 1; i++)
            {
                delta[i] = (ys[i + 1] - ys[i]) / (xs[i + 1] - xs[i]);
                if (i > 0)
                {
                    m[i] = (delta[i - 1] + delta[i]) / 2;
                }
            }
            var toFix = new List<int>();
            for (int i = 1; i < length - 1; i++)
            {
                if ((delta[i] > 0 && delta[i - 1] < 0) || (delta[i] < 0 && delta[i - 1] > 0))
                {
                    toFix.Add(i);
                }
            }
            foreach (var val in toFix)
            {
                m[val] = 0;
            }

            m[0] = delta[0];
            m[length - 1] = delta[length - 2];

            toFix.Clear();
            for (int i = 0; i < length - 1; i++)
            {
                if (delta[i] == 0)
                {
                    toFix.Add(i);
                }
            }
            foreach (var val in toFix)
            {
                m[val] = 0;
                m[val + 1] = 0;
            }

            var alpha = new double[length - 1];
            var beta = new double[length - 1];
            var dist = new double[length - 1];
            var tau = new double[length - 1];
            for (int i = 0; i < length - 1; i++)
            {
                alpha[i] = m[i] / delta[i];
                beta[i] = m[i + 1] / delta[i];
                dist[i] = Math.Pow(alpha[i], 2) + Math.Pow(beta[i], 2);
                tau[i] = 3/Math.Sqrt(dist[i]);
            }

            toFix.Clear();
            for (int i = 0; i < length - 1; i++)
            {
                if (dist[i] > 9)
                {
                    toFix.Add(i);
                }
            }

            foreach (var val in toFix)
            {
                m[val] = tau[val] * alpha[val] * delta[val];
                m[val + 1] = tau[val] * beta[val] * delta[val];
            }

            var y_interp = new double[x_interp.Length];
            int ind = 0;

            foreach (var x in x_interp)
            {
                int i;
                for (i = xs.Length - 2; i >= 0; --i)
                {
                    if (xs[i] <= x)
                    {
                        break;
                    }
                }
                var h = xs[i + 1] - xs[i];
                var t = (x - xs[i])/h;
                var t2 = Math.Pow(t, 2);
                var t3 = Math.Pow(t, 3);
                var h00 = 2*t3 - 3*t2 + 1;
                var h10 = t3 - 2*t2 + t;
                var h01 = -2*t3 + 3*t2;
                var h11 = t3 - t2;
                y_interp[ind++] = h00*ys[i] + h10*h*m[i] + h01*ys[i + 1] + h11*h*m[i + 1];

                continue;
            }

            return y_interp;
        }
    }

Please comment about style, correctness and complexity.

Don't throw Exception

throw new Exception("Need an equal count of xs and ys");

It forces client code to catch any subclass of Exception. In this case I would throw an ArgumentException.

The continue at the end of the last loop is redundant.

Here you're using double.Equals

if (delta[i] == 0)

From MSDN

  The Equals method should be used with caution, because two apparently
  equivalent values can be unequal due to the differing precision of the
  two values.

That link covers two techniques for dealing with this.

As far as I can tell, toFix can be removed. For example,

var toFix = new List<int>();
for (int i = 1; i < length - 1; i++)
{
    if ((delta[i] > 0 && delta[i - 1] < 0) || (delta[i] < 0 && delta[i - 1] > 0))
    {
        toFix.Add(i);
    }
}
foreach (var val in toFix)
{
    m[val] = 0;
}

Can be rewritten as

for (int i = 1; i < length - 1; i++)
{
    if ((delta[i] > 0 && delta[i - 1] < 0) || (delta[i] < 0 && delta[i - 1] > 0))
    {
        m[i] = 0;
    }
}


Linked List simple implementation I made a simple linked list in C with the following functionalities:
Create(creates the list); Find(searches for an element in the list); Insert(inserts a value at the beginning of the list); Destroy(destroys the entire list). It only uses integers (for simplicity), and I want to know for possible errors and how to improve it in general.

The node structure:

typedef struct sllist
{
    int val;
    struct sllist* next;
}
sllnode;

The functions:

#include <stdio.h>
#include <stdlib.h>
#include "node.c"

/*
 *Creates the linked list with a single value
*/
sllnode *create(int value)
{
    sllnode *node;
    node = (sllnode *) malloc(sizeof(sllnode));

    if (node == NULL)
    {
        puts("Memory error");
        return NULL;
    }

    node->val = value;
    node->next = NULL;

    return node;
}

/*
 *Searches for an element in the list.
 *Returns 1 if its found or 0 if not
*/
int find(sllnode *head, int value)
{
    sllnode *trav = head;

    do
    {
        if (trav->val == value)
        {
            return 1;
        }
        else
        {
            trav = trav->next;
        }
    } while (trav != NULL);

    return 0;
}

/*
 *Inserts a value at the beginning of the list
*/
sllnode *insert(sllnode *head, int value)
{
    sllnode *new_node = (sllnode *) malloc(sizeof(sllnode));

    if (new_node == NULL)
    {
        puts("Memory error");
        return NULL;
    }

    new_node->val = value;
    new_node->next = head;
    head = new_node;

    return head;
}

/*
 *Destroys (i.e frees) the whole list
*/
void *destroy(sllnode *head)
{
    sllnode *temp = head;

    while (temp != NULL)
    {
        free(temp);
        temp = temp->next;
    }
    free(head);
}

int main(void)
{
    return 0;
}

Including a C file

Convention is that all shared stuff is declared in .h files.

Minor 1

In destroy(), you have the return type void*. Remove the pointer star and it's fine.

Minor 2

sllnode *create(int value)
{
    sllnode *node;
    node = (sllnode *) malloc(sizeof(sllnode));

    if (node == NULL)
    {
        puts("Memory error");
        return NULL;
    }
    ...

It's a bad idea to abuse standard output from within algorithms/data structures. After all, everything that may go wrong is that there is no memory enough for the new node; returning NULL may well indicate that situation.

Minor 3

node = (sllnode *) malloc(sizeof(sllnode));

More idiomatic C is

node = malloc(sizeof *node);

Minor 4

You "append" the new nodes in reversed direction. Basically this is a (linked) stack and not a list.

Minor 5

while (temp != NULL)
{
    free(temp);
    temp = temp->next;
}

Don't do this. Instead have something like

while (temp)
{
    next_node = temp->next;
    free(temp);
    temp = next_node;
}

Summa summarum

For a starter, I had this in mind:

#include <stdio.h>
#include <stdlib.h>

typedef struct linked_list_node {
    int value;
    struct linked_list_node* next;
} linked_list_node;

typedef struct {
    linked_list_node* head;
    linked_list_node* tail;
} linked_list;

void linked_list_init(linked_list* list)
{
    if (!list)
    {
        return;
    }

    list->head = NULL;
    list->tail = NULL;
}

void linked_list_free(linked_list* list)
{
    linked_list_node* current_node;
    linked_list_node* next_node;

    if (!list)
    {
        return;
    }

    current_node = list->head;

    while (current_node)
    {
        next_node = current_node->next;
        free(current_node);
        current_node = next_node;
    }
}

int linked_list_append(linked_list* list, int value)
{
    linked_list_node* new_node;

    if (!list)
    {
        return 1;
    }

    new_node = malloc(sizeof *new_node);

    if (!new_node)
    {
        return 1;
    }

    new_node->value = value;

    if (list->head)
    {
        list->tail->next = new_node;
    }
    else
    {
        list->head = new_node;
    }

    list->tail = new_node;
    return 0;
}

int linked_list_index_of(linked_list* list, int value)
{
    int index;
    linked_list_node* node;

    if (!list)
    {
        return -2;
    }

    for (node = list->head, index = 0; node; node = node->next, index++)
    {
        if (node->value == value)
        {
            return index;
        }
    }

    return -1;
}

int main() {
    int i;

    linked_list my_list;
    linked_list_init(&my_list);

    for (i = 0; i < 10; ++i)
    {
        linked_list_append(&my_list, i);
    }

    printf("%d %d\n",
           linked_list_index_of(&my_list, 4),
           linked_list_index_of(&my_list, 100));

    linked_list_free(&my_list);
    return 0;
}

Hope that helps.


Drawing a cloud of points I decided that it would be nice to write some code to draw a cloud of points.

Of course my first thought was to draw some random points but the result is not cloud-ish, it is well... random.

In the end I decided to use the Mitchell random points algoritmh generating pretty good results:

Blue = Random
Red = Cloud (Michelin)

from __future__ import division
import matplotlib.pyplot as plt
import random

# The more the nearest the points
RANDOM_POINTS_PER_GENERATION = 6

def random_point():
    return [random.random(), random.random()]

def average(lst):
    return sum(lst) / len(lst)

def distance(a,b):
    return ((a[0]-b[0])**2 + (a[1]-b[1])**2)

def nearest(random_points, seen_points):
    def distance_to_points(point):
        return average([distance(point, seen_point)
                             for seen_point in seen_points])
    return min(random_points, key=distance_to_points)

def next_point(points):
    random_points = [random_point() for _ in range(RANDOM_POINTS_PER_GENERATION)]
    return nearest(random_points, points)

def mitchell(number_of_points):
    """
    Generates almost random points, but at each iteration
    `k` points are generated and the nearest is taken, in
    order to have a better looking points-cloud.
    """
    points = [random_point()]
    while len(points) < number_of_points:
        points.append(next_point(points))
    return points

def plot_points(points, color):
    plt.plot(*zip(*points), color=color, linestyle='none', marker='.',
             markerfacecolor='blue', markersize=4)

if __name__ == "__main__":
    plot_points(mitchell(250), "red")
    plot_points([random_point() for _ in range(250)], "blue")
    plt.show()

mitchell is the only function with a docstring. What do all the other functions do? How do I call them? What do they return?
A function's name should describe what it does, or what it returns, rather than the name of algorithm it uses (which is typically an implementation detail). So I would use a name like point_cloud instead of mitchell.
Your implementation picks the random sample that is closest to all the other points, but the algorithm described in Mitchell's 1991 paper "Spectrally optimal sampling for distribution ray tracing" picks the random sample that is furthest away from any other point (see section 4, "Sequential Poisson-Disk Sampling"). This is so different that I think it would be better not to use the name "Mitchell" to describe the algorithm.
It would make the code easier to use if RANDOM_POINTS_PER_GENERATION were a keyword argument to mitchell, rather than a global variable. Consider a program that tries to determine a good value for this parameter, or a program with two pieces of code that need different values for the parameter.
mitchell(0) returns a list of one point, but surely this should return a list of no points?
Instead of:

while len(points) < number_of_points:

write:

for _ in range(number_of_points - 1):

since you know that each iteration adds exactly one point.
The function average is built into Python under the name statistics.mean.
The function distance is misleadly named: it actually calculates the squared distance between points. Similarly, distance_to_points returns the mean square distance to the points.
The function nearest doesn't need to take the average: since seen_point has the same length in every case, the sum of the squared distances would be just as good.
Instead of creating a temporary list that gets thrown away immediately:

average([distance(point, seen_point) for seen_point in seen_points])

use a generator expression:

sum(distance(point, seen_point) for seen_point in seen_points)

At each stage nearest finds the average squared distance from a candidate point to each point in the cloud, making the overall runtime \$ O(kn^2) \$. However, it's possible to do better than that. For each candidate point \$ q \$, the value that needs to be computed is the sum of squared distances, $$ \sum_j \left|p_j - q\right|^2. $$ Using the dot product we can expand the square, getting $$ \sum_j (p_j - q)·(p_j - q). $$ Multiplying out, we get $$ \sum_j p_j·p_j - 2q·\sum_j p_j + q.q \sum_j 1. $$ Since we taking the minimum of this value over all the candidate points \$ q \$, we don't need the first term \$ \sum_j p_j·p_j \$ as this is the same for all \$ q \$ and so cannot affect the result.

What this means is that if we keep a running sum \$ \sum_j p_j \$ then we can find the candidate point with the minimum sum of squared distances to all the points in the cloud in \$ O(k) \$, bringing the runtime down to \$ O(kn) \$.

To implement this, we need a representation of vectors that supports sums and dot products. Here I'm using NumPy arrays:

import numpy as np

def point_cloud(n, k=6):
    """Generate a point cloud with n points. At each step k (default 6)
    candidate points are generated and the one whose sum of squared
    distances to the other points is smallest is used.

    """
    if n == 0: return
    p = np.random.random(size=2)
    yield p
    sum_p = p.copy()
    for i in range(1, n):
        # Generate k candidate points.
        q = np.random.random(size=(k, 2))

        # Sum of squared distances from each candidate point to all
        # the points generated so far, less the sum of squares of the
        # points generated so far (as this doesn't affect the minimum,
        # we don't need it here).
        s = i * (q * q).sum(axis=1) - 2 * (q * sum_p).sum(axis=1)

        # Pick the candidate that's closest to the points generated so
        # far.
        p = q[np.argmin(s)]
        yield p
        sum_p += p


Classes to encode and decode images I'm a developing a image processor, which reads a Base64 (compressed) image and writes its subimage to targets on threads.

(Constructors, exception handling, and resource management are omitted for brevity)

The base abstract class is AbstractImageProcessor and it implements Runnable to run on threads:

abstract class AbstractImageProcessor implements Runnable {
    protected final String base64; // input
    protected final List<Target> targetList; // list of target information

    protected abstract void read(byte[] data);
    protected abstract void write(Target t);

    @Override
    public void run() {
        byte[] data = decompress(decode()); // decode base64 and decompress
        read(data);
        for (Target t : targetList) {
            write(t)
        }
    }
}

Another abstract class, ImageIOProcessor extends AbstractImageProcessor to use ImageIO:

abstract class ImageIOProcessor extends AbstractImageProcessor {
    private BufferedImage image;

    protected abstract void writeBufferedImage(BufferedImage subimage, Target t)

    @Override
    protected void read(byte[] data) {
        image = ImageIO.read(new ByteArrayInputStream(data));
    }

    @Override
    protected void write(Target t) {
        writeBufferedImage(image.getSubimage(/* x, y, w, h from t */), t);
    }
}

There are two default implementations(concrete classes) of ImageIOProcessor, one writes to file and the other writes to base64 String:

class ImageIOFileWriter extends ImageIOProcessor {
    private File filepath;

    @Override
    protected void writeBufferedImage(BufferedImage subimage, Target t) {
        ImageIO.write(subimage, "png", filepath);
    }
}

class ImageIOBase64Writer extends ImageIOProcessor {
    @Override
    protected void writeBufferedImage(BufferedImage subimage, Target t) {
        ImageIO.write(subimage, "png", /* Base64-encoder-wrapped outputstream*/);
    }
}

The factory interface is responsible for creating appropriate image processing Runnable instance:

public interface ImageProcessorFactory {
    AbstractImageProcessor create(String base64, List<Target> t);
}

Is this the best way to implement them? Or is there better design pattern?

A couple of minor things:

Some people like to always declare what interfaces a class implements, for the sake of clarity. Thus, you could have your subclasses of AbstractImageProcessor also specify implement Runnable.
For consistency, I would rename ImageIOProcessor to AbstractImageIOProcessor.
Not so minor, I would have the method create in ImageProcessorFactory to return an interface, not an abstract class.
While naming is not easy, perhaps you could find a better alternative to ImageIOFileWriter and ImageIOBase64Writer, because they're not only image writers, they also read. They are processors as your other classes.
Do you have any other class extending AbstractImageProcessor besides ImageIOProcessor? If not, maybe you don't need both.


Universal query preparation function I've finally finished my universal query preparation function. Any suggestions on making it better? I feel it could use some tweaking, but am not sure how I would go about it.

Variable examples/explanation

$db - The database connection
$query - Example: $query = "SELECT * FROM users WHEREusername=?"
$operation - The operation you wish to perform (number of rows, insert data)
$type - The variable types in mysqli_stmt_bind_param (example: $type = "ssi")
$variables - The variables to replace the question marks with

function manipulate($db, $query, $operation, $type, $variables)
{
    //Check Number of Variables inputted
    $numVars = (int)strlen($type);
    //Seperate variables into an array
    $var = explode(",",$variables);

    //Handle Statement
    $stmt = mysqli_stmt_init($db);
    if(mysqli_stmt_prepare($stmt, $query)) {
        echo "
 QUERY PREPARED
";
        $x = 0;
        if( $numVars == 1 ) {
            mysqli_stmt_bind_param($stmt, $type, $var[0]);
        } else if ( $numVars == 2 ) {
            mysqli_stmt_bind_param($stmt, $type, $var[0], $var[1]);
        } else if ( $numVars == 3 ) {
            mysqli_stmt_bind_param($stmt, $type, $var[0], $var[1], $var[2]);
        } else if ( $numVars == 4 ) {
            mysqli_stmt_bind_param($stmt, $type, $var[0], $var[1], $var[2], $var[3]);
        } else if ( $numVars == 5 ) {
            mysqli_stmt_bind_param($stmt, $type, $var[0], $var[1], $var[2], $var[3], $var[4]);
        }

        mysqli_stmt_execute($stmt);
        mysqli_stmt_store_result($stmt);

        if($operation == "numRows" || $operation == "num_rows") {
            $output = mysqli_stmt_num_rows($stmt);
        } else {
            echo "Operation not supported";
        }

        mysqli_stmt_close($stmt);
        return $output;

    } else {
        echo mysqli_stmt_error($stmt);
        echo "
 QUERY DENIED
";
    }
}

This is going to sound a bit harsh, but I don't see a purpose to this function.  It offers a tiny bit of a shortcut, but it cripples a lot of functionality to do so.

I'm not very familiar with MySQLi (PDO seems to have a lot more advantages than MySQLi and only a few, insignificant disadvantages -- though PDO does not have a procedural API like MySQLi does), but it seems like the only thing this function ever returns is the number of rows?  So this is a glorified num_rows() function?

debugging output should be removed, and errors should either be returned or thrown as exception.  Directly printing errors is bad since the calling code then doesn't know that an error occurred.
$variables is a blatant anti pattern.  If you want an array, use an array.  What if one of your bindings needs to have a comma in it?
What if numVars is >= 6? Doesn't seem very flexible.

If you decide to keep this function, use call_user_func_array to handle a dynamic amount of parameters.

Supporting numRows and numRows would probably come back to be a pain.  I'd stick with one or the other.

Or, better yet, use constants like MANIPULATE_OP_NUMROWS (like built in functions do).

manipulate is a vague name.  Manipulate what?  And manipulate how?
(int)strlen($type); strlen already returns an int.  (And even if it didn't, you're using loose comparisons anyway.)


Is my logic correct checking for a newer remote file? String slm = status.getHeader("last-modified");
        SimpleDateFormat sdf = new SimpleDateFormat(
                "EEE, dd MMM yyy HH:mm:ss zzz");

        Date serverLastMod = null;
        try {
            serverLastMod = sdf.parse(slm);
        } catch (ParseException e) {
            Log.e("SAC XML last-modified", e.getMessage());
        }
        Date fileLastFetched = status.getTime();

        Log.e("DATE1", serverLastMod.toGMTString());
        Log.e("DATE2", fileLastFetched.toGMTString());

        //if the server date is before my file date, update it, else dont

        if (serverLastMod.after(fileLastFetched)) {
            Log.e("DS", "YES! Getting ROSE");

            XmlDom r = xml.tag("reportrose");
            roseUrl = r.tag("img").attr("src").trim();
            aq.download(roseUrl, file, new AjaxCallback<File>() {

                public void callback(String url, File file, AjaxStatus status) {
                    String s = status.getMessage();
                    int i = status.getCode();
                    String e = s + " | Status Code: " + i;

                    if (file != null) {
                        getInfo();
                    } else {
                        Log.e("ACR SAC Widget Error", e);
                    }
                }

            });
        } else {
            Log.e("DS", "NO! Don't do anything... Wait until next check...");
        }

Well, if it works, it works. There is not much to say about this small part of the code without any context. You should know this by yourself, because you should have some tests.

For the code:

The first catch is not suitable. If it happens, you will get a NullPointerException for the following if.

Avoid abbreviations. There are a lot of bad examples in this code, the most noticeable one is String e = s + " | Status Code: " + i;

Reduce indentation levels:

if (serverLastMod.after(fileLastFetched)) {
...
} else {
    Log.e("DS", "NO! Don't do anything... Wait until next check...");
}

could be:

if (!serverLastMod.after(fileLastFetched)) // or last <= current
{
    log...
    return
}
... rest of code...

and so on.
The part ...rest of code... could probably have its own method update..., so the purpose is clear.


Archiving Tweets via Python I'm fairly new to Python - though not new to programming by any means. I have this Python script and it works perfect for my needs but being new(er) to Python I'm just looking to see if there are any general things I could be doing better or 'more Python-esque'. I recently refactored all of this into what you see now - it used to be one long script. It's now separated out into logical methods. But I know all too well that code can ALWAYS be cleaner. So, any tips or things that could be improved?

Side note: I changed the file paths/Twitter authentication tokens/etc for security purposes but the rest of the code is untouched.

import tweepy
import pytz
import os

idfile = '/pathtofile/tweet_lastID.txt'

def get_tweets():
    '''
    Sets up and returns the twitter api.
    '''
    consumerKey = 'consumerKey'
    consumerKeySecret = 'consumerKeySecret'
    accessToken = 'accessToken'
    accessTokenSecret = 'accessTokenSecret'
    auth = tweepy.OAuthHandler(consumerKey, consumerKeySecret)
    auth.set_access_token(accessToken, accessTokenSecret)
    return tweepy.API(auth).user_timeline('mytwitterhandle', since_id=get_last_id_from_file())

def get_last_id_from_file():
    '''
    Gets the id of the last downloaded tweet from the id file.
    '''
    with open(idfile, 'r') as f:
        return f.readline().rstrip()

def update_last_id(tweets):
    '''
    Updates the id file with the latest id.
    '''
    if not tweets:
        return

    for tweet in reversed(tweets):
        latestID = tweet.id_str

    with open(idfile, 'w') as f:
        f.write(latestID + '\n')

def write_tweets_to_file(tweets):
    '''
    Writes the given tweets to the backup file.
    '''
    if not tweets:
        return

    doc_path = '/pathtofile/tweets_backup.txt'
    with open(doc_path, 'a') as f:
        for tweet in reversed(tweets):
            f.write('\n'.join(tweet).encode('utf8'))

def format_date(date):
    '''
    Formats a given date to the format '%-d %b %Y at %-I:%M%p'.
    '''
    datefmt = '%-d %b %Y at %-I:%M%p'
    homeTZ = pytz.timezone('US/Central')
    utc = pytz.utc

    ts = utc.localize(date).astimezone(homeTZ)
    timeString = ts.strftime(datefmt)
    timeOfDay = timeString[len(timeString)-2:]
    timeString = timeString[:-2]
    return timeString + timeOfDay[:-1].lower()

def make_tweet(tweet):
    '''
    Returns a formatted tweet to be written to the backup file from the given tweet data.
    '''
    return [format_date(tweet.created_at), tweet.text, '----------\n']

if __name__ == '__main__':
    tweets = get_tweets()
    new_tweets = []

    for tweet in tweets:
        new_tweets.extend([make_tweet(tweet)])

    write_tweets_to_file(new_tweets)
    update_last_id(tweets)

The code is more 'Pythonic' than the majority of the codes I have seen here. Are you sure you are just a beginner in python?

One suggestion is to keep consumerKey, consumerKeySecret, accessToken and accessToken secret in another python file, and import these values from that file.
If you do that then, when you need to send this code to somebody or upload it anywhere, you can do it without worrying about making your consumerKey, consumerKeySecret, accessToken and accessToken values public.

Some of your lines are too long. I recommend you to follow PEP 8 standards while writing the code.

Also keep the naming convention uniform throughout the code.If you are using camelcase, then follow camelcase throughout.

I believe that you can do the following task

for tweet in reversed(tweets):
    latestID = tweet.id_str

like this:

lastID = tweet[0]

Also you are appending the tweets into a file and get_last_id() returns the topmost tweet id. I think that is a bug.

A class for each Tweet would have been better. That will make the code more beautiful.

Here is my implementation: github. You can refer it while converting this into a class.
PS: My code is also not perfect. So I also welcome any comments, tips on my code too.


Validation for Model Object using Fluent Validation I am using fluentvalidation to validate my object model. I have to make sure that the first two characters of an ISIN must be letters / non numeric. The way I did it is to convert the string to a char array and then perform regex on the first two characters. I was wondering if anyone has a nicer way of doing the validation using a custom fluent validation. Thanks
    public class Create
    {
        public class Command : IRequest
        {
            public string Name { get; set; }

            public string Exchange { get; set; }

            public string Ticker { get; set; }

            public string Isin { get; set; }

            public string Website { get; set; }
        }

        public class CommandValidator : AbstractValidator<Command>
        {
            public CommandValidator()
            {
                RuleFor(x => x.Name).NotEmpty();
                RuleFor(x => x.Isin).SetValidator(new MyValidator());
            }
        }

        public class MyValidator : PropertyValidator
        {
            public MyValidator(
                string errorMessage = "Isin. The first two characters of an ISIN must be letters / non numeric.") : base(errorMessage)
            {
            }

            protected override bool IsValid(PropertyValidatorContext context)
            {
                var stringToValidate = context.PropertyValue as String;
                return IsValid(stringToValidate);
            }

            public bool IsValid(string stringToValidate)
            {
                var charString = stringToValidate.ToCharArray();
                if (Regex.IsMatch(charString[0].ToString(), "[0-9]") || Regex.IsMatch(charString[1].ToString(), "[0-9]"))
                {
                    return false;
                }

                return true;
            }

            public class Handler : IRequestHandler<Command>
            {
                private readonly DataContext _context;

                public Handler(DataContext context)
                {
                    _context = context;
                }
                public async Task<Unit> Handle(Command request,
                    CancellationToken cancellationToken)
                {
                    var company = new Company
                    {
                        Name = request.Name,
                        Exchange = request.Exchange,
                        Ticker = request.Ticker,
                        Isin = request.Isin,
                        Website = request.Website
                    };

                    await _context.Companies.AddAsync(company);

                    var success = await _context.SaveChangesAsync() > 0;

                    if (success) return Unit.Value;

                    throw new Exception("Problem saving changes");
                }
            }
        }
    }
}

public bool IsValid(string stringToValidate)
{
    var charString = stringToValidate.ToCharArray();
    if (Regex.IsMatch(charString[0].ToString(), "[0-9]") || Regex.IsMatch(charString[1].ToString(), "[0-9]"))
    {
        return false;
    }
    return true;
}

Be aware that each char in a string can be reached by index as stringToValidate[0], so you don't need to call ToCharArray():
public bool IsValid(string stringToValidate)
{
    if (Regex.IsMatch(stringToValidate[0].ToString(), "[0-9]") || Regex.IsMatch(stringToValidate[1].ToString(), "[0-9]"))
    {
        return false;
    }
    return true;
}

But Char has a static method Char.IsLetter() that can be used instead of the Regex expression:
public bool IsValid(string stringToValidate)
{
     return stringToValidate.Length > 1 && Char.IsLetter(stringToValidate[0]) && Char.IsLetter(stringToValidate[1]);
}

This is more consistent with modern unicode character sets, and it will also catch if the two first characters are special chars like '&' or '!' etc. If you just want to ensure they are not numeric, you can use Char.IsDigit() instead.


Bash script to open or close gnome-terminal with a single keybinding I wanted to bind a single hot key to open and close gnome-terminal in a way similar to terminal emulators such as tilda or guake. I use Ubuntu, and have configured this script to be executed when I press the key F1

The script will focus or minimize the first gnome-terminal window that is running.
If no gnome-terminal is running, it will start a new one and place it in the top half of the screen.

#!/bin/bash
# Bind this script to a keyboard shortcut to add "pull-down" behaviour to
# the Gnome terminal similar to guake or tilda

# It seems to work with gnome-terminal or xfce4-terminal
# I haven't tested any other terminal emulators
TERMINAL_EMULATOR=gnome-terminal

function main {
  if [[ -z $(terminal_process) ]]; then
    # no running terminal emulator
    # open new terminal at top of the screen
    $TERMINAL_EMULATOR --geometry 80x1+0+0 &
    win_id=$(get_terminal_window)
    # set initial width to 100% and height to 45%
    xdotool windowsize $win_id $(absolute_window_size 100 45)
    xdotool windowactivate $win_id
  else
    win_id=$(get_terminal_window)
    if [[ $(xdotool getactivewindow) == $win_id ]]; then
      # if terminal window is active, mimize
      xdotool getactivewindow windowminimize
    else
      # if terminal window is inactive, bring in focus
      xdotool windowactivate $win_id
    fi
  fi
}

function terminal_process {
  # Is gnome-terminal already running
  echo $( ps aux | grep $TERMINAL_EMULATOR | grep -v grep)
}

function get_terminal_window {
  # get id of open terminal window. Will wait for window to open if neccessary
  echo $(xdotool search --onlyvisible --sync --limit 1 --class $TERMINAL_EMULATOR)
}

function absolute_window_size {
  # convert relative window size to absolute pixel size, based on screen res
  width_percent=$1
  height_percent=$2
  IFS=x
  screen_dimensions=$(xrandr | grep connected | grep -o '[1-9]\+x[0-9]\+')
  set $screen_dimensions
  echo $(($width_percent*$1/100)) $(($height_percent*$2/100))
}

main

As someone coming from python, bash feels quite unfamiliar, and I would like some feedback on whether this script follows best practices. I also wonder if there's anything that can be simplified without losing readability.

Instead of this:

function terminal_process {
  # Is gnome-terminal already running
  echo $( ps aux | grep $TERMINAL_EMULATOR | grep -v grep)
}

You don't need the echo, you can let the standard output simply go through:

terminal_process() {
  # Is gnome-terminal already running
  ps aux | grep $TERMINAL_EMULATOR | grep -v grep
}

The same goes for get_terminal_window too.

Notice that I also changed the wording style of the function definition. This is the recommended style in bash,
I suggest to apply this to all your functions.

You can actually go even further. [[ -z $(terminal_process) ]] is actually a text comparison, checking if the output of the command as a string is empty. Instead of that, it would be better to use the exit code of the function, like this:

if ! terminal_process; then
    # ...

This works, because the exit of grep will be success (= 0) if it matched something (process exists), and failure (non-zero) if it didn't match.

However, when using the function like this, since the output of grep is not captured within a $(...), it will be printed on stdout, which you probably want to avoid. You can easily suppress that by adding the -q flag:

terminal_process() {
  # Is gnome-terminal already running
  ps aux | grep $TERMINAL_EMULATOR | grep -qv grep
}

This can also be simplified:

echo $(($width_percent*$1/100)) $(($height_percent*$2/100))

Like this:

echo $((width_percent * $1 / 100)) $((height_percent * $2 / 100))


Implement Insertion sort in Python Task : Implement Insertion Sort in Python.

def insert_sort(a):
""" Fuction to sort a given list a """
    if len(a)==0:
        print "Please give valid input"
    for i in range(1, len(a)):
        current = 0
        while current < i:
            if a[i] < a[current]:
                a[current], a[i]= a[i] , a[current]
            current = current + 1
    return a

print insert_sort([12,54,66,43,11,33,34,33,1,45])

How can this code be better?

As this looks like a school assignment, I'm answering with a bunch of questions for the student of algorithms to ponder:

Why do you think an empty list should be invalid input?
Could you think of a more descriptive name for the parameter than a?
You have a loop from 1 to length of array. This looks like an off by one error. Normally one would loop over indexes of 0..length-1. Could this loop be improved so it doesn't look suspicious?
Why are you using an outer for-loop and an inner while loop? Why not not in reverse? Why not use the same looping construct for both?
As this is an insertions sort, how do you perform an insertion to Python list, and where in this code is that insertion used?
How can you make sure that this solution really works? Could you write an automated test to check that it's correctly sorting various inputs?


Division number and remainder php code I created a function that will tell me how many times a number is divisible by another number and what the remainder of it is:

 <?php
function divisible_times($number, $divider) {
    $count = 0;
    $remainder = $number % $divider;
    $num_array = range($number, 1);
    foreach($num_array as $num) {
        if($num % $divider == 0){
            $count++;
        }
    }

    $arr[$number.':'.$divider]['divisions_no'] = $count;
    $arr[$number.':'.$divider]['remainder'] = $remainder;
    return $arr;
}
print_r(divisible_times(4, 3)); // Returns Array ( [4:3] => Array ( [divisions_no] => 1 [remainder] => 1 ) )
print_r(divisible_times(14, 3)); // Returns Array ( [14:3] => Array ( [divisions_no] => 4 [remainder] => 2 ) )
print_r(divisible_times(13, 3)); // Returns Array ( [13:3] => Array ( [divisions_no] => 4 [remainder] => 1 ) )
print_r(divisible_times(12, 3)); // Returns Array ( [12:3] => Array ( [divisions_no] => 4 [remainder] => 0 ) )

Can this code be optimized somehow? I tried to use $number directly, but couldn't find out the count that way. The only thing I could think is to create am array of numbers and go through each one.

I mean this does the trick, but optimization is important, and I was wondering if there is any other way of doing it?

Try the following:

function divisible_times($number, $divider) {
  $arr[$number.':'.$divider]['divisions_no'] = floor($number / $divider);
  $arr[$number.':'.$divider]['remainder'] = $number % $divider;
  return $arr;
}


Codewars Challenge Kata I was recently doing a challenge on Codewars and this is what challenge said:

  There are no explanations. You have to create the code that gives the
  following results:

  oneTwoThree(0) → ['0', '0']
    oneTwoThree(1) → ['1', '1']
  oneTwoThree(3) → ['3', '111']
  oneTwoThree(19) → ['991', '1111111111111111111']

I have solved the challenge but it appears to be quite slow and I was hoping to get some constructive ideas on how to improve the speed and efficacy of this code thanks.

function oneTwoThree(n) {
  var finalOutput = ['', ''],
    slotsArray = [],
    sum = 0,
    currentSlot = 0,
    slots = 1;

  /*
      My Idea was to create an array and the sum of that array will be equal to the parameter given "n". The array will have slots and the number of slots is unknown because the parameter "n" is arbitrary. each slot will contain a number from 9-1, we will start out with 9 because it is the highest, I will add all the slots and the sum must be equal to parameter "n" thus will give us the first item in our final array.
  */
  if (n >= 10) {
    while (!(9 * slots >= n)) { // check how many slots are needed
      slots += 1;
    }
    while (slotsArray.length != slots) {
      // place a 9 in every slot, starting with the highest numbern this should give us something like "[9,9,9]" for examply
      slotsArray.push(9);
    }

    // this is how I will keep track of what slot I am currently in and we will start from right to left.
    currentSlot = slotsArray.length - 1;

    for (var i = 0; i < slotsArray.length; i++) {
      //here I test if the sum of the array is equal to "n"
      sum += slotsArray[i];
    }

    // if it is not equal we proceed here.
    while (sum != n) {
      slotsArray[currentSlot] -= 1; // subtract 1 from the current slot example: [9,9,8]

      // I check agian if the sum is equal to n
      for (var i = 0; i < slotsArray.length; i++) {
        sum += slotsArray[i];
      }

      // reset the sum to 0 if its not equal
      if (sum != n) {
        sum = 0
      }

      // This will continue until if finds the sum that equals "n"
    }

    // convert the array into a string for the final output
    for (var i = 0; i < slotsArray.length; i++) {
      finalOutput[0] += String(slotsArray[i]);
    }

    while (finalOutput[1].length != n) {
      finalOutput[1] += '1';
    }
  }

  // if the parameter "n" is less than 10
  if (n < 10) {
    finalOutput[0] = String(n);
    if (n == 0) {
      finalOutput[1] += String(n);
    } else {
      while (finalOutput[1].length != n) {
        finalOutput[1] += '1';
      }
    }
  }
  console.log(finalOutput);
  return finalOutput;
}
oneTwoThree(19);
oneTwoThree(1);
oneTwoThree(7);
oneTwoThree(36);
oneTwoThree(0);

Do you need to iterate?

There is a lot of room for improvement..

Use one loop if possible.

You have many loops where you iterate the same number of times. Rather than doing each loop in turn do as much as possible in one loop.

Eg.

while (!(9 * slots >= n)) {
  slots += 1;
}
while (slotsArray.length != slots) {
  slotsArray.push(9);
}

Can be done in one

// Note new solution does not require extra item
while (!(9 * slots >= n)) {
    slots += 1;
    slotsArray.push(9);
}

Simple conditions

Make the condition statement as simple as possible. You had !(9*slots >= n) the additional not clause is extra work (if not optimised) 9*slots < n is the same

Use variables

Some javascript engines (V8) are not as quick handling literal numbers. eg x *= 10 is slower than x *= ten but you must take into account the declaration and assignment of the variable. If you use the number once or twice there is no advantage, but if you do it many time there is.

Math rather than Loops

Alway check to see if there is a way to directly calculate the result rather than use iteration.

You do not actually need any loops

Use memory

Memory is cheap and fast, CPU cycles are not. So rather than iterate have a predefined object that can be used to create a result

You have

  while (finalOutput[1].length != n) {
    finalOutput[1] += '1';
  }

but the following is quicker (note it should be defined outside the function but in this case you may only have the function's scope)

  // long enough for all possible results?
  var ones = "111111111111111111111111111111111111111111111111111111111";
  finalOutput[1] = ones.substr(0,n);

There are many other optimisations that I have not included as the resulting optimised code is very simple and most of what you had is not needed.

Testing

To ensure I got it correct I wrote a test that compares your function to the new one. Making sure the results are the same, then running each version 100,000 times to get the performance.

There results are

// calls to function oneTwoThree Ori is the original and New is the optimised version
// time in ms (1/1000);

-- Test value : (19) -------------------
Ori : 191.53ms Mean : 0.00192
New : 22.38ms Mean : 0.00022
-- Test value : (1) -------------------
Ori : 8.78ms Mean : 0.00009
New : 6.61ms Mean : 0.00007
-- Test value : (7) -------------------
Ori : 62.03ms Mean : 0.00062
New : 6.02ms Mean : 0.00006
-- Test value : (36) -------------------
Ori : 208.12ms Mean : 0.00208
New : 18.57ms Mean : 0.00019
-- Test value : (0) -------------------
Ori : 8.47ms Mean : 0.00008
New : 4.00ms Mean : 0.00004

For the results of n < 10 the improvement is not that great but near 25%-50% is worth the effort. The improvement for n >= 10 are well worth the effort.

The new code

No loops needed. Warning longStr1s may need to be larger as I have not tested all possible inputs.

function oneTwoThree_Q(n) {
    var nine = 9; // variable = 9 is quicker than litteral value
    var slots;
    var longStr1s = "11111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111";
    var longStr9s = "999999999999999999999999999999999999999999999999999999999";
    if (n < 10) {
        return ['' + n, n === 0 ? '' + n : longStr1s.substr(0,n)];
    }
    slots = Math.ceil(n / nine);
    nine += n - slots * 9;
    return [longStr9s.substr(0,slots-1) + nine, longStr1s.substr(0,n)];
}