A concurrent queue that supports exactly two threads and not more. One thread is allowed to push, the other to pop My header file implementation:

#ifndef CIRCULARSPSC_H
#define CIRCULARSPSC_H

#include <atomic>
#include <cstddef>

template<typename Data, size_t Size>
class CircularSpsc
{
public:
    enum { queue_size = Size + 1 };

    CircularSpsc() : back(0), front(0){}
    virtual ~CircularSpsc() {}

    bool push_back(const Data& item)    ;
    bool pop_front(Data& item);

    bool isEmpty() const;
    bool isFull() const;

private:
    size_t increment(size_t idx) const;
    std::atomic<size_t> back;
    Data dataArray[queue_size];
    std::atomic<size_t> front;
};

#include "circularspsc.cpp"

#endif

My source file implementation:

#ifndef CIRCULARSPSC_CPP
#define CIRCULARSPSC_CPP

#include "circularspsc.hpp"

template<typename Data, size_t Size>
bool CircularSpsc<Data, Size>::push_back(const Data& item)
{
    const auto current_back = back.load(std::memory_order_relaxed);

    const auto next_back = increment(current_back);
    if(next_back != front.load(std::memory_order_acquire))
    {
        dataArray[current_back] = item;
        back.store(next_back, std::memory_order_release);
        return true;
    }
    return false;
}

template<typename Data, size_t Size>
bool CircularSpsc<Data, Size>::pop_front(Data& item)
{
    const auto currentfront = front.load(std::memory_order_relaxed);
    if(currentfront == back.load(std::memory_order_acquire))
        return false;

    item = dataArray[currentfront];
    front.store(increment(currentfront), std::memory_order_release);
    return true;
}

template<typename Data, size_t Size>
bool CircularSpsc<Data, Size>::isEmpty() const
{
    return (front.load() == back.load());
}

template<typename Data, size_t Size>
bool CircularSpsc<Data, Size>::isFull() const
{
    const auto nextback = increment(back.load());
    return (nextback == front.load());
}

template<typename Data, size_t Size>
size_t CircularSpsc<Data, Size>::increment(size_t idx) const
{
    return (idx + 1) % queue_size;
}

#endif

Client file implementation:

#include <iostream>
#include <thread>
#include <vector>
#include "circularspsc.hpp"
#include "dataobject.hpp"

#define QUEUESIZE 10000

void reader(CircularSpsc<int, QUEUESIZE>& cspsc_q)
{
    for(int i = 0; i < QUEUESIZE; i++)
    {
        int dataItem;
        cspsc_q.pop_front(dataItem);
        std::cout << "popped value: " << dataItem << std::endl;
    }
}

void writer(CircularSpsc<int, QUEUESIZE>& cspsc_q)
{
    for(int i = 0; i < QUEUESIZE; i++)
    {
        cspsc_q.push_back(i);
    }
}

int main()
{
    CircularSpsc<int, QUEUESIZE> cspsc_q;

    std::thread writer_thread(writer, std::ref(cspsc_q));
    std::thread reader_thread(reader, std::ref(cspsc_q));

    reader_thread.join();
    writer_thread.join();
}

This has the clear drawback that we can only create queues of very limited size (because with larger values of QUEUESIZE I am getting segfault (surely because the stack size is limited?)). In my next iteration I plan to implement a version with dynamic memory allocation inside the constructor of the queue for QUEUESIZE number of objects and then use placement new to move values inside the queue.

Naming

I don't understand what CircularSpsc is supposed to mean, and how to even pronounce it (circular space?). I suggest you use a full name without omitting the vowels.

Include guards

You were right to add an include guard to the header file (.hpp). It is supposed to prevent the header file to be included more than once in a single translation unit, which could cause "multiple definition" errors.

However, in the implementation file (.cpp) an include guard is not needed, because this file is not supposed to be included at all.

The implementation file is in fact included at the end of the header file, but this is not the right way to get it compiled. Instead, you should pass its name directly to the compiler in addition to the "main" .cpp file.

You can read more about how compilation works in C++ here, for example.


Check for null/empty in dictionary I am doing the following about 10 times with other strings, so the code is duplicated.  How can I refactor this?

queryWhere is a Dictionary<string,string> that contains parameters that will be passed to a query.

string account = string.Empty;

if (queryWhere.ContainsKey("account")
&& queryWhere["account"] != null
&& !string.IsNullOrEmpty(queryWhere["account"].ToString()))
    account = queryWhere["account"].ToString();

string customer = string.Empty;

if (queryWhere.ContainsKey("customer ")
&& queryWhere["customer "] != null
&& !string.IsNullOrEmpty(queryWhere["customer "].ToString()))
    customer = queryWhere["customer "].ToString();

string balance = string.Empty;

if (queryWhere.ContainsKey("balance ")
&& queryWhere["balance "] != null
&& !string.IsNullOrEmpty(queryWhere["balance "].ToString()))
    balance = queryWhere["balance"].ToString();

Each of these:

string account = string.Empty;

if (queryWhere.ContainsKey("account")
   && queryWhere["account"] != null
   && !string.IsNullOrEmpty(queryWhere["account"].ToString()))
{
    account = queryWhere["account"].ToString();
}

Should be reduced to:

string account;
if(!queryWhere.TryGetValue("account", out account))
{
   account = string.Empty; // if you need the string to be empty // default is null
}

Dictionary.TryGetValue

But that logic can then be moved to a method:

private string GetValue(string key)
{
   string returnValue;
   if(!queryWhere.TryGetValue(key, out returnValue))
   {
      returnValue= string.Empty;
   }
   return returnValue;
}

string account = GetValue("account");
string customer = GetValue("customer");

I really don't see the point of your original code, BTW. For instance, the .ToString() is completely superfluous, since you're working with a Dictionary<string,string>. It is always going to return a string.

But why do you even check for string.IsNullOrEmpty()? You already know it isn't null from the previous line's check -- queryWhere["account"] != null -- so at worst it is empty which is your default value anyway!


Displaying a leader board from a text file and calculating points The text file is laid out like this: first name, last name, wins, losses

dex,kop,5,6

desder,jok,0,11

aslan,vert,11,0

jax,sas,2,9

buy,guy,1,10

Code:

def points():

    template = "|{0:<30}|{1:<30}|{2:<30}|{3:<30}|{4:<30}"
    lol = template.format("1st name: ","2nd Name: ", "won: ", "lost: ","points: ")

    print(lol)

    with open(r'prac2.txt', 'r') as file:

        for line in file:

            data = line.strip().split(',')

            if int(data[2]) >= 1:

                poin = int(data[2]) * 3

                data.append(poin)

                print(template.format(data[0], data[1], data[2], data[3], data[4]),'\n')

        file.close()

points()

The program should only display players that have won at least 1 game and calculate their points, a win is equal to 3 points.

Any suggestions as to how to improve the code any would be great.

Since you are using all the elements of data,  You can turn:

print(template.format(data[0], data[1], data[2], data[3], data[4]))

into this:

print(template.format(*data))

See here for some details.


Loading a file in vim via piped output of ag I feel as if my scenario is fairly simple but that I might be overcomplicating it.

I am searching through files looking for a special condition, that is where the method .all with arguments exists.  In the real world that looks like:

ba_mbn_programs = Program.ba_and_managed_bar.all(:conditions => "user_programs.user_id = #{current_user.id} and brand_id = #{brand.id}", :order => :name, :joins => [:user_programs], :select => "distinct programs.*")

I'm able to find those using ag like this:

ag '\.all\(.*\)'

That gives me a big list of file names and line numbers (here's a snippet):

lib/ui_elements.rb
4:      self.active.all(:select => ["name, #{model}.id"], :order => :name).collect{|m| [m.name, m.id]}.insert(0, ["All", nil])

So in no particular order, I need to address these lines in my editor.  So what I really want to do is open up the file lib/ui_elements.rb in vim and go right to line 4.

I've learned that using vim +line_num file_name is a way to do that.  However just isolating the line number and file name from the ag output has forced me to do something like this:

vim `ag '\.where\(.*\)' app | tail -1 | sed -E 's/([^:]*):([0-9]*):.*/\1 \2/' | awk -v q="+" '{print q $2, $1}'`

So that just reads the last file from that search and opens the file in vim to that line number.  Surely there is a better way then this?

You could simplify the vim call like this:

vim $(ag '\.where\(.*\)' app | sed -nE '$s/([^:]*):([0-9]*):.*/+\2 \1/p')

That is, no need for the awk to print the matched filename and line number in reverse, you can reverse in the sed, and also prefix the number with + there.

I dropped the tail -1, by using the -n flag of sed to not print output by default, added $ in front of the s/// command to apply for only the last line of input, and the /p flag to print after substitution.
Although this has the benefit of eliminating a tail process in the middle, it has the disadvantage that it's a bit harder to understand. So you can put back tail -n 1 (more portable than tail -1) if you prefer.

I also changed the obsolete `...` to $(...).


Finding sum of divisors of numbers between the range (1 to 1e16) The exact link to the question : https://www.spoj.com/problems/DIVSUM2/

Input: An integer stating the number of test cases (equal to 500), and that  many lines follow, each containing one integer between \$1\$
and \$10^{16}\$ inclusive.
Output: Print one integer, the sum of the proper divisors of the number.

An easier version of this question exists in which the constraints are relaxed.
My code works for the easier version , but exceeds the time limit for the harder one.
Example of approach: \$36=2^2 \times 3^2\$. So sum of proper factors = $$(2^0+2^1+2^2)\times(3^0+3^1+3^2)-36=55$$
My code:
#include<iostream>
long long sum_proper_divisors(long long n)
{
    long long sum=1,x=n;
    for(long long i=2;i*i<=n;i++)
      {
          long long current_sum=1,current_term=1;
           while(n%i==0)
           {
              n/=i;
              current_term*=i;
              current_sum+=current_term;
            }
         sum*=current_sum;
      }
   if(n>=2){
    sum*=(n+1);
    }
  return (sum-x);
}
int main()
{
   long long q;
   std::cin>>q;
    while(q--)
    {
      long long n;
      std::cin>>n;
      std::cout<<sum_proper_divisors(n)<<std::endl;
    }
 }

What's going on with the indentation?

  long long

No. We're not in the 1970s. Use <cstdint>. Here you probably want std::uint_fast64_t.

IMO n should be const and the copy (x) should be the variable which is modified in the loop.

    for(long long i=2;i*i<=n;i++)

The first step to optimising prime divisor searches is to use a wheel. If you special-case i=2 then you can search for(i=3;i*i<=x;i+=2), which is already a speedup by about a factor of 2.

On some architectures, multiplication is expensive and it's faster to maintain a variable ii or iSquared which is updated using the identity \$(i+a)^2 - i^2 = 2ai + a^2\$: e.g. (untested, may be buggy): for(i=3,ii=9;ii<=x;ii+=(i+1)<<2,i+=2)

          long long current_sum=1,current_term=1;
           while(n%i==0)
           {
              n/=i;
              current_term*=i;
              current_sum+=current_term;
            }

This can be simplified:

           std::uint_fast64_t current_sum=1;
           while(x%i==0)
           {
              x/=i;
              current_sum=current_sum*i+1;
           }

The first big step to improving the algorithm would be to exploit the fact that up to 500 test cases are passed at a time. If you read them all and find the largest, you can do a variant of Eratosthenes' sieve once to set up an array which allows you to factor all numbers up to that largest one very fast.

If that's not enough, it's probably necessary to switch factorisation approach, starting with Pollard's rho algorithm.


C++ sum and averaging functions for arrays and argument sets Here are two sets of variadic C++ functions to do fast summing and averaging of arrays (plain & std::array) or argument sets, (re)written as part of my generic toolbox.

The sum functions are pretty generic, the averaging ones operate on floating point based types only (the integer counterparts are application specific).

Usage example:

std::array<volatile float, 5> arr1 { 1.1f, 1.2f, 1.3f, 1.4f, 1.5f };
volatile float arr2[] { 1.1f, 1.2f, 1.3f, 1.4f, 1.5f };

volatile float res1 = sum(arr1);
volatile float res2 = avg(arr2, sizeof(arr2)/sizeof(arr2[0]));
volatile auto res3 = avg(1.2f, 1.4f, 1.6);

Live demo at: https://godbolt.org/g/DMHv7Y

The sum functions:

#include <cstdint>
#include <utility>
#include <type_traits>
#include <array>

/**
 * Return the sum of all values in the provided `std::array`.
 * @param  arr std::array
 * @return     sum
 */
template <typename T, size_t S>
inline constexpr auto sum(const std::array<T, S> arr) {
    std::remove_cv_t<std::remove_reference_t<T>> t = 0;
    for (const auto v : arr) {
        t += v;
    }
    return t;
}

/**
 * Return the sum of all values in the provided plain array.
 * @param  arr Array
 * @param  l   length
 * @return     sum
 */
template <typename T, typename = typename std::enable_if<std::is_pointer<T>::value, void>::type>
inline constexpr auto sum(const T arr, size_t l) {
    std::remove_cv_t<
        std::remove_reference_t<
            decltype(arr[0])>> t = 0;
    for (size_t i = 0; i < l; i++) {
        t += arr[i];
    }
    return t;
}

/**
 * Return the sum of all values. Tries to use the common type for the return value.
 * @param  ts Arguments to sum
 * @return    sum
 */
template <typename... Ts,
          typename = typename std::enable_if<!std::is_pointer<std::common_type_t<Ts...>>::value, void>::type>
inline constexpr auto sum(Ts&&... ts) {
    std::common_type_t<Ts...> t = 0;
    for (const auto val : std::initializer_list<std::common_type_t<Ts...>>{ts...}) {
        t += val;
    }
    return t;
}

The averaging functions:

#include <cstdint>
#include <utility>
#include <type_traits>
#include <array>

/**
 * Return the average of all values in the provided `std::array`.
 * @param  arr std::array
 * @return     average
 */
template <typename T, size_t S, typename = typename std::enable_if<std::is_floating_point<T>::value, void>::type>
inline constexpr auto avg(const std::array<T, S> arr) {
    std::remove_cv_t<std::remove_reference_t<T>> t = 0;
    for (const auto v : arr) {
        t += v;
    }
    return t / S;
}

/**
 * Return the average of all values in the provided array.
 * @param  arr array
 * @param  l   length
 * @return     average
 */
template <typename T, typename = typename std::enable_if<std::is_pointer<T>::value && std::is_floating_point<std::remove_pointer_t<T>>::value, void>::type>
inline constexpr auto avg(const T arr, size_t l) {
    std::remove_cv_t<
        std::remove_reference_t<
            decltype(arr[0])>> t = 0;
    for (size_t i = 0; i < l; i++) {
        t += arr[i];
    }
    return t / l;
}

/**
 * Return the average of all arguments. Tries to use the common type for the return value.
 * @param  ts Arguments to sum
 * @return    average
 */
template <typename... Ts,
          typename = typename std::enable_if<!std::is_pointer<std::common_type_t<Ts...>>::value && std::is_floating_point<std::common_type_t<Ts...>>::value, void>::type>
inline constexpr auto avg(Ts&&... ts) {
    std::common_type_t<Ts...> t = 0;
    for (const auto val : std::initializer_list<std::common_type_t<Ts...>>{ts...}) {
        t += val;
    }
    return t / sizeof...(Ts);
}

The generic patterns here can of course be used to create all kinds of data processing tools. I'd love to know if/how I can improve them.

Different approach:

I'd use iterators and standard library. Using them it becomes really simple:

template <typename ForwardIt, typename T = typename std::iterator_traits<ForwardIt>::value_type>
T average(ForwardIt first, ForwardIt last, T initvalue = {}) //will be 0 for builtins
{
    auto distance = std::distance(first, last);
    auto result = std::accumulate(first, last, initvalue);

    return result / distance;
}

It is also possible to have a distance argument that will be defaulted to what std::distance() produces, but since my use cases are mostly bound to RandomAccessIterators, I don't care, since distance is constant time on random access iterators.

It can be noticed that iterators abstract container type, so they become really easy to use.

The code actually is part of my library. It is so ancient that I forgot it even exists.

As for sum, it is simply using std::accumulate().

Some more tips:

Standard library has type traits. They help to debug templates. The toolbox should try to provide them as well.

template <typename T>
struct is_floating_point_array :
                public std::false_type
{};

template <typename T, std::size_t size>
struct is_floating_point_array<T[size]> :
                public std::bool_constant<std::is_floating_point_v<T>>
{};

//one for std::array

Ironically, std::is_array<> won't work on instantiations of std::array, so I recommend writing your own. The above will not only provide easier debugging experience, but will also allow tagged dispatch or if constexpr opportunities. Even more, it will provide concept support, which will make metaprogramming even easier. I didn't directly inherit from std::bool_constant on the first phase since I favored readability to less code height, maybe I was wrong.

Iterators are essential part of standard library. I recommend using them at least in obvious cases. Currently using the code one needs to use std::size() or write one themselves. Pretty much everyone I know doesn't know about std::size(). std::begin() and std::end() (or their member counterparts) destroy opportunity for making the mistakes.


Concatenating CSV filenames into a single file I've created a PHP script which take an array of filenames as a comma-separated parameter and concatenates them, serving them as one, single JavaScript file.

I've also built a caching mechanism into it - each time, it checks if any of the individual JS files have been updated and, if one or more of them has, it updates the cache and serves the new output. Otherwise, it simply loads the relevant cached file.

<?php
    $scripts_to_concatenate = $_GET['scripts'];
    $scripts_array = explode(',', $scripts_to_concatenate);
    $cached_script = '/public_html/assets/scripts/cache/' . str_replace('.js', '', str_replace(',', '_', $scripts_to_concatenate)) . '.js.cache';
    $cache_expired = false;
    if (file_exists($cached_script)) {
        foreach ($scripts_array as $script) {
                if (filemtime($script) > filemtime($cached_script)) {
                    $cache_expired = true;
                }
        }
    } else {
        $cache_expired = true;
    }
    if ($cache_expired) {
        foreach ($scripts_array as $script) {
            $script = '/public_html/assets/scripts/' . $script;
            if (file_exists($script)) {
                $output = $output . @file_get_contents($script);
            }
        }
        file_put_contents($cached_script, $output);
    } else {
        $output = @file_get_contents($cached_script);
    }
    echo $output;

Is anyone able to suggest any ways for me to improve this code, to make it more efficient or more fool-proof?

UPDATE: I've incorporated janos's recommendations into my code (original, unoptimised code above). Does anyone have any other suggestions?

As soon as you find an outdated file,
you can break out from the first loop:

    foreach ($scripts_array as $script) {
            if (filemtime($script) > filemtime($cached_script)) {
                $cache_expired = true;
                break;
            }
    }

And instead of accumulating the script content in a variable,
you can write to a file and echo directly:

if ($cache_expired) {
    unlink($cached_script);
    foreach ($scripts_array as $script) {
        $script = '/public_html/assets/scripts/' . $script;
        if (file_exists($script)) {
            $content = @file_get_contents($script);
            file_put_contents($cached_script, $content, FILE_APPEND);
            echo $content;
        }
    }
} else {
    echo @file_get_contents($cached_script);
}


Two solutions to 2-sum I have written a solution for two sum problem in variation. first is simple with two loops and another one is one loop and hash map.

But I wonder why the simple solution is faster than the efficent solution

simple solution -

func TwoSum(arr []int, s int) [][]int {
    var sum [][]int
    for i := 0; i < len(arr); i++ {
        for j := i + 1; j < len(arr); j++ {
            if arr[i]+arr[j] == s {
                sum = append(sum, []int{arr[i], arr[j]})
            }
        }
    }
    return sum
}

Hashmap solution -

func TwoSum1(nums []int, target int) [][]int {
    var r = make([][]int, 0)
    var m = make(map[int]int, 0)
    for p, i := range nums {
        j := target - i
        if pos, ok := m[j]; ok {
            if pos != p {
                r = append(r, []int{j, i})
            }
        } else {
            m[i] = p
        }
    }
    return r
}

also, I have written Benchmark

type args struct {
    arr []int
    s   int
}

var tests = []struct {
    name string
    args args
    want [][]int
}{
    {
        "One",
        args{[]int{3, 5, 2, -4, 8, 11},
            7,},
        [][]int{{5, 2}, {-4, 11}},
    },
}

func BenchmarkTwoSum(b *testing.B) {
    for i := 0; i < b.N; i++ {
        for _, tc := range tests{
            TwoSum(tc.args.arr, tc.args.s)
        }
    }
}

  BenchmarkTwoSum-4      3000000           382 ns/op

func BenchmarkTwoSum1(b *testing.B) {
        for i := 0; i < b.N; i++ {
            for _, tc := range tests{
                TwoSum1(tc.args.arr, tc.args.s)
            }
        }
    }

  BenchmarkTwoSum1-4     2000000           561 ns/op

When you ask if your first solution is faster, if you are talking about theory it isn't faster. To understand why you need to understand the the difference between big O complexity and actual run time. For the purposes of this question, it is important only that you realize that big O complexity does not guarantee that a piece of code with O(n^2) run time will run faster than O(n) for all inputs merely that for some very large inputs O(n^2) will be faster. More exactly, Big O complexity merely talks about the growth rate of a function at really large values. Without getting into it too much you can imagine that the if we have x^2 and 1000000x, I can guarantee that at some input x^2 will surpass 1000000x. Similarly, if I have x^2 + x I can guarantee that at some  point the x^2 term will be so much bigger than the x term that the any increase in in our input will essentially just increase the function by approximately x^2.  If that doesn't make sense, then I would suggest brushing up on big O here https://medium.com/karuna-sehgal/a-simplified-explanation-of-the-big-o-notation-82523585e835 .

Now for your problem, the big O complexity of the hash map based solution is O(n) the big O for the brute force double loop solution is O(n^2). Now as mentioned about big O does not guarantee that an O(n^2) piece of code will always run faster that an O(n) piece of code for all inputs. Big O is a mathematical guarantee only for large inputs. All big O says is that for some large input O(n^2) will be faster O(n). Let me give you a specific example of why despite a greater big O the double loop solution might be faster for a constrained input. let's say that your two loop solution runs in n^2 time exactly and your hash map solution runs in 1000000000*n time.  Well technically your hash map solution would benchmark much much slower for reasonable sized inputs, but eventually if the input were big enough then your hash map solution would run faster.  Now, maybe you only care about the performance for a constrained sample size. That's great. This is why you will always have to run a benchmark to compare solutions. Big O is not meant as a tool to guarantee that a certain piece of code will run faster for all inputs, only as a tool to theoretically reason about performance for extremely large inputs.  Knowing the time complexity in O notation, won't give you any sort of idea what might be faster for a given constrained input size.  That's why in the real world, understanding big O is important, but it is also very important to run performance benchmarks based on how big you actually expect your input to get.  I've repeated this central theme in different ways throughout this post but I will summarize again here Big O only guarantees run time for some extremely large input (think as the limit of your input goes to infinity) it says absolutely nothing about some constrained input.  I could reasonably come up with a Big O problem where an O(n^2) piece of code would never run faster than an O(n) piece of code given the constrained of the amount of memory on your computer. Basically, use big O to get a general idea of run time when writing your code, and then if you see some performance issues benchmark to find improvements.

Now in the case of this question, someone with a little more understanding of what's actually happening in Go might be able to tell you exactly what operation is making your hash map solution slower for those inputs. My guess... when you call make(map[int]int, 0) you are allocating space on the heap, which is likely taking a good amount of time.


Pull out text within specific divs on a webpage I am looking for some guidance and feedback on my first golang program. The language is very new to me and its been a long while since I have written anything close to C.

Below is a simple program that takes a url and looks for divs within the page containing a specific class. Once found I read the text within the div outputting it to the terminal.

I am looking for general feedback especially on my use of pointers and slices, the way I am getting the text out of the div (in a loop with a bool variable) which seems a bit hacky to me

package main

import (
    "fmt"
    "net/http"
    "sort"

    "golang.org/x/net/html"
)

// Helper function to identify if the token is an image div
func isImageDiv(token html.Token) (ok bool) {
    // Iterate over all of the Token's attributes until we find "class"
    for _, a := range token.Attr {
        if a.Key == "class" {
            // The image labels are surrounded by div with the class "project-title"
            if a.Val == "project-title" {
                ok = true
                return
            }
        }
    }

    // "bare" return will return the variables (ok) as defined in
    // the function definition
    return
}

func getLabel(z *html.Tokenizer) (label string) {
    return string(z.Text())
}

func crawl(url string, foundLabels *[]string) {
    resp, err := http.Get(url)

    if err != nil {
        fmt.Println("ERROR: Failed to crawl \"" + url + "\"")
        return
    }

    b := resp.Body
    defer b.Close() // close Body when the function returns

    z := html.NewTokenizer(b)

    readText := false
    for {
        tt := z.Next()

        switch {
        case tt == html.ErrorToken:
            // End of the document, we're done
            return
        case tt == html.StartTagToken:
            t := z.Token()

            // Check if the token is an <div> tag
            isDiv := t.Data == "div"
            if !isDiv {
                continue
            }

            // Verify that is has the class we are looking for, if there is one
            ok := isImageDiv(t)
            if !ok {
                continue
            }
            readText = true
        case tt == html.TextToken && readText == true:
            *foundLabels = append(*foundLabels, getLabel(z))
            readText = false
        }
    }
}

func main() {

    url := ""

    foundLabels := make([]string, 0)

    crawl(url, &foundLabels)

    sort.Strings(foundLabels)

    for _, element := range foundLabels {
        // element is the element from someSlice for where we are
        fmt.Println(element)
    }
}

First of all: how big is this document? If you're not expecting it to be big, you may want to use html.Parse instead of a tokenizer - that way, you'll get the whole document in a tree and would be able to iterate over children of your div when found, which should make the whole thing simpler.

Your current approach is streaming, which conserves memory, but you are making the assumption that the div with the class project-title will always contain just text. What if the document looks like this:

not title
title

or this

wrong title

In the first case, "not title" would be added. In the second, "wrong title" would be added.

Assuming you want to keep your logic as-is, let's move on the review!

A few general tips:

Always put string literals on top as constants (e.g. const className = "project-title").
You don't need to name your return values unless you see a need.
While Go values brevity, z is (almost) never a good variable name.
Most of the comments don't add any value (e.g. "Check if the token is an
 tag").

Some specific things:

In isImageDiv, you don't need a separate if block. You also don't need to name the return value, which actually simplifies your code in this case. Finally, you should also check if the element is a div - that way, you needn't do it in crawl:

func isImageDiv(token html.Token) bool {
    if token.Data != "div" {
        return false
    }
    for _, a := range token.Attr {
        if a.Key == "class" && a.Val == "project-title" {
            return true
        }
    }
    return false
}

getLabel seems too simple to have a need to exist.
In crawl, you shouldn't use fmt.Println. Instead, return an error, the same way http.Get returns it. Let the caller handle it.
In crawl, you can simplify the switch a lot - continue is not really needed and we made our isImageDiv function do the div check as well:

switch tt {
case html.ErrorToken:
    return
case html.StartTagToken:
    if isImageDiv(z.Token()) {
        readText = true
    }
case html.TextToken:
    if readText {
        *foundLabels = append(*foundLabels, getLabel(z))
        readText = false
    }
}

In main, you don't need to use make for a slice since the default value for a slice is an empty slice (you can just define it as var foundLabels []string and it will just work). Also, you shouldn't pass the slice as a pointer - slices are passed by reference anyway (not really, but due to the nature of how they work, they're already pointers). Make sure you change your crawl function as well after that.


Dictionary brute force on DES encrypted passwords For a school project I am tasked to decrypt DES passwords.  I have sample code provided below and I want to know if this is the best method. Also should I code in error handling for the dictionary file or does the while condition take care of this?

#include <stdio.h>
#include <crypt.h>
#include <unistd.h>
#include <string.h>

#define _XOPEN_SOURCE

int main(int argc, char* argv[])
{
    if (argc != 2)
    {
        printf("Usage: ./crack <password>\n");
        return 1;
    }

    int n = strlen(argv[1]);
    char password[n];

    for (int i = 0; i < n; i++)
    {
        password[i] = argv[1][i];
    }

    char salt[2];
    salt[0] = password[0];
    salt[1] = password[1];

    FILE *fp;
    fp = fopen("/usr/share/dict/words", "r");
    char line[50];

    while(fgets(line,50,fp)!= NULL)
    {
        line[strlen(line) - 1] = '\0';
        if (!strcmp(argv[1], crypt(line, salt)))
        {
            printf("Password found!\n");
            return 0;
        }
    }
    printf("Not found\n");
    fclose(fp);
    return 0;
}

You're keeping us in suspense — I'm dying to know which password matched!

Many password crackers also test for common character substitutions, e.g. o → 0, i → 1, s → $.

Other than that, the general technique seems sound.

I do have a few general remarks, though.

What is #define _XOPEN_SOURCE for?  Sometimes, defining it will change some of the functions you call.  However, it would only have any effect if you put it before your #includes.
It would be good to make a function, even for a simple program like this.  I suggest that main() be responsible for parsing and validating the command line and opening the word list, calling crack(const char *pwhash, FILE *wordlist) to do the actual work.
You don't close the word list if you find a match.
Consider returning a non-zero exit status if the password is not found.  That makes your program more useful to scripts.


Pascal's triangle JS formatting For my code, up to the 5th row, the formatting of my triangle is fine. But once I hit the 6th row, the formatting turns funny because I start having double digits. Is there any way to fix this?

Also is my code optimized? Any way I can make it better?

Thanks!

function pascals(num) {
  var result = [[1],[1,1]];

  if (num === 0) {
    console.log(0);
  }

  if (num === 1) {
    console.log(1);
  }

  else {
    for (var i = 2; i < num; i++) {
      result[i] = [];
      result[i][0] = 1;

      for (var j = 1; j < i; j++) {

        result[i][j] = result[i - 1][j - 1] + result[i - 1][j];
      }
      result[i][j] = 1
    }
  }
  for (var i = 0; i < result.length; i++) {
    console.log(' '.repeat(result.length - i) + result[i]);
  }
}

pascals(6)

      1
     1,1
    1,2,1
   1,3,3,1
  1,4,6,4,1
 1,5,10,10,5,1

Multi-digit number is what is throwing it off. I typically fix this by opting to find the highest number and then offset the rest of them appropriately.  I did this back when I was messing around with polynomial triangles.

Fixing the spacing issue

I started by getting the longest possible number and then prepending the space before it to make it line up properly.

Nextly, you have to fix the preappend so it looks better over size 2.  This is accomplished over a fairly odd algorithm.

As for the rest of the code, I did not have time to look at it, but will in a bit.

Fixing structure

Overall, your code looked pretty good.  Here are just some suggestions.

I went ahead and moved the pascal array generation to another function.  The idea of a function is that it performs a task.  Logically breaking up these tasks into multiple functions limits the size of each function and increases readability.  From there, we can assign it to a variable inside the pascal function.  I used the intuitive name generatePascalArrays, so you can get the gist of what the function does from the variable assignment.

Secondly, I noticed you split the 1 and 0 exception cases, I combined that into one if and console.log(num).

Additionally, I did use some arrow functions.  In other languages, you may have heard of them as lambda functions.  If you are not familiar with them, here is a link.

function generatePascalArrays(num) {
  var result = [[1],[1,1]];
  for (var i = 2; i < num; i++) {
    result[i] = [];
    result[i][0] = 1;
    for (var j = 1; j < i; j++) {
      result[i][j] = result[i - 1][j - 1] + result[i - 1][j];
    }
    result[i][j] = 1;
  }
  return result;
}

function pascals(num) {
  if (num <= 1) {
    console.log(num);
  }
  var result = generatePascalArrays(num),
      width = Math.max(...result[result.length-1]).toString().length;
  for (var i = 0; i < result.length; i++) {
 let preOffsetter = result[result.length-i-1].length * Math.floor(width/3) + result.length - i
 console.log(' '.repeat(preOffsetter) + result[i].map((x) => ' '.repeat(width - x.toString().length) + x));
  }
}

pascals(5)
pascals(6)
pascals(10)


A simple brainf*** interpreter in Haskell I am very new to Haskell and come from languages like C++, although I do have some experience with Scheme. Here, I wrote a simple brainf*** interpreter, which so far is my largest project. I followed a tutorial for some of the code (such as the Tape data type), but did most of this on my own. I have one major concern with my approach, which is the way I handle the loops (which in brainf*** are written with [] and repeat until the current cell in the tape is zero. In my implementation, I used the loop as a parameter to the main recursive function, which is called if the current instruction is ] and the current cell is not 0. Since these loops modify the tape, I had to make them return an instance of Tape that would replace the current one. However, since the function also handles IO (and therefore must return an instance of IO), I made it return an IO (Tape Int) which is then unpacked in a do block. This all felt very messy and hacky, so I would really appreciate any help from more experienced Haskell developers.

import Data.Maybe

--The Tape data type and functions

data Tape a = Tape [a] a [a]

newTape :: a -> Tape a
newTape x = Tape r x r
    where r = repeat x

moveLeft :: Tape a -> Tape a
moveLeft (Tape (l:ls) x rs) = Tape ls l (x:rs)

moveRight :: Tape a -> Tape a
moveRight (Tape ls x (r:rs)) = Tape (x:ls) r rs

--The Brainf*** instruction data types

data BfInstruction
    = MovLeft
    | MovRight
    | Increment
    | Decrement
    | Output
    | Input
    | BeginLoop
    | EndLoop
    deriving (Show, Eq)

type BfProgram = [BfInstruction]

--Convert string to BfProgram

parseBf :: String -> BfProgram
parseBf = mapMaybe parse
    where
        parse :: Char -> Maybe BfInstruction
        parse x = case x of
            '<' -> Just MovLeft
            '>' -> Just MovRight
            '+' -> Just Increment
            '-' -> Just Decrement
            ',' -> Just Input
            '.' -> Just Output
            '[' -> Just BeginLoop
            ']' -> Just EndLoop
            x   -> Nothing         --anything but the above chars is a comment

--Main running function

runBf :: String -> IO ()
runBf p = runBf' (parseBf p) (newTape 0) [] >> return ()
    where
        runBf' :: BfProgram -> Tape Int -> BfProgram -> IO (Tape Int)
        runBf' [] tape _ = return tape
        runBf' prog@(p:ps) tape@(Tape ls x rs) loop = case p of
            MovLeft   -> advance prog (moveLeft tape)
            MovRight  -> advance prog (moveRight tape)
            Increment -> advance prog (Tape ls (x+1) rs)
            Decrement -> advance prog (Tape ls (x-1) rs)
            Input     -> do
                char <- getChar
                advance prog (Tape ls (fromEnum char) rs)
            Output    -> putChar (toEnum x) >> advance prog tape
            BeginLoop ->
                let lp = getLoop 1 ps
                in runBf' (length lp `drop` ps) tape lp  --Drop so that we are at the ] now
            EndLoop   ->
                if x /= 0
                then do
                    lt <- runBf' loop tape []
                    runBf' prog lt loop       --Copy the tape from the result of the loop into next iteration
                else advance prog tape

        advance :: BfProgram -> Tape Int -> IO (Tape Int)
        advance (p:ps) tape = runBf' ps tape []

        getLoop :: Int -> BfProgram -> BfProgram
        getLoop _ [] = error "Mismatched brackets in BF program"
        getLoop 1 (EndLoop:ps) = []
        getLoop n (p:ps) = p:case p of
            BeginLoop -> getLoop (n + 1) ps
            EndLoop   -> getLoop (n - 1) ps
            _         -> getLoop n ps

--Simple IO

main = do
    program <- readFile "program.bf"
    runBf program

Well done. Overall, there are no big flaws, just some minor issues.

Type annotations

While it's great that all top-level functions have proper type signatures, the local bindings inside those functions usually don't. After all, their types should get inferred, e.g.

example :: [Int] -> [Int]
example = map inc
  where
    inc x = 1 + x

doesn't need a type signature since x's type is already fixed to Int. It makes refactoring also a lot easier if we change the type later. If we started with

example :: [Int] -> [Int]
example = map inc
  where
    inc :: Int -> Int
    inc x = 1 + x

and later want to generalize, we might forget the second type signature and end up with an error message:

example :: Num a => [a] -> [a]
example = map inc
  where
    inc :: Int -> Int -- whoops, GHC will yell about that
    inc x = 1 + x

Therefore, type signatures for local functions are usually not written out. There are some instances where they're necessary, but that's usually with RankNTypes or other extensions.

The tape

The tape works well, and is pretty much how you would expect it.

Infinite tapes and debugging

That being said, an infinite tape has the slight inconvenience that you can never inspect it for debugging purposes.

Also, if you ever create a module from your code, you must not export the Tape data constructor, as it would enable Tape [] 0 [] and therefore break assertions.

A finite tape circumvents those issues, but needs slightly more effort in the movements.

Working on the current value

In runBf we can find several spots where we advance the program after we worked on the current value, e.g.:

        Increment -> advance prog (Tape ls (x+1) rs)
        Decrement -> advance prog (Tape ls (x-1) rs)

That's now a possible source of errors, since we could have used

        Increment -> advance prog (Tape ls (x+1) ls)
        Decrement -> advance prog (Tape ls (x-1) ls)

by accident. A small helper can prevent that issue:

onCurrent :: (a -> a) -> Tape a -> Tape a
onCurrent f (Tape ls x rs) = Tape ls (f x) rs

current   :: Tape a -> a
current (Tape _ x _ ) = x

Then we end up with

        MovLeft   -> advance prog (moveLeft tape)
        MovRight  -> advance prog (moveRight tape)
        Increment -> advance prog (onCurrent (+1) tape)
        Decrement -> advance prog (onCurrent (subtract 1) tape)
        Input     -> do
            char <- getChar
            advance prog (onCurrent (const (fromEnum char)) tape)

Naming and scope

As neither advance nor getLoop use any of the bindings in their scope, they're candidates for top-level functions.

runBf' can be called go or another short name. Calling the inner worker just go is really common and won't alienate other readers.

Make interfaces hard to use wrong

getLoop uses an Int as first argument that's not properly documented. Types only go so far as documentation, and we could accidentally use getLoop 0 in BeginLoop.

Instead, we should make it impossible to misuse getLoop:

getLoop :: BfProgram -> BfProgram
getLoop = go 1
  where
    go _ [] = error "Mismatched brackets in BF program"
    go 1 (EndLoop:ps) = []
    go n (p:ps) = p:case p of
        BeginLoop -> go (n + 1) ps
        EndLoop   -> go (n - 1) ps
        _         -> go n ps

Similarly, runBf should probably take a BfProgram, not an arbitrary String, as this doesn't decrease the strength of your program, we can recreate the previous behaviour with

runBf . parseBf

However, speaking of parsing…

Loop validation

A drawback with our current BfProgram is that we might end up with mismatched brackets, e.g.

parseBf "]["

parses fine and leads to a runtime error. However, we could easily detect that during parsing. Our parseBf needs a way to report errors:

type ParserError = String

parseBf :: String -> Either ParserError BfProgram
parseBf = go
  where
    go []     = Right []
    go (x:xs) = case x of
        '<' -> MovLeft <$:> go xs
        '>' -> MovRight <$:> go xs
        '+' -> Increment <$:> go xs
        '-' -> Decrement <$:> go xs
        ',' -> Input <$:> go xs
        '.' -> Output <$:> go xs
        '[' -> -- exercise ; use `getLoop`-like function
        ']' -> -- exercise ; easier if previous one done correctly.
        x   -> go xs
   x <$:> xs = fmap (x:) xs

but afterwards, we can be sure that parseBf only returns BfPrograms with valid brackets.

Unfortunately, we still need to use getLoop, as BeginLoop and EndLoop are still in our instruction set. If we change the instruction set, we can get rid of that too:

data BfInstruction
    = MovLeft
    | MovRight
    | Increment
    | Decrement
    | Output
    | Input
    | Loop BfProgram
    deriving (Show, Eq)

I go into more details in some of my previous Bf reviews, feel free to read them if you get stuck on Loop.

Final remarks

Other than the usual re-evaluation of loops (which is a common scenario in Haskell Bf interpreters), your code was fine, so all the issues are really minor. Again: well done.


Method to save child objects I'm using Entity Framework 6 with database first. See the image for the relevant database schema

I have the following method to save a Set to the database:

/// <summary>
/// Saves the list of data sources. This assumes that the ModelTemplateID, SetNumber and TaskNumber
/// are already properly set. Adds a new record to the DataSourceGroups table if necessary.
/// </summary>
/// <param name="dataSources">The list of data sources to save</param>
public void SaveSet(IEnumerable<DataSource> dataSources)
{
    using (var db = _contextFactory.GetContext())
    {
        foreach (var dataSource in dataSources)
        {
            var dataSourceFromDb =
                db.DataSources.FirstOrDefault(x => x.DataSourceID == dataSource.DataSourceID);

            if (dataSourceFromDb == null)
            { // not in database yet, add it
                var dataSourceGroupFromDb =
                    db.DataSourceGroups.SingleOrDefault(
                        x => x.ModelTemplateID == dataSource.ModelTemplateID &&
                             x.SetNumber == dataSource.SetNumber &&
                             x.TaskNumber == dataSource.TaskNumber);
                if (dataSourceGroupFromDb == null)
                { // Add new DataSourceGroup to db
                    db.DataSourceGroups.Add(new DataSourceGroup
                    {
                        ModelTemplateID = dataSource.ModelTemplateID,
                        SetNumber = dataSource.SetNumber,
                        TaskNumber = dataSource.TaskNumber
                    });
                }
                db.DataSources.Add(dataSource);
            }
            else
            { // already in database, if any fields are different then update it
                if (dataSourceFromDb.QueryID      != dataSource.QueryID || // todo make DataSource Comparer
                    dataSourceFromDb.SourceType   != dataSource.SourceType ||
                    dataSourceFromDb.TargetFormat != dataSource.TargetFormat ||
                    dataSourceFromDb.SourcePath   != dataSource.SourcePath ||
                    dataSourceFromDb.TargetPath   != dataSource.TargetPath)
                {
                    db.DataSources.Attach(dataSource);
                    db.Entry(dataSource).State = EntityState.Modified;
                }
            }
        }
        db.SaveChanges();
    }
}

Is this the best way to implement this method? Should I be attaching the DataSources to the Set when they are created and then just pass the Set into the method? Any other critiques welcome.

/// <summary>
/// Saves the list of data sources. This assumes that the ModelTemplateID, SetNumber and TaskNumber
/// are already properly set. Adds a new record to the DataSourceGroups table if necessary.
/// </summary>
/// <param name="dataSources">The list of data sources to save</param>
public void SaveSet(IEnumerable<DataSource> dataSources)

The method name is telling a lie. SaveSet would be appropriate if the method took an IEnumerable<Set> sets parameter. I would have called the method SaveDataSources.

Per the XML comments, the method is making assumptions, but it does nothing to verify these assumptions, and since the method has no return type, the only way you can fail is by letting it throw - throwing is good in this situation, however it's best to fail early, if possible before you even create the context instance.

This is done with a guard clause at the top of your method, that aborts the call if an argument is invalid - for example:

if (dataSources.Any(dataSource =>
       dataSource.ModelTemplateID == default(typeof(dataSource.ModelTemplateID))
    || dataSource.SetNumber == default(typeof(dataSource.ModelTemplateID))
    || dataSource.TaskNumber == default(typeof(dataSource.TaskNumber))))
{
    throw new ArgumentException("Members 'ModelTemplateID', 'SetNumber' and 'TaskNumber' not set.", "dataSources");
}

This makes your code throw a more meaningful exception, instead of adding half the items, throwing some foreign key exception and then disposing the context without persisting anything.

using (var db = _contextFactory.GetContext())

While a conveniently short identifier, db is a misleading name to use for a DbContext; it's more of a context than a database, so context would be more appropriate.

I like that the method isn't directly responsible for creating the DbContext instance, however GetContext() sounds like you're not creating anything, rather just returning an existing instance - which is counter-intuitive in the context (pun not intended) of a factory. CreateContext would be better, and since _contextFactory is already a quite descriptive name, a Create method wouldn't be any less clear:

using (var context = _contextFactory.Create())
{

foreach (var dataSource in dataSources)
{
    var dataSourceFromDb =
        db.DataSources.FirstOrDefault(x => x.DataSourceID == dataSource.DataSourceID);

Unless I look at your schema and see that DataSourceID is the entity's primary key, I have no way of being sure that you're expecting only a single record, because FirstOrDefault could just as well operate on 20 entities and only return the first that matches the criteria.

This would be less ambiguous, because if more than 1 entity is returned, SingleOrDefault would throw an exception:

foreach (var dataSource in dataSources)
{
    var dataSourceFromDb =
            context.DataSources
                   .SingleOrDefault(e => e.DataSourceID == dataSource.DataSourceID);

This part looks like it belongs in another method:

        if (dataSourceFromDb == null)
        { // not in database yet, add it
            var dataSourceGroupFromDb =
                db.DataSourceGroups.SingleOrDefault(
                    x => x.ModelTemplateID == dataSource.ModelTemplateID &&
                         x.SetNumber == dataSource.SetNumber &&
                         x.TaskNumber == dataSource.TaskNumber);
            if (dataSourceGroupFromDb == null)
            { // Add new DataSourceGroup to db
                db.DataSourceGroups.Add(new DataSourceGroup
                {
                    ModelTemplateID = dataSource.ModelTemplateID,
                    SetNumber = dataSource.SetNumber,
                    TaskNumber = dataSource.TaskNumber
                });
            }
            db.DataSources.Add(dataSource);

I don't like the comments on the same line as the scope-opening brace { //, and these comments wouldn't be needed at all if you extracted the method; the added level of abstraction would speak for itself:

if (dataSourceFromDb == null)
{
    EnsureDataSourceGroupExists(context, dataSource);
    context.DataSources.Add(dataSource)
}
else
{
    dataSourceFromDb.QueryID = dataSource.QueryID;
    dataSourceFromDb.SourceType = dataSource.SourceType;
    dataSourceFromDb.TargetFormat = dataSource.TargetFormat;
    dataSourceFromDb.SourcePath = dataSource.SourcePath;
    dataSourceFromDb.TargetPath = dataSource.TargetPath;
}

context.SaveChanges();

Notice the how the update code is simply copying the values from dataSource into the members of dataSourceFromDb - that entity is already attached and managed by the change tracker. Why complicate things? It's there, might as well use it!

One thing your code also assumes, is that SaveChanges will insert the DataSourceGroup entity before it inserts dataSource. I don't know about EF6, but in other versions one cannot make that assumption, so EnsureDataSourceGroupExists would call SaveChanges on the context if a new entity was created. Not doing this is like asking for a foreign key violation.


Pairwise grouping of elements of a list I want to group the elements in an arbitrary long list into pairs and implemented the following so far:

(defun group-pairwise (numbers)
  "Returns a list with the elements of NUMBERS grouped in pairs."
  (cond ((null numbers) nil)
        ((null (rest numbers)) nil)
        (t (append (list (list (first numbers) (second numbers)))
                   (group-pairwise (rest numbers))))))

It works, i.e.

> (group-pairwise '(3 6 0))
((3 6) (6 0))

but I found (append (list (list (first numbers) (second numbers) a bit cumbersome. Is there any way to express it more elegant?

Furthermore, I want in my later code not only to return the list of pairs but to perform an action on each pair. To keep the recursion pattern unchanged I was thinking about using progn instead of append , e.g.

(defun generate-graph-of-digits%%% (numbers &optional (graph *pe79-graph*))
  "Returns a graph structure of NUMBERS."
  (cond ((null numbers) nil)
        ((null (rest numbers)) nil)
        (t (progn
             (add-edge-between-vertexes graph (first numbers) (second numbers))
             (generate-graph-of-digits%%% (rest numbers))))))

add-edge-between-vertexes being defined in the cl-graph package.

I am unsure about the usage of progn. Is it recommended to use it here?

Much too complicated. Use MAPCAR:

(defun group-pairwise (numbers)
  "Returns a list with the elements of NUMBERS grouped in pairs."
  (mapcar #'list numbers (rest numbers)))

Similar:

(defun generate-graph-of-digits (numbers graph)
  (mapc (lambda (a b)
          (add-edge-between-vertexes graph a b))
        numbers
        (rest numbers)))

Such primitive recursion can be replaced with mapping functions. Bonus: mapping functions are not limited by recursion stack.

Style

Also:

(append (list (list a b)) some-list)

is

(cons (list a b) some-list)

Then:

(cond (a (progn b c d))
      ((evenp n) (progn o p q))
      (t (progn e f g)))

is:

(cond (a b c d)
      ((evenp n) o p q)
      (t e f g))

Each cond clause accepts any number of Lisp forms after the condition.


Querying multiple related records in Rails I have an Account, User and Group model. All User and Group records belong to an Account, so it is possible for both users and groups to be related to various accounts.

Basically, I have some really hacky, pieced together SQL queries that lookup all Account records related to a given User or Group record through various
associations.

Even though it's fast and only requires a single query for everything to work, I really hate the way it's written.

I was wondering if there is any way I could do this more programatically with something like arel, or if there was a better approach. I also have some security concerns about the code.

class Account < ActiveRecord::Base

  belongs_to :accountable, touch: true, polymorphic: true

  CLAUSE_FOR_RELATED = '"accounts"."accountable_type" = \'%s\' AND "accounts"."accountable_id" IN (%s)'.freeze

  def self.related_to_user(user)
    groups = user.groups.select('"groups".id')
    friends = user.following_friendships.select('"friendships".following_id')

    queries = []
    queries.push ['Group', groups.to_sql]
    queries.push ['User', friends.to_sql]
    queries.push ['User', user.id]

    self.related_to(queries)
  end

  def self.related_to_group(group)
    queries = [
      ['User', group.members.select('"users".id').to_sql]
    ]

    self.related_to(queries)
  end

  def self.related_to(queries)
    clauses = queries.map do |clause|
      sprintf(CLAUSE_FOR_RELATED, clause[0], clause[1])
    end

    self.where(clauses.join(" OR "))
  end
end

I would try to use as much as possible what is provided by ActiveRecord. I was thinking to something like:

class Account < ActiveRecord::Base
  belongs_to :accountable, touch: true, polymorphic: true
  CLAUSE_FOR_RELATED = '"accounts"."accountable_type" = \'%s\' AND "accounts"."accountable_id" IN (%s)'.freeze

  class << self
    def related_to_user(user)
      group_ids = user.groups.pluck('groups.id')
      friend_ids = user.following_friendships.pluck('friendships.following_id')

      query = [
        compose_clause('Group', group_ids.join(', ')),
        compose_clause('User', [user.id, *friend_ids].join(', '))
      ].join(' or ')

      where(query)
    end

    def related_to_group(group)
      member_ids = group.members.pluck('users.id')
      where(compose_clause('User', member_ids.join(', ')))
    end

    private
    def compose_clause(accountable_type, ids)
      sprintf(CLAUSE_FOR_RELATED, accountable_type, ids)
    end
  end
end

Now in Rails 5 you could even write it like:

class Account < ActiveRecord::Base
  belongs_to :accountable, touch: true, polymorphic: true

  class << self
    def related_to_user(user)
      group_ids = user.groups.pluck('groups.id')
      friend_ids = user.following_friendships.pluck('friendships.following_id')

      where(accountable_type: 'Group', accountable_id: group_ids).or(
         Account.where(accountable_type: 'User', accountable_id: [user.id, *friend_ids])
      )
    end

    def related_to_group(group)
      member_ids = group.members.pluck('users.id')
      where(accountable_type: 'User', accountable_id: member_ids)
    end
  end
end


Is this the best message delay algorithm? In my application I am attempting to queue outgoing messages by the following rules

By default they should be sent no less than messageDelay apart
Some messages can be sent immediately, completely bypassing all queued messages
Additionally, some immediately sent messages can reset the waiting time of queued messages

If this is confusing, this is what it would look like on the wire

|  Normal  |Immediate |Immediate-reset|
M----------M---M------M----M----------M ....

This was originally handled by a dedicated OutputThread and queue but for various reasons outside of the scope of this question I am attempting to implement this with ReentrantLock and Condition so when any of the methods finally returns, the message is known to of been sent (or an Exception is thrown meaning it didn't)

However this is my first time using ReentrantLock and while I think I've implemented it correctly I'm needing someone to verify that its correct since this is an extremely difficult thing to test

protected final ReentrantLock writeLock = new ReentrantLock(true);
protected final Condition writeNowCondition = writeLock.newCondition();

public void sendRawLine(String line) {
    //[Verify state code]
    writeLock.lock();
    try {
        sendRawLineToServer(line);
        //Block for messageDelay. If rawLineNow is called with resetDelay
        //the condition is tripped and we wait again
        while (writeNowCondition.await(getMessageDelay(), TimeUnit.MILLISECONDS)) {
        }
    } catch (Exception e) {
        throw new RuntimeException("Couldn't pause thread for message delay", e);
    } finally {
        writeLock.unlock();
    }
}

public void rawLineNow(String line, boolean resetDelay) {
    //[Verify state code]
    writeLock.lock();
    try {
        sendRawLineToServer(line);
        if (resetDelay)
            //Reset the
            writeNowCondition.signalAll();
    } finally {
        writeLock.unlock();
    }
}

Is this the correct way to implement ReentrantLock based on my requirements above? Is there a better way (NOT USING THREADS) to do this?

I can see some problems with the code:

As @fge mentions, catching Exception is problematic.  You should catch the exceptions that you are expecting to occur, and let any other unchecked exceptions propagate.
Wrapping the exceptions in RuntimeException is a bad idea unless it is really necessary.  It is better to either let them propagate, or wrap them in a custom checked or unchecked exception.
The behaviour of the code doesn't match your description in one respect.  You talk about queuing messages. In fact, messages are being sent immediately in all cases, and you are delaying after sending the message in the sendRawLine case.
In reality, you are blocking client threads rather using a queue data structure and a separate output thread.  This might be the right things to do ... but it might be the wrong thing.  It depends whether the client threads should be blocked until the their respective messages are sent.  Either way, your characterization this as a "message queuing" problem is a bit misleading.

  Re: #3) The idea was to delay the next message from sending. Otherwise all calls to sendRawLine would have a weird pause before doing anything. I'm looking into using the current time though to determine if waiting is even necessary.

First, that is NOT what your description of "message queuing" implies ... to me.  And consider me as an examplar of someone else reading your code.

Second, (if I understand you correctly) using the current time to decide to whether to wait before a send would require that something keeps track of when each thread last sent a message.

Third, there is actually a good reason to not wait before sending a message.  If you do wait before then, by the time the wait period has finished, the message could be rather out of date ... especially if there were a number of resets.

Finally, it strikes me that you may be over-using threads here and/or that it might be better for the entity (thread or FSM) responsible for generating messages should check whether a message needs to be sent before generating it.


Sourcing content from a webpage in an organized manner I've written a script in python to grab different title and address from different pages of a website. Firstly the script will collect all the property links from the landing page and then go one layer deep to collect the title and address. When I run my script, I get the results accordingly.

Should it not be a better approach If I call a single function and the rest of the functions work like a chain to produce the same results? If so, what is the right way to do so?

This is the link to that site

Here is the working script:

import requests
from urllib.parse import urljoin
from bs4 import BeautifulSoup

url = 'https://www.booking.com/searchresults.html?aid=304142;label=gen173nr-1FCAEoggJCAlhYSDNYBGhpiAEBmAExuAEHyAEM2AEB6AEB-AECkgIBeagCAw;sid=7abf6bf275d09e8d4f617d51f4d6c803;class_interval=1;dest_id=102;dest_type=country;dtdisc=0;from_sf=1;group_adults=2;group_children=0;inac=0;index_postcard=0;label_click=undef;no_rooms=1;offset=0;postcard=0;raw_dest_type=country;room1=A%2CA;sb_price_type=total;search_selected=1;slp_r_match=0;src=index;src_elem=sb;srpvid=134253d18d97017e;ss=Ireland;ss_all=0;ss_raw=ireland;ssb=empty;sshis=0&'
headers = {"User-Agent":"Mozilla/5.0 (Windows NT 6.1) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/69.0.3497.100 Safari/537.36"}

def get_token():        #getting csrf_token
    r = requests.get(url)
    soup = BeautifulSoup(r.text,"lxml")
    item = soup.select_one("[name='bhc_csrf_token']")['value']

    payload = {
        'bhc_csrf_token': item,
        'logout': 1
    }
    return payload

def make_post(payload):  #making a post http request with the payload and parsing target page links
    res = requests.post(url,data=payload,headers=headers)
    sauce = BeautifulSoup(res.text,"lxml")
    linklist = []
    for elem in sauce.select(".sr_property_block a.hotel_name_link"):
        linklist.append(urljoin(url,elem.get("href").strip()))
    return linklist

def get_info(link): #scraping title and address by using each of the links
    response = requests.get(link,headers=headers)
    soupobj = BeautifulSoup(response.text,"lxml")
    name = soupobj.select_one("h2#hp_hotel_name").get_text(strip=True)
    addr = soupobj.select_one(".hp_address_subtitle").get_text(strip=True)
    print(f'{name}\n{addr}\n')

if __name__ == '__main__':
    for link in make_post(get_token()):
        get_info(link)

Perhaps I misunderstand you. But it looks as though you've already written the functions to call one another: get_info() calls make_post() internally, and make_post() then delegates getting the token to get_token() by calling it internally to itself. So in your final line of code, you should be able to replace the three calls with a single call to get_info().

Whether or not that's "better" than calling them independently is a question of style. Personally, I think it's best to keep the functions independent and call them separately. This would mean re-writing them to take parameters that are the results of the preceding function call.

This way the functions don't have to know about each other's existence, and means you will be able to change each one independently over time as requirements evolve, without needing to update the others (this is called low coupling, a very useful design quality to keep in mind).


Agnostic means of identifying a member I'm attempting to provide an agnostic means of identifying a member. I need a nickname, a guid, and one of user_id, email, or phone_number. Since I'm using Python, I try to not care about the Python type of member because, after all, if it has what I'm looking for then I don't really care whether it really is a Member... or so I have been taught.

There is a circumstance in my application in which actual Member instances may be lumped in with objects that represent the information necessary to create a member. Also, note that actual Member instances have a handy identification() method, which my code tries first.

The problem is in attempting to not care about its type.  I've gone to the extent of trying attributes and items, and now my code looks incredibly repetitive.

try:
    return member.identification()
except AttributeError:
    pass

id_types = ['user_id', 'email', 'phone_number']
if isinstance(member, dict):
    if 'guid' not in member:
        member['guid'] = cls._next_guid()
    for idt in id_types:
        if idt in member:
            try:
                return {
                    'nickname': member['nickname'],
                    'guid': member['guid'],
                    idt: member[idt]
                }
            except KeyError:
                raise ValueError('no nickname')
    else:
        raise ValueError('no user_id, email, or phone_number')
else:
    if 'guid' not in member:
        member['guid'] = cls._next_guid()
    for idt in id_types:
        if hasattr(member, idt):
            try:
                return {
                    'nickname': member.nickname,
                    'guid': member.guid,
                    idt: getattr(member, idt)
                }
            except AttributeError:
                raise ValueError('no nickname')
raise ValueError('no identification could be made')

The question is, if my attempts to not care about the type to this extent are reasonable, then is there a way to make this code better adhere to the principle of DRY?

I see 2 approaches to this:

Allow Member to be initialized with a dictionary.
Allow your Member class to behave like a dictionary.

If you proceed by approach 1, then your resulting code will be very concise:

if isinstance(member, Member):
    return member.identification()
else if isinstance(member, dict):
    member = Member(member)
    return member.identification()
raise ValueError('no identification could be made')

But you will have a lot of additional logic in Member

Alternatively, if you do not like that approach, we can make Member work like a dictionary. Basically member['guid'] would refer to member.guid if member is a Member or the dictionary version if member is a dictionary.

We can do this by adding some magic methods to Member: __getitem__(self, key): and __setitem__(self, key, val): and __contains__(self, val):. The code would look something like this:

def __getitem__(self, key):
    if key == "nickname":
        return self.nickname
    elif key == "guid":
        return self.guid
    # and so on

def __setitem(self, key, val):
    if key == "nickname":
        self.nickname = val
    elif key == "guid":
        self.guid = val
    # and so on

def __contains__(self, val):
    if val == "guid":
        return self.guid != None
    elif val == "user_id":
        return self.user_id != None
    # and so on

This would make code snippet something like this:

try:
    return member.identification()
except AttributeError:
    pass

if not isinstance(member, Member) and not isinstance(member, dict):
    raise ValueError('no identification could be made')

id_types = ['user_id', 'email', 'phone_number']
if 'guid' not in member:
    member['guid'] = cls._next_guid()
for idt in id_types:
    if idt in member:
        try:
            return {
                'nickname': member['nickname'],
                'guid': member['guid'],
                 idt: member[idt]
            }
        except KeyError:
            raise ValueError('no nickname')
 else:
    raise ValueError('no user_id, email, or phone_number')


Using ID's with a "scope" -like hierarchy I currently have a ResourceManager class which is responsible for searching for resources with a given identifier, returning a reference counted pointer to the resource if a valid resource is found, and instantiating a new object if none has been found. Often a resource itself depends on other resources, which are requested from the resource manager.  For example, Window may depend on SdlMain (which initializes and shuts down SDL), and thus the constructor of Window would ask ResourceManager for the SdlMain object.

Example:

ResourceManager resources;
//the return type is std::shared_ptr<Window>, the first argument "MainWindow" is the
//..ID to search for, the rest of the arguments are simply forwarded to the constructor
//..of class Window if needed.
auto mainWindow = resources.make<Window>("MainWindow", "Hello World",
                    512, 512, SDL_WINDOW_SHOWN, SDL_RENDERER_ACCELERATED |
                    SDL_RENDERER_PRESENTVSYNC);
//The Window object contains the renderer, too, so the ID of the current window
//"MainWindow" must be passed to the texture's constructor
auto tex1 = resources.make<Texture>("tex/tex1.png", "tex/tex1.png", "MainWindow");
//This renders the texture to the screen.
tex1->renderToBuffer(0,0);
mainWindow->bufferToDisplay();

However, the requirement of knowing the actual ID of the Window when creating a texture becomes a problem when writing a sprite class, if multiple window objects are to be supported:

//Unlike Texture, a Sprite is not a resource but it's lifetime is bound to the  object
//..owning it (and there may be many sprites using the same Texture). Thanks to
//..how Texture is implemented, now everything that wishes to use a sprite must be
//..aware of the ID of the window "MainWindow" in order to initialize a Sprite.
Sprite sprite(resources, "tex/tex1.png", "MainWindow");

I recently came up a solution for this. I would give the identifiers a context, similar to the "scope" of programming languages. SdlMain would be found in global context, with it's ID "SdlMain" available and meaningful anywhere, while one window may reside in context Game, and a second window may reside in the context Editor. Both Window instances would have the name "Window", but there would be no conflict since they reside in a different contexts. The resulting system might look somewhat like this:

ResourceManager resources;
//Set the scope to "Game"
resources.setContext("Game");
auto mainWindow = resources.make<Window>("Window", "Hello World",
                    512, 512, SDL_WINDOW_SHOWN, SDL_RENDERER_ACCELERATED |
                    SDL_RENDERER_PRESENTVSYNC);
//Texture asks ResourceManager simply for a "Window", with no idea about the current
//context.
Sprite sprite(resources, "tex/tex1.png");
sprite.draw();
mainWindow->bufferToDisplay();
//Set the context back to global
resources.setContext();

The actual ID search would function as follows: First the ID is searched for in the current context, if there are no matches the preceding context is evaluated too, until the search arrives at the global context.

Note that the final version would use integer ID:s instead of strings. I do realize that this might be the best example of overengineering ever, but I'm mostly doing this for its learning value.

Before fully committing to such a system, I would like to know some opinions on the matter from more experienced programmers:

Is this a practicable solution?  Are there any blatant flaws in this concept?  Any suggestions?

Preface: these are just my personal opinions based on experience.

Don't force the user of the class to remember to do stuff

The setContext(const char*) method is a method which the user of the class is required to call to have proper operation. As such the user is at risk of forgetting to call this method and thus introduce a bug in the software. Further more the user must remember to be consistent in spelling of the context name or face the risk of introducing subtle bugs where different contexts are used by mistake.

If you wish to go with the concept of contexts (say that 10 time quickly), I would make a "context" a first class object which is required for creating instances of resources. Like this:

class ResourceContext{
public:
    template<typename... Arg>
    std::shared_ptr<Resource> make(Arg... args);
};

class ResourceManager{
public:
    std::shared_ptr<ResourceContext> createContext(const char*);
    std::shared_ptr<ResourceContext> findContext(const char*);
private:
    friend class ResourceContext;
};

ResourceManager mgr;
auto gameContext = mgr.createContext("Game");
Sprite sprite(gameContext, "tex/tex1.png");

This approach solves my two worries

You can not forget to change context. You are forced to find the correct context.
You can not silently introduce a typo, either you explicitly create a new context or you look up an existing one (with an error if it doesn't exist).

Lookup by reference when possible

If your texture/sprite class is dependent on the window they are displayed in (as opposed to the SDL context) then it is quite reasonable to expect them to be created from a context where the window is available. Why not simply pass an instance of the resource<Window> to the constructor of the sprite/texture instead of doing a lookup by string ID?

Your current approach implicitly reserves "magic" names for specific instances of specific classes the risk here is that an unknowing user may create some kind of conflict because they didn't know of all the reserved names in your system.

One size doesn't fit all (Stop sugar coating my globals)

In general I believe that this kind of resource manager should only be used for obtaining external resources such as textures, sounds etc. To me it looks like you're trying to sugar coat having global variables, because that's essentially what this is being used like in your example. Using it for SDL context is overkill, why would you want to shutdown SDL during any point of your program's lifetime? What is the point of having it managed as a resource over just a simple global?

In all honesty, I think you are over-engineering this.


Parsing a Wikipedia page for a country The program should accept a name of a country as input. It should then parse the Wikipedia page for that country and find all links to the wikipedia pages of other countries on that page and make a list of all such countries.

For example, if the argument was India and http://en.wikipedia.org/wiki/India has links to wiki pages of Nepal and Pakistan, the list associated with India is [Nepal, Pakistan]. The program should recursively perform the above operation for any country you have not encountered so far.

After parsing the wiki page for India, it should parse the wiki page of Nepal, Pakistan and so on.

The final output is a json object with key being the country and value being the list of countries which were linked to the wiki page for the key country.

Sample output can be:

{"India": ["Nepal", "Pakistan"], "Nepal": ["India", "China"], "Pakistan": ["India"], "China": ["Japan"], "Japan": ["India", "China"]}

This is what I have tried, but I want to get the best approach for it:

import pycountry
import wikipedia

country_dict = {}
final_dict = {}

refr_list = []

def GetCountryDict():
    country_list = list(pycountry.countries)

    for country in country_list:
        country_dict[country.name] = 1

    return country_dict

def internalParse(internal_list,final_dict,countryDict,refr_list):

    for i in refr_list:
        if not final_dict.has_key(i):
            WikiParser(i,countryDict)
        else:
            refr_list.remove(i)

def WikiParser(country_name,country_dict):
    wiki_resp =[]
    internal_list = []
    try:
        wiki_resp = wikipedia.page(country_name)
    except:
        pass

    for i in wiki_resp.links:

        if country_dict.has_key(i):

            if i not in internal_list:
                refr_list.append(i)
                internal_list.append(i)
    final_dict[country_name] = internal_list
    try:

        refr_list.remove(country_name)
    except:
        pass
    return internal_list, final_dict,refr_list

if __name__ == '__main__':
    country_name = raw_input("Please enter country name: ")
    countryDict = GetCountryDict()

    internal_list,final_dict,refr_list = WikiParser(country_name,countryDict)

    while refr_list:

        internalParse(internal_list,final_dict,countryDict,refr_list)

You have quite a few violations of the style guide. This isn't necessarily crucial to follow (although highly recommended!), but more important is that your code lacks consistency.

You have three functions, two named with CapitalizedWords and one with mixedCase; and
internalParse's parameters are a mix of lower_case_with_underscores and mixedCase.

Your whitespace is at least consistent, but spaces after commas makes it easier to read; compare:

def internalParse(internal_list,final_dict,countryDict,refr_list):

with:

def internalParse(internal_list, final_dict, countryDict, refr_list):

country_dict, final_dict and refr_list shouldn't be defined at the top level of the script; global variables are a bad idea. You could create country_dict in GetCountryDict and final_dict and refr_list in WikiParser.

The current country_dict seems a bit pointless - you fill it with 1s for some reason, but then only ever use the keys. I would suggest a set instead, and using foo in bar rather than bar.has_key(foo) (in would still work with a dictionary, by the way). This really simplifies GetCountryDict:

def get_countries():
    """Get a set of all valid country names."""
    return set(country.name for country in pycountry.countries)

Note that I have renamed the function in line with the style guide and provided a docstring to explain what it does.

internalParse has four parameters but only uses three of them; you should either remove internal_list or, better, explicitly pass it through to WikiParser (where it's currently accessed by scope).

WikiParser is currently called in two places - directly from the if __name__ == '__main__': block, and indirectly via internalParse. This makes it more difficult that it needs to be to figure out what the code is doing.

Given the output you want, a better structure might be:

if __name__ == '__main__':
    # 1. Get set of valid countries first
    valid_countries = get_countries()

    # 2. Add input validation
    country_name = input_valid_country(valid_countries)

    # 3. Call parser
    print wiki_parser(country_name, valid_countries)

Notes:

Already covered;
See Asking the user for input until they give a valid response;
Refactor wiki_parser to return the dictionary of lists that you want. You can still have something like internalParse, but as a private function that's only called via wiki_parser (conventionally, it would therefore be named with a leading underscore: _internal_parse).

  I don't have pycountry or wikipedia installed, so this is untested. The ideas should be useful, though!

Given the nature of the task, one option for wiki_parser would be to do it recursively. This has advantages in terms of clear and comprehensible code, although one issue can be with hitting the system recursion limit if you delve too deeply:

def wiki_parser(country_name, valid_countries, out=None):
    """Recursively parse Wikipedia for linked countries."""
    if out is None:
        out = {}
    out[country_name] = []
    for country in _parse_page(country_name, valid_countries):
        if country not in out:
            out[country_name].append(country)
            wiki_parser(country_name, valid_countries, out=None)
    return out

This uses out both to provide the output and to determine if we've already seen a given country - membership testing (foo in bar) is as efficient with a dict as a set, as they're both hash-based (O(1), vs. O(n) for a list/tuple).

If you're wondering why I've used out=None rather than out={}, see “Least Astonishment” in Python: The Mutable Default Argument.

def _parse_page(country_name, valid_countries):
    """Parse a single page and return list of linked countries."""
    try:
        wiki_resp = wikipedia.page(country_name)
    except Exception:
        return []
    return [link for link in wiki_resp.links if link in valid_countries]

Note that except Exception: is the bare minimum - bare except is a very bad idea. Ideally, you should figure out what errors wikipedia.page can raise and handle them explicitly. I've also used a list comprehension to build the list of linked countries in one step.

You currently create a dictionary of lists, but not JSON; if you actually want to, look into json.


Handling game states for an online RPG game I'm writing an ORPG. The code below is client-side.

I have a main thread looping in Game::Run:

Game::~Game()
{
    PopAllStates();
}

void Game::Run()
{
    sf::Event Event;

    while(Window->isOpen())
    {
        if(LoadQueueMutex.try_lock())
        {
            while(!LoadQueue.empty())
            {
                LoadQueue.front().first->Load(LoadQueue.front().second);
                LoadQueue.pop();
            }
            LoadQueueMutex.unlock();
        }
        while(Window->pollEvent(Event))
        {
            StateStack.top()->HandleEvent(Event);
        }
        Window->clear();
        StateStack.top()->Draw();
        Window->display();
        StateStack.top()->Update();
    }
}

void Game::AddToLoadQueue(Loadable* pLoadable, WorldPacket Argv)
{
    boost::mutex::scoped_lock lock(LoadQueueMutex);
    LoadQueue.push(std::make_pair(pLoadable, Argv));
}

void Game::PushState(GameState* pState)
{
    StateStack.push(pState);
}

void Game::PopState()
{
    if(!StateStack.empty())
    {
        delete StateStack.top();
        StateStack.pop();
    }
}

void Game::PopAllStates()
{
    while(!StateStack.empty())
    {
        PopState();
    }
}

And another thread waiting for a packet:

void WorldSession::Start()
{
    Packet = WorldPacket((uint16)MSG_NULL);

    boost::asio::async_read(Socket,
        boost::asio::buffer(Packet.GetDataWithHeader(), WorldPacket::HEADER_SIZE),
        boost::bind(&WorldSession::HandleHeader, this, boost::asio::placeholders::error));
}

When the packet "arrives", this is usually done because OpenGL breaks if loaded from another thread:

void WorldSession::HandleSomeCoolOpcode()
{
    //...
    sGame->AddToLoadQueue(pSomethingCool, Packet);
}

When the main thread loads the Loadable*, it will usually mess with the state stack:

void World::Load(WorldPacket Argv)
{
    //...
    sGame->PopState();
    sGame->PushState(this);
}

Is there any way to improve this?

Your variables and functions should either be in camelCase or another naming convention that differentiates them from user-defined types (such as Game here), which are capitalized.
If a parameter is not supposed to be modified, and it's not a primitive type (such as an int), pass it by const-reference in order to avoid an unnecessary copy.

In both AddToLoadQueue() and PushState(), you're passing a raw pointer to an object that will not be modified within the function.  This is also discouraged as you should refrain from passing raw pointers to functions in C++, mostly due to issues such as dangling pointers.  If you still need to pass around pointers, pass smart pointers instead (if you have C++11).

Argv in AddToLoadQueue() should also be passed by const-reference, not by value.
Prefer not to cast the C-way in C++:

Packet = WorldPacket((uint16)MSG_NULL);

Cast the C++ way, with static_cast<> in this case:

Packet = WorldPacket(static_cast<uint16>(MSG_NULL));

See this answer for more information on different types of casts.
It's not exactly clear how StateStack is supposed to be used here.  You use delete on its elements in PopState(), so it's assumed that push() uses new to add new elements, which then means it's a dynamic container.  Either way, this implementation should be provided here.

However, if this is entirely identical to a regular dynamic stack, then just use std::stack.  It will also handle all the memory-management for you (no new and delete necessary).

std::stack<GameState> stateStack;

You'll also no longer need to use a separate top() and pop() when popping a state:

stateStack.pop();

If PopAllStates() is just done in the destructor, then you can just put that code there instead of defining a separate function.

It also calls empty() twice (PopState() also calls empty()), which isn't necessary.  If this is only needed in the destructor, put that code (with just one check) into it:

Game::~Game()
{
    while (!StateStack.empty())
    {
        delete StateStack.top();
        StateStack.pop();
    }
}

If you use std::stack, you'll just need a call to pop(), not top().


The difference of two arrays I want to get the difference between arr1 and arr2 and to get the correct result. However, I think my code is a little bit redundant.

function diffArray(arr1, arr2) {
var newArr = [];
// Same, same; but different.
  for(var i = 0;i<arr2.length;i++){
      var count = 0;
      for(var j = 0;j<arr1.length;j++){
          if(typeof arr2[i] == "number" && typeof arr1[j] == "number" && arr2[i] == arr1[j]) {
              count += 1;
          }
          if(typeof arr2[i] == "string" && typeof arr1[j] == "string" && arr2[i] === arr1[j]) {
              count += 1;
          }else{

          }

      }
      if(count == 0){
          newArr.push(arr2[i]);
      }
  }
  for(var j = 0;j<arr1.length;j++){
      var count = 0;
      for(var i = 0;i<arr2.length;i++){
          if(typeof arr2[i] == "number" && typeof arr1[j] == "number" && arr2[i] == arr1[j]) {
              count += 1;
          }
          if(typeof arr2[i] == "string" && typeof arr1[j] == "string" && arr2[i] === arr1[j]) {
              count += 1;
          }
      }
      if(count == 0){
          newArr.push(arr1[j]);
      }
  }
return newArr;
}

diffArray([1, "calf", 3, "piglet"], [1, "calf", 3, 4]);

Style

Your function name doesn't tell me whether you compute the symmetric or non-symmetric set difference. How about symmetricDifference instead of diffArray?
Instead of newArr I'd prefer reading e.g. result which tells me something about the role of that variable instead of its type.
There is an empty else { } clause in the first inner loop but not the second inner loop. Without that purely stylistic difference, the similarity of these two code blocks becomes more apparent.
The statements within the succeeding if clauses are identical, so you can merge them into one by combining the conditions with a logical OR operator.
The type checks and equality tests performed within the if conditions seem overly complex. AFAIK == is identical to === when both operands have the same type, so there is no need to mix them. Also, a generic function named diffArray should handle all possible array values, not only numbers and strings. This would allow you to simplify the two if conditions to e.g. a simple if (arr2[i] === arr1[j]) { ... }.
The iterator variable i is used to iterate through arr2 while iterator variable j is used to iterate through arr1. Readability improves by matching i with arr1 and j with arr2 according to their alphabetical order.

Performance

The inner loops are counting how often elements from the first array appear in the second array and vice versa. However, you are not interested in the exact count, only if count > 0. So you could use a labeled continue as soon as the count increments for the very first time:

outer: for (var i = 0; i < arr1.length; i++) {
  for (var j = 0; j < arr2.length; j++) {
    if (arr1[i] === arr2[j]) continue outer;
  }
  result.push(arr1[i]);
}

Now, it is much easier to see that the inner loop is actually just checking the existence of an element within an array. You can use the faster built-in indexOf method instead:

for (var i = 0; i < arr1.length; i++) {
  if (arr2.indexOf(arr1[i]) < 0) result.push(arr1[i]);
}

Even better, use the newer and more explicit includes method. However, this changes the semantics of your code due to the different handling of NaN:

NaN === NaN // false
[NaN].indexOf(NaN) // -1
[NaN].includes(NaN) // true

Now, your updated code could look as follows:

function symmetricDifference(arr1, arr2) {
  const result = [];
  for (const val of arr1) {
    if (!arr2.includes(val)) result.push(val);
  }
  for (const val of arr2) {
    if (!arr1.includes(val)) result.push(val);
  }
  return result;
}

Declarative vs. Imperative

Those two remaining loops actually filter the input arrays and return the remaining unique values. A more descriptive and possibly self-documenting way of writing such an operation is given by the filter method:

function symmetricDifference(arr1, arr2) {
  const difference1 = arr1.filter(val => !arr2.includes(val));
  const difference2 = arr2.filter(val => !arr1.includes(val));
  return difference1.concat(difference2);
}

However, the performance of plain for loops is superior.

Runtime Complexity

If you have to deal with larger arrays and prefer to have an implementation with higher setup costs but linear instead of quadratic runtime complexity, convert the input arrays into sets first and use the much faster set.has(val) instead of arr.includes(val).

Generalization

A generic solution which is not restricted to arrays but handles any iterable input could look as follows:

function* symmetricDifference(iterable1, iterable2) {
  const set1 = new Set(iterable1);
  const set2 = new Set(iterable2);
  for (const val of set1) if (!set2.has(val)) yield val;
  for (const val of set2) if (!set1.has(val)) yield val;
}


Seeking a quick and dirty way to populate a list of entities for testing purposes Here's what I have at the moment. Any ideas for improving on this?

public static Snapshot[] GetSnapshots()
{
    var arrayedSnapshot = new
    {
        Id = Enumerable.Range(1, 9),
        ScenarioTimeInSeconds = new[] { 0, 0, 0, 60, 60, 60, 120, 120, 120 },
        BearingInDegrees = new[] { 0, 120, 220, 0, 120, 220, 0, 120, 220},
        RangeInNauticalMiles = new[] { 0, 1, 1, 0, 1, 1, 0, 1, 1 },
        CourseInDegrees = new[] { 90, 90, 90, 90, 90, 180, 90, 90, 90 },
        SpeedInKnots = Enumerable.Repeat(7, 9),
        XCoordinate = new[] { 0, 0, 0, 0, 0, 0, 0, 0, 0 },
        YCoordinate = new[] { 0, 0, 0, 0, 0, 0, 0, 0, 0 }
    };
    return arrayedSnapshot.Id
        .Select((td, index) =>
            new Snapshot
            {
                Id = arrayedSnapshot.Id ElementAt(index),
                ScenarioTimeInSeconds = arrayedSnapshot.ScenarioTimeInSeconds.ElementAt(index),
                BearingInDegrees = arrayedSnapshot.BearingInDegrees.ElementAt(index),
                RangeInNauticalMiles = arrayedSnapshot.RangeInNauticalMiles.ElementAt(index),
                CourseInDegrees = arrayedSnapshot.CourseInDegrees.ElementAt(index),
                SpeedInKnots = arrayedSnapshot.SpeedInKnots.ElementAt(index),
                XCoordinate = arrayedSnapshot.XCoordinate.ElementAt(index),
                YCoordinate = arrayedSnapshot.YCoordinate.ElementAt(index),
            })
        .ToArray();
}

Edit

Here is a new version based on Anton Golov's answer:

public static IEnumerable<Snapshot> GetSnapshots()
{
    for (var i = 0; i < 9; i++)
    {
        yield return new Snapshot
        {
            Id = i + 1,
            ScenarioTimeInSeconds = new[] { 0, 0, 0, 60, 60, 60, 120, 120, 120 }[i],
            BearingInDegrees = new[] { 0, 120, 220, 0, 120, 220, 0, 120, 220 }[i],
            RangeInNauticalMiles = new[] { 0, 1, 1, 0, 1, 1, 0, 1, 1 }[i],
            CourseInDegrees = new[] { 90, 90, 90, 90, 90, 180, 90, 90, 90 }[i],
            SpeedInKnots = new[] { 7, 7, 7, 7, 7, 7, 7, 7, 7 }[i],
            XCoordinate = new[] { 0, 0, 0, 0, 0, 0, 0, 0, 0 }[i],
            YCoordinate = new[] { 0, 0, 0, 0, 0, 0, 0, 0, 0 }[i]
        };
    }
}

This seems a lot more elegant than my original.

As far as I can see, most of your data is fairly predictable.  What about:

for (int i = 0; i < 9; ++i)
{
   int scenarioTimeInSeconds = 60*(i/3); // Intentional loss of fractional part
   int bearingInDegrees = (i % 3) * 120; // It's now 0, 120, 240
   int rangeInNauticalMiles = (i % 3 == 0) ? 0 : 1;
   int courseInDegrees = (i == 5) ? 180 : 90;
   yield return new Snapshot
       {
           Id = i + 1,
           ScenarioTimeInSeconds = scenarioTimeInSeconds,
           BearingInDegrees = bearingInDegrees,
           RangeInNauticalMiles = rangeInNauticalMiles,
           CourseInDegrees = courseInDegrees,
           SpeedInKnots = 7,
           XCoordinate = 0,
           YCoordinate = 0,
      };
}

This already seems more readable to me.  Now you can split the logic that makes the values for the various local ints off into separate functions and document each one.  In order to do it this way, you do have to have GetSnapshots return an IEnumerable<Snapshot>, but that makes sense anyway -- and if you really want them as an array, just call ToArray() on it outside the function (i.e. GetSnapshots().ToArray()).

Even if you'd rather not do that, you should still change numbers that are used multiple times (in your case: 60, 90, 120, 180, 220) into named constants, unless they're entirely unrelated.


Serving a growing audio file with Django I would like to serve a growing audio file (raw PCM audio for now) with the possibility of seeking. I tried to manage it with Apache, but in the end wasn't able to achieve what I wanted.

Now I managed to come up with a solution within Django that seems to work quite nicely. Basically I created a view that delivers a part of the file and then sends a response with status code 206 and Content-Range header.

### DJANGO VIEW

MAX_CHUNK_SIZE = 32000
HEADER_PATH = "/path/to/header.wav"
MAX_WAV_LENGTH = 4294967303
BYTES_PER_SECOND = 32000

def get_audio(request, file_name):

    # get offset
    pos = int(request.GET.get("pos", 0))
    offset = pos * BYTES_PER_SECOND

    # get range
    r = request.META["HTTP_RANGE"]
    start = int(r.replace("bytes=", "").split("-")[0])
    start_o = start + offset

    # in case this is the first request, a wave header is added
    if start == 0:
        with open(HEADER_PATH, "rb") as f:
            data = f.read()
        length = 44
    else:
        data = ""
        length = 0
        start_o -= 44

    file_path = join(settings.STREAMS_DIR, file_name)

    # wait up to 10 seconds if the end of the file is reached
    # (in case it grows more)
    for _ in range(10):
        size = getsize(file_path)
        if size - start_o > 0:
            break
        else:
            sleep(1)

    length += min(MAX_CHUNK_SIZE, size - start_o)

    # if the length is zero (if end of file is reached and there is
    # nothing more to server), set the total length to the actual
    # served length
    if length == 0:
        total = size - offset + 44
    # else, set the total length to the maximum possible wav length
    # so that the browsers know that it has to send subsequent requests
    else:
        total = MAX_WAV_LENGTH

    # get the actual data from the raw audio file
    with open(file_path, "rb") as f:
        f.seek(start_o)
        data += f.read(length)

    # send response with use of Content-Range
    resp = HttpResponse(data, content_type="audio/x-wav", status=206)
    resp["Content-Range"] = "bytes " + str(start) + "-" + str(start + length) + "/" + str(total)
    return resp

In the browser, I can then simply request my audio like in the following way, to make it play from any point:

var fileName = "xyz.wav";
var start = 25; //seconds
audio = new Audio();
audio.src = "/getAudio/" + fileName + "?pos=" + start;
audio.play();

As I said, this all seems to work pretty well. But I would like to know this is a valid approach that I can use in production or if there are some major problems that I am not aware of.

For large constants consider writing them as an expression and
possibly comment on the size, just in case, to make it easier to
understand.
I'm not particularly fond of waiting for the file to grow, but I guess
if it works it's fine.
Serving static files is implemented in Django, including HTTP range
parsing.  However
the one method
I found only works in debug mode.  Still, I'd try to reuse
django.utils.http.parse_http_range and
django.http.response.PartialHttpResponse if possible, so that not
all of the data is read into memory at once.

Otherwise looks okay I'd say.