Class-Definition Comparison Whenever I needed something similar like a "Class" in JavaScript, I wrote it like this:

function Point(x, y){
    var self = this;
    self.x = x;
    self.y = y;
    self.dist = function(){
        return Math.sqrt( (self.x*self.x) + (self.y*self.y) );
    }
}

var p = new Point(10, 20);
p.x = 30;
alert(p.dist);

I declared a function and wrote the constructor directly into it. Public variables and methods have the self.-operator in front of it.

Now I took a look at TypeScript, which declares the exact same function this way:

var Point = ( function(){
    function Point(x, y) {
        this.x = x;
        this.y = y;
    }
    Point.prototype.dist = function(){
        return Math.sqrt( (this.x*this.x) + (this.y*this.y) );
    }
    return Point;
})();

var p = new Point(10, 20);
p.x = 30;
alert(p.dist());

This definition makes use of a closure, and uses prototype.

My Questions:

What are the advantages/disadvantages of the 2 definitions?
Which method is more efficient?
Which method should I prefer to use (when I don't use TypeScript)?

Using prototype is useful for polymorphism. If you want to create another class, eg. ColourfulPoint, you can use this code:

ColourfulPoint.prototype = Object.create(Point.prototype);

to make your new class inherit all the methods of Point.
The first method (using only a constructor) is a way faster (at least in Firefox and Chrome) than the prototype one. Take a look at the test.
Unless you intend to create polymorphic classes, use the first one.


AsyncTask and IntentService I was disappointed using GCM, so I'll use long polling for crucial parts. Following my approach:

protected void onCreate(Bundle savedInstanceState) {
    new LongPolling().execute();
}

private class LongPolling extends AsyncTask<String, Void, String> {

    @Override
    protected String doInBackground(String... params) {
        HttpClient hclient = new DefaultHttpClient();
        HttpPost httpr = new HttpPost(new URL("http://10.0.2.2/page.php").toURI());
        HttpResponse response = hclient.execute(httpr);
        String result = EntityUtils.toString(response.getEntity());
        return result;
    }

    @Override
    protected void onPostExecute(String result) {
        new LongPolling().execute();
    }

}

Is it the right approach for the long polling, or do you want to suggest a better approach? Also, a IntentService starts when you turn on the phone to run that code, would be better to use a Service?

About long polling

To clarify about long polling, from wikipedia:

  With long polling, the client requests information from the server exactly as in normal polling, except it issues its HTTP/S requests (polls) at a much slower frequency. If the server does not have any information available for the client when the poll is received, instead of sending an empty response, the server holds the request open and waits for response information to become available. Once it does, the server immediately sends an HTTP/S response to the client, completing the open HTTP/S Request. In this way the usual response latency (the time between when the information first becomes available and the next client request) otherwise associated with polling clients is eliminated.

In particular, paraphrasing the key points:

The client issues its requests at a slow frequency: this is not true in your implementation, you poll the server in sequence without an explicit delay in between requests. But maybe you're ok with that.
When the information is not available at the time of the request, the server holds the request open, and responds when the information is ready: this is to implement on the server side, of course, and not visible in the posted code.

About your main questions

  Is it the right approach for the long polling, or do you want to suggest a better approach? Also, a IntentService starts when you turn on the phone to run that code, would be better to use a Service?

An AsyncTask calling itself repeatedly... sounds a bit strange.
Also, the doc says this:

  AsyncTasks should ideally be used for short operations (a few seconds at the most.) If you need to keep threads running for long periods of time, it is highly recommended you use the various APIs provided by the java.util.concurrent pacakge such as Executor, ThreadPoolExecutor and FutureTask.

Although a single polling action is short,
your desired behavior is repeated polling for presumably a long time (you did not include a termination condition).
As such, an IntentService sounds like a better approach:

  In most cases an IntentService is the preferred way to simple background operations.

Code review

Although following the recommendations above you might well reimplement using an IntentService,
the original code has some interesting issues worth reviewing:

The AsyncTask is defined to return a String, but it's not used at all.
You can get rid of that, and then you won't need the EntityUtils.toString call anymore
The code in your post doesn't catch exception,
even though some should be thrown,
for example IOException during HTTP requests.
You should add some error handling for that.
Instead of hardcoding the URL inside the class,
it would be better to make it a parameter (initialize in the constructor).

Something like this:

private class LongPolling extends AsyncTask<String, Void, Void> {
    private final URI uri;

    private LongPolling(URI uri) {
        this.uri = uri;
    }

    @Override
    protected Void doInBackground(String... params) {
        try {
            HttpClient httpClient = new DefaultHttpClient();
            HttpPost httpPost = new HttpPost(uri);
            httpClient.execute(httpPost);
        } catch (IOException e) {
            // server down? maybe you should let the client know!
        }
        return null;
    }

    @Override
    protected void onPostExecute(Void aVoid) {
        new LongPolling(uri).execute();
    }
}


Get the indices of a range elements by relative order of the values I've just asked

What's the idiomatic way to get the sort order of data, without actually applying it?

(or see the original it was found to be a dupe on). So, I want to implement properly what seems to be the only suggested option - generating an array of indices and sorting it by the original data's relative order. Here's my code (ignoring includes):

template<typename RAIterator, typename Comparator, typename Size = size_t>
inline std::vector<Size>
sorted_indices(RAIterator first, RAIterator last, Comparator comparator)
{
    std::vector<Size> result(last - first);
    std::iota(result.begin(), result.end(), 0);
    auto adapted_comparator = [&first, comparator](Size lhs, Size rhs) {
        return comparator(first[lhs], first[rhs]);
    };
    std::sort(result.begin(), result.end(), adapted_comparator);
    return result;
}

template<typename RAIterator, typename Size = size_t>
inline std::vector<Size>
sorted_indices(RAIterator first, RAIterator last)
{
    using value_type = typename std::iterator_traits<RAIterator>::value_type;
    auto comparator = [](const value_type& lhs, const value_type& rhs) { return lhs < rhs; };
    return sorted_indices<RAIterator, decltype(comparator), Size>(first, last, comparator);
}

template<typename Container, typename Comparator, typename Size = typename Container::size_type>
inline std::vector<Size> sorted_indices(Container& container, Comparator comparator)
{
    return sorted_indices<typename Container::const_iterator, Size>(
        std::begin(container), std::end(container), comparator);
}

template<typename Container, typename Size = typename Container::size_type>
inline std::vector<Size> sorted_indices(Container& container)
{
    return sorted_indices<typename Container::const_iterator, Size>(
        std::begin(container), std::end(container));
}

Have I covered all my bases or is there something I might be missing? Also, how legitimate is it to return the vector of indices?

Two thing I noticed I can do are:

use std::less<T> as a default comparator class
set a default value for the comparator parameter, using the default initializer of the comparator class

The combination of these two cuts code length by half without reducing legibility (!):

template<
    typename RAIterator,
    typename Size = size_t,
    typename Compare = std::less<typename std::iterator_traits<RAIterator>::value_type>
>
inline std::vector<Size>
sorted_indices(RAIterator first, RAIterator last, Compare compare = Compare())
{
    std::vector<Size> result(last - first);
    std::iota(result.begin(), result.end(), (Size) 0);
    auto lookup_and_compare = [&first, compare](Size lhs, Size rhs) {
        return compare(first[lhs], first[rhs]);
    };
    std::sort(result.begin(), result.end(), lookup_and_compare);
    return result;
}

template<
    typename Container,
    typename Size = typename Container::size_type,
    typename Compare = std::less<typename Container::value_type>
>
inline std::vector<Size> sorted_indices(Container& container, Compare compare = Compare())
{
    return sorted_indices<typename Container::const_iterator, Size>(
        std::begin(container), std::end(container), compare);
}

The downside is that you must then depend on <iterator> and <functional>; since I use them already for other things, I don't really mind.


Implementation of tree with different node types and faux-polymorphism in C I'm currently learning C by working on my first project.
It's a calculator with a parser that transforms input into an operator tree. These trees consist of nodes of different types: Operators (inner nodes), constants and variables. My first attempt was to create a single struct node and just put everything a Node might have in it -- which wastes a lot of memory because not all of the fields are used, depending on the actual type a Node has.

My solution:
I implemented three different structs for the three different Node types.

typedef enum
{
    NTYPE_OPERATOR,
    NTYPE_CONSTANT,
    NTYPE_VARIABLE
} NodeType;

typedef NodeType* Node;

struct VariableNode_
{
    NodeType type;
    char var_name[];
};

struct ConstantNode_
{
    NodeType type;
    double const_value;
};

struct OperatorNode_
{
    NodeType type;
    Operator *op;
    size_t num_children;
    Node children[];
};

They all have a type as their first field -- a header to let me know which type a Node actually has. A Node is always a pointer to the heap, so I typedef'ed it to hide its pointer type. A node may be created and used by the following "constructors" and accessors:

Node malloc_variable_node(char *var_name)
{
    VariableNode res = malloc(sizeof(struct VariableNode_) + (strlen(var_name) + 1) * sizeof(char));
    if (res == NULL) return NULL;
    res->type = NTYPE_VARIABLE;
    strcpy(res->var_name, var_name);
    return (Node)res;
}

Node malloc_constant_node(ConstantType value)
{
    ConstantNode res = malloc(sizeof(struct ConstantNode_));
    if (res == NULL) return NULL;
    res->type = NTYPE_CONSTANT;
    res->const_value = value;
    return (Node)res;
}

Node malloc_operator_node(Operator *op, size_t num_children)
{
    if (num_children > MAX_ARITY)
    {
        // Max. arity exceeded
        return NULL;
    }

    OperatorNode res = malloc(sizeof(struct OperatorNode_) + num_children * sizeof(Node));
    if (res == NULL) return NULL;

    for (size_t i = 0; i < num_children; i++)
    {
        res->children[i] = NULL;
    }

    res->type = NTYPE_OPERATOR;
    res->op = op;
    res->num_children = num_children;
    return (Node)res;
}

void free_tree(Node tree)
{
    if (tree == NULL) return;
    if (get_type(tree) == NTYPE_OPERATOR)
    {
        for (size_t i = 0; i < get_num_children(tree); i++)
        {
            free_tree(get_child(tree, i));
        }
    }
    free(tree);
}

NodeType get_type(Node node)
{
    return *node;
}

Operator *get_op(Node node)
{
    return ((OperatorNode)node)->op;
}

size_t get_num_children(Node node)
{
    return ((OperatorNode)node)->num_children;
}

Node get_child(Node node, size_t index)
{
    return ((OperatorNode)node)->children[index];
}

Node *get_child_addr(Node node, size_t index)
{
    return &((OperatorNode)node)->children[index];
}

void set_child(Node node, size_t index, Node child)
{
    ((OperatorNode)node)->children[index] = child;
}

char *get_var_name(Node node)
{
    return ((VariableNode)node)->var_name;
}

double get_const_value(Node node)
{
    return ((ConstantNode)node)->const_value;
}

Is this common practice? Would there be a better way to do it? Would it have been okay to just waste the space and use a single struct as I did before? This would almost always safe me a level of indirection when dealing with trees and transforming them because all Nodes would have the same size and could be recycled.
If you want to look at the full file(s): node.h hosted at GitHub

Thanks in advance :)

Edit: Changed ConstantType to double because the type was not defined in the excerpt I posted

Remarks for beginners:

Never hide a pointer behind a typedef! Never! It makes the code confusing and very hard to read and maintain.
You can't unfortunately use a generic Node pointer like in your code, because this isn't well-defined by the language. There are ways around it, but they are a bit controversial (see advanced reply below).
Generally, when doing any kind of polymorphism, you might want to have an "abstract base class". In this case it would have been NodeType, but it should be a struct of its own and each inherited class should contain an instance of the base class as first member. Not a pointer to it.
Avoid calling functions in the controlling statement of a for loop like you do in for (size_t i = 0; i < get_num_children(tree); i++). This will often produce needlessly slow code. Instead call the function once before the loop, then store the result in a variable.

Remarks for veterans (beginners ignore):

One advanced trick which is supposedly fully supported by the standard (but might have diverse quality of implementation on different compilers) is the "union common initial sequence" trick.

There's a rule stating that if you expose a union containing all those structs to the same translation unit as where those structs are used, type punning between the structs is well-defined. In your case something like this:

union type_punning
{
  struct VariableNode_ a;
  struct ConstantNode_ b;
  struct OperatorNode_ c;
};

This union just needs to sit there; you don't need to use it, or even show it to the caller. This tells the compiler that since these struct share a common initial sequence, the NodeType member, we can type pun between them and it is well-defined, as long as we only access shared members of that initial sequence.

Meaning that suddenly we can do this:

void func (struct VariableNode_* vn)
{
  if(vn->type == NTYPE_CONSTANT)
  {
    struct ConstantNode_* cn = (struct ConstantNode_*)vn;
    cn->const_value = something; // well-defined!
  }
}

There's also exceptions to the strict aliasing rule allowing this use.

Source: C17 6.5.2.3

  One special guarantee is made in order to simplify the use of unions: if a union contains
  several structures that share a common initial sequence (see below), and if the union
  object currently contains one of these structures, it is permitted to inspect the common
  initial part of any of them anywhere that a declaration of the completed type of the union
  is visible. Two structures share a common initial sequence if corresponding members
  have compatible types (and, for bit-fields, the same widths) for a sequence of one or more
  initial members.


excel vba udf: entire column as fn parameter I have a function that accepts a range as an input. It works fine when I supply the function with a range like a1:a5, but not when I use an entire column like a:a.

I could always loop over the whole column (ie. a:a) but that is so slow.

Here is a simple example.

Function r_sum(r1 As Range) As Long

    For Each c In r1.Cells
        r_sum = r_sum + c.Value
    Next c

End Function

How can I supply my functions with ranges (ie. a:a or 2:2, entire columns or rows) and have the function execute efficiently? How do excel's worksheet functions do it so well?

The method I used above (ie. looping over every cell in the range) is so slow that I can't possibly imagine using such a technique, especially since many of the functions I have currently are complex and will crash the program.

What you have there is equivalent to =SUM(A:A) - which runs much faster as a native Excel function. Always use Native Excel in Excel itself before resorting to VBA.

As a general note: if you are going to do something bespoke - convert the range to an array and work with the array. This has a significant performance improvement for a number of reasons which have been explained many times on this site. The key ones are that you are not switching between the Excel model and the VBA model each loop and that you can work with data types instead of objects.


Calculating template page numbers I have a function which sets the text to a label displaying current active "products" and current active pages. The label displays "X products, Y pages/page" depending on the amount of products active in the template. Products is the same as rows really, the client wants to call it products.

The template consists of a number of tags, activeTags, which holds the tags that are not deleted. One row consists of a number of tags, perhaps 2 or 3, in the property _selectedTemplateConfiguration.TemplateConfigurationTagsPerRow. Every template page can hold a number of products/rows, _selectedTemplateConfiguration.TemplateConfigurationRowsPerPage.

Let's say I have 41 rows and the template takes 20 rows on each page. This means I have 3 pages active. When I made the function I could not get it to work without the use of the divisionRestvariable, ie. when I had 40 rows it still said it was 3 active pages, due to Math.Ceiling.

The _currentTagCollectionconsists really of a number of Telerik RadDocks with a text input, the idea with the whole template is that the client can enter textvalues into a template online, which is the printed.

I probably missed something, but this is a function that I feel could be made different. Feel free to ask if I need to clarify anything. I'm using .NET 4.0.

private string displayTemplateNumbers()
{
  int totalPagesActive;
  var activeTags = _currentTagCollection.Count - totalAmountOfDeletedTags();

  var divisionRest = (Convert.ToDouble(activeTags / _selectedTemplateConfiguration.TemplateConfigurationTagsPerRow) % _selectedTemplateConfiguration.TemplateConfigurationRowsPerPage);

  if (divisionRest == 0) //Math.Ceiling thinks it's 3 pages when it's only 2, have to check divisionRest like this
  {
       totalPagesActive = Convert.ToInt32((activeTags / _selectedTemplateConfiguration.TemplateConfigurationRowsPerPage) / _selectedTemplateConfiguration.TemplateConfigurationTagsPerRow);
  }
  else
  {
       totalPagesActive = Convert.ToInt32(Math.Ceiling((Convert.ToDouble(activeTags) / Convert.ToDouble(_selectedTemplateConfiguration.TemplateConfigurationRowsPerPage)) / Convert.ToDouble(_selectedTemplateConfiguration.TemplateConfigurationTagsPerRow)).ToString());
  }
  var pages =  totalPagesActive > 1 ? string.Format("{0} pages", totalPagesActive) : string.Format("{0} page", 1);
  return string.Format("{0} products, {1}", activeTags / _selectedTemplateConfiguration.TemplateConfigurationTagsPerRow, pages);
}

private int totalAmountOfDeletedTags()
{
 int counter = 0;
 foreach (var tag in _currentTagCollection)
 {
   if (tag.Deleted)
   {
      counter++;
   }
 }
 return counter;
}

Your method names do not follow capitalization standards, they should be PascalCase.

However, what's worse is that their names do not correspond to that they do:

displayTemplateNumbers() returns a formatted string (and thus doesn't "display" anything),
totalAmountOfDeletedTags() feels like it should be a property instead of a method.

And totalAmountOfDeletedTags() isn't even necessary, since it can be replaced by simple LINQ: _currentTagCollection.Count(x => x.Deleted);. And since you only need the active tags, you can simply do var activeTags = _currentTagCollection.Count(x => !x.Deleted); (h/t to @RobertSnyder).

At the end of displayTemplateNumbers() you twice call string.Format(). Instead you should do this once:

return string.Format("{0} products, {1} page{2}",
    activeTags / _selectedTemplateConfiguration.TemplateConfigurationTagsPerRow,
    totalPagesActive,
    (totalPagesActive > 1 ? "s" : string.Empty));


Program that generates random IQ numbers and multiplies them by a user-entered multiplier My code is working well and uses a lot of mechanics in C++, however, I want to know if there are any ways to make my code better, some possible problems in use. this is a simple program and I started yesterday.

What other mechanics could I implement to make the code better? And where can I find more information about those mechanics?

// IQ rand generator by ashar rahman

#include "stdafx.h"         //includes rand and other related commands
#include <stdlib.h>         //something windows wants
#include <iostream>         //allows for c++ IO interaction
#include <time.h>           //allows for srand time manipulation
#include <cstdlib>          //standard c++ libraryu

using namespace std;        //tells compiler to use std for all namespaces ex. std::cout --> cout

class IQClass{              //class containing all IQ generating, name and multiplyer selection related functions.
public:

    int y;                  //random IQ storage integer
    long int a;             //answer after  multiplyer storage integer, long for larger numerical answers
    int m;                  //multiplyer container
    char name[80];          //char array container for user name supporting upto 80 bytes

    void nameInput() {      //function including input prompt for user name
        cout << "What is your name?\n\n";       //prints 'What is your name?' with cout \n are line returns
            cin >> name;                        //allows user to input name with cin
    };

    void multiplierSelection() {        //function contains input prompt for multiplier
        cout << "\nWhat should the multiplier be?\n\n ";                    //prints
            cin >> m;                                                           //asks for input to fill m variable
    }

    void calc() {   //function contains equasion to find answer of y*m
        a = y*m;                                        //equasion
    }

    void print() {                      //function contains printing of answer, random IQ, multiplyer, and name, endl is another line return type
        cout << name << "'s IQ is probably: " << y << endl << "That times:     " << m << " is " << a << endl;
    }

    void randGen() {            //creates random sequence
        y = rand() % 160;           //out of 160 finds random value
    };

private:

};

    IQClass IC;                 //changes identifer for IQCLass ex.              IQClass.nameInput(); ---> IC.nameInput

int main()
{
     srand(time(NULL));             //generates random seed once for rand to use

     IC.nameInput();                                            //allows user to input name
     IC.multiplierSelection();                              //allows user to select multiplier

        for (int x = 0; x < 100;) {             //prints 100 random IQs for user and the answers relative to the multiplier
            IC.randGen();                               //generates random sequence
            IC.calc();                      //calculates product
            IC.print();                     //prints answer(s)
         x = x + 1 ;
     };
    return 0;           //ends program with return code of 0
}

As you can see I have annotated everything for maximum explanation and so I can review/remember it if I forget.  Are any of the comments wrong and if so, how can they be improved?

You don't need comments explaining the standard library headers - credit your readers with enough knowledge to know what those lines are for.

Most of the other comments could be removed (in some cases, we can use better names for variables such as y, a and m to communicate the meaning instead).

There's nothing to stop overrun of the name variable - prefer to use std::string instead, as it will automatically resize as required.

Avoid using namespace std - importing all names of a namespace is a bad habit to get into, and can cause surprise when names like begin and size are in the global namespace.  Get used to using the namespace prefix (std is intentionally very short), or importing just the names you need into the smallest reasonable scope.

IC doesn't need to be a global variable - it can be a local to main().

IQ scores are usually defined as a Normal distribution of mean 100 and standard deviation 15.  You can use a std::normal_distribution (from <random>) to generate values that fit.


Weak Goldbach Conjecture in Rust This is a program to find solutions to Goldbach's weak conjecture. I've tried to optimise the solution function by never looking at primes that are larger than the remaining sum, but it feels clunky. Any ideas?

extern crate primal;

use std::env;

fn get_input() -> usize {
    env::args()
        .nth(1)
        .expect("Single positive integer expected as argument")
        .parse::<usize>()
        .expect("Single positive integer expected as argument")
}

fn list_primes_until(input: &usize) -> Vec<usize> {
    let sieve = primal::Sieve::new(input.clone());
    return sieve.primes_from(0).take_while(|x| x <= input).collect();
}

fn get_goldbach_solution(input: &usize) -> Option<[usize; 3]> {
    let first_primes = list_primes_until(&input);

    for first_prime in first_primes {
        if input - first_prime < 1 {
            continue;
        }

        let second_primes = list_primes_until(&(input - first_prime));
        for second_prime in second_primes {
            if input - first_prime - second_prime < 1 {
                continue;
            }

            let third_primes = list_primes_until(&(input - first_prime - second_prime));
            for third_prime in third_primes {
                if &(first_prime + second_prime + third_prime) == input {
                    return Some([first_prime, second_prime, third_prime]);
                }
            }
        }
    }

    return None;
}

fn main() {
    let input = get_input();

    let solution = get_goldbach_solution(&input).expect("No solution found for input");
    println!(
        "{} = {} + {} + {}",
        input,
        solution[0],
        solution[1],
        solution[2]
    );
}

Pass by value
There is little point passing integrals by reference:

They are Copy, meaning that they can be copied implicitly, so there is no usability penalty to having the caller by pass value,
They are cheap to copy, so there is no performance penalty either. Actually... it may actually perform better.

It's cheaper and easier so pass by value!
This will get rid for the weird &(first_prime + second_prime + third_prime) == input thing.
Expression oriented language
Rust is an expression oriented language. In short, it means that practically everything is an expression.
Notably, a block {} is an expression which evaluates to the value of its last expression (or () if its ends with a statement).
Therefore, the body of a function is an expression, you can simple use None at the end instead of return None;.
Avoid recomputing primes over and over
At the moment, the complexity of your program is horrendous because you keep recomputing the primes over and over. This is unnecessary.
Instead, you can compute the primes up until input once, and then filter those primes.
First Draft
Applying the two above strategies already cleans up the code quite a bit:
fn get_goldbach_solution(input: usize) -> Option<[usize; 3]> {
    let primes = list_primes_until(input);

    for &first_prime in &primes {
        let remainder = input - first_prime;

        for &second_prime in primes.iter().take_while(|p| **p < remainder) {
            let remainder = remainder - second_prime;

            for &third_prime in primes.iter().take_while(|p| **p <= remainder) {
                if third_prime == remainder {
                    return Some([first_prime, second_prime, third_prime]);
                }
            }
        }
    }

    None
}

The last step however is not, in terms of algorithmic complexity, as efficient as it could be. We are iterating over primes until finding one which equals remainder. Since primes is sorted, a simple binary_search would be more algorithmically palatable; although in practice, for small numbers, linear iteration might be faster.
Result
fn get_goldbach_solution(input: usize) -> Option<[usize; 3]> {
    let primes = list_primes_until(input);

    for &first_prime in &primes {
        let remainder = input - first_prime;

        for &second_prime in primes.iter().take_while(|p| **p < remainder) {
            let third_candidate = remainder - second_prime;

            if let Ok(_) = primes.binary_search(&third_candidate) {
                return Some([first_prime, second_prime, third_candidate]);
            }
        }
    }

    None
}

It might be more efficient to start from the highest primes, as it reduces the size of the list to iterate further.
I'm not quite sure about it; you can test it using .rev() on the iterators to reverse them.


Lazy properties for the angle and length of a line segment The code below shows the pattern I use to manage properties in my classes when I have the following requirements:

Some properties are slow to calculate and are rarely used, so I want them to be lazy
Some methods change the value of a property

The properties are defined the first time they are requested and are deleted when the methods are executed. Calculating the length in the Line class shown in the example is fairly fast, but calculating the Length of a shape made of many Lines would be slow and must be avoided unless required. The full code include many more properties and the list in _reset_attributes([...]) can have dozens of items.

Is this the correct/pythonic/fast way of managing properties?

class Line:
    @property
    def angle(self):
        try:
            return self._angle
        except AttributeError:
            self._angle = math.atan2(self.end.y - self.start.y, self.end.x - self.start.x)
            return self._angle

    @property
    def length(self):
        try:
            return self._length
        except AttributeError:
            self._length = math.sqrt((self.end.x - self.start.x) ** 2 + (self.end.y - self.start.y) ** 2)
            return self._length

    def rotate(self, pivot_point, angle):
        self.start.rotate(pivot_point, angle)
        self.end.rotate(pivot_point, angle)
        self._reset_attributes(['_angle'])

    def scale(self, pivot_point, factor):
        self.start.scale(pivot_point, factor)
        self.end.scale(pivot_point, factor)
        self._reset_attributes(['_length'])

    def _reset_attributes(self, attributes):
        for attribute in attributes:
            try:
                delattr(self, attribute)
            except:
                pass

You can create a custom decorator to cache the values, with the help of this decorator we can also remove the unnecessary repetitive if-else or try-except checks present in each of the properties. Hence, the methods will only contain the required code.

The trick here is to use each method's name and store an additional attribute on the instance itself with _ prefixed to it. So, when we look for a value we are first going to check if it is present on that instance, if yes then return the value otherwise call the function and store the value on the instance and return it.

def cached_property(func):
    attr = '_' + func.__name__

    class Descriptor(object):
        def __get__(self, ins, type):
            if hasattr(ins, attr):
                return getattr(ins, attr)
            else:
                value = func(ins)
                setattr(ins, attr, value)
                return value
    return Descriptor()

class Test(object):

    def __init__(self, name):
        self.name = name

    @cached_property
    def find_max(self):
        print ('property max of {!s} accessed'.format(self))
        lst = range(10**5)
        return max(lst)

    @cached_property
    def find_min(self):
        print ('property min of {!s} accessed'.format(self))
        lst = range(10**5, -1, -1)
        return min(lst)

    def __str__(self):
        return self.name

if __name__ == '__main__':
    t1 = Test('t1')
    t2 = Test('t2')
    print(t1.find_max)
    print(t1.find_min)
    print(t2.find_max)
    print(t2.find_min)
    print(t1.find_max,
    t1.find_min,
    t2.find_max,
    t2.find_min)

    t1.remove_value('_find_max')
    t2.remove_value('_find_min')

    print(t1.find_max)
    print(t1.find_min)
    print(t2.find_max)
    print(t2.find_min)

Output:

property max of t1 accessed
99999
property min of t1 accessed
0
property max of t2 accessed
99999
property min of t2 accessed
0
99999 0 99999 0
property max of t1 accessed
99999
0
99999
property min of t2 accessed
0

For removing a single value I would recommend defining __delete__ method on the descriptor itself:

def cached_property(func):
    attr = '_' + func.__name__

    class Descriptor(object):
        def __get__(self, ins, type):
            if hasattr(ins, attr):
                return getattr(ins, attr)
            else:
                value = func(ins)
                setattr(ins, attr, value)
                return value

        def __delete__(self, ins):
            try:
                delattr(ins, attr)
            except AttributeError:
                pass

    return Descriptor()

Now these two calls:

t1.remove_value('_find_max')
t2.remove_value('_find_min')

Can be replaced with:

del t1.find_max
del t2.find_min


Trim to the nearest word under limit Given a text and the length limit I need to trim the text to the word which is nearest to the limit.

function trim(text, limit) {
  if (text.length < limit) return text;

  let end = -1;
  var i;
  for (i = 0; i < limit - 1; i++) {
    let current = text.charAt(i);
    let next = text.charAt(i + 1);
    if (current !== ' ' && next === ' ') {
      end = i;
    }
  }
  if (i === text.length - 1) {
    end = i;
  }

  return text.substring(0, end + 1);
}

[0, 2, 4, 6, 8, 10, 12, 50].forEach((limit) => {
  console.log(trim('Hello World', limit));
});

O/P:

//

Hello
Hello
Hello
Hello World
Hello World

The current code has a bug when the limit is exactly is exactly the length of a word. For example,

trim('Hello World', 5)

will return an empty string, when it should return "Hello", since it is of length 5, therefore it fits in the length limit.

The issue stems from the loop over i going from 0 to limit - 1 (excluded). The variable end represents the index of the last character to keep in the string, (because the method is returning text.substring(0, end + 1)). Therefore, it could be the case that end is equal to limit - 1, this happens when the limit falls right before a space. For example, when the limit is 5, and the string is 'Hello World', the last character to keep is 'o' at index 4. So the loop should go up to limit (excluded).

Then, there are slight adjustements to make:

The early-return testing text.length < limit should be text.length <= limit (because otherwise the next character in the loop over i won't exist for the last character of the string);
The added check if (i === text.length - 1) can safely be removed as a consequence: the only case where i would be equal to text.length - 1 is if limit == text.length (since i goes up to limit - 1 included). But that was already handled with the modified early-return.

There is a simpler and efficient way to tackle this problem, using the built-in lastIndexOf(searchValue, fromIndex). This method starts at the specified fromIndex, and searches backwards the first occurence of the given value. If we tell it to search the first occurence of ' ' searching backwards from limit, we'll exactly have the length of the substring to return.

The only corner case is if limit is greater than the length of the string, in which case the method should simply return the text unchanged (like the current code is doing). It needs to be tested separately; otherwise lastIndexOf would start searching from the end of the string, and the last word would always be cut. This is what it could look like:

function trim(text, limit) {
  if (limit >= text.length) {
    return text;
  }
  return text.substring(0, text.lastIndexOf(' ', limit));
}

[0, 2, 4, 5, 6, 8, 10, 11, 12, 50].forEach((limit) => {
  console.log(limit + ' -> ' + trim('Hello World', limit));
});

Note that, even if the lastIndexOf call returns -1, because there was no space character found before the limit length, it is still safe to call substring since:

  If either argument is less than 0 [...], it is treated as if it were 0.


JavaScript Determine if Array has Almost Increasing Sequence A coding challenge on a website says to determine if an array would have an increasing sequence if one element were removed from that array.

Here is the function:

const almostIncreasingSequence = sequence => {
  const increasingSequence = arr => {
    for (let i = 0; i < arr.length; i++) {
      if (arr[i - 1] >= arr[i]) {
        return false;
      }
    }
    return true;
  };
  for (let i = 0; i < sequence.length; i++) {
    const left = sequence.slice(0, i);
    const right = sequence.slice(i + 1);
    const newArr = left.concat(right);
    if (increasingSequence(newArr)) {
      return true;
    }
  }
  return false;
};

The function produces the exact same output as all of the visible tests on the site:

var toTest = [
  [1, 3, 2, 1],
  [1, 3, 2],
  [1, 2, 1, 2],
  [1, 4, 10, 4, 2],
  [10, 1, 2, 3, 4, 5],
  [1, 1, 1, 2, 3],
  [0, -2, 5, 6],
  [1, 2, 3, 4, 5, 3, 5, 6],
  [40, 50, 60, 10, 20, 30],
  [1, 1],
  [1, 2, 5, 3, 5],
  [1, 2, 5, 5, 5],
  [10, 1, 2, 3, 4, 5, 6, 1],
  [1, 2, 3, 4, 3, 6],
  [1, 2, 3, 4, 99, 5, 6],
  [123, -17, -5, 1, 2, 3, 12, 43, 45],
  [3, 5, 67, 98, 3]
];

for (let i = 0; i < toTest.length; i++) {
  console.log(almostIncreasingSequence(toTest[i]));
}

Outputs:

false
true
false
false
true
false
true
false
false
true
true
false
false
true
true
true
true

But then a hidden error triggers upon submission telling me that the execution time is longer than the maximum of 4 seconds. How can I improve the time complexity of this algorithm?

Your code has complexity \$ O(N^2) \$ (with \$ N \$ being the array length), and also creates up to \$ N \$ temporary arrays.

Also note that your increasingSequence function accesses arr[-1], the
loop should start with let i = 1.

As already mentioned in the comments, the result can be
obtained with a single traversal of the array, thus reducing the complexity to \$ O(N) \$, and without additional storage.

The idea is to find the first element  violating the (strict) increasing
condition, i.e. the first index i with a[i - 1] >= a[i]. Then one of
a[i-1] or a[i] must be removed, and both cases can occur,
as the following examples show:

a = [ 4, 9, 5, 6, 7 ], i = 2  -->  remove a[i - 1] = 9
a = [ 4, 5, 1, 6, 7 ], i = 2  -->  remove a[i] = 1

Therefore we check if

a[i - 2], a[i - 1], a[i], a[i + 1]

can be made strictly increasing by removing one of the middle elements.

Since no more elements can be removed, the remaining elements, starting at index i + 1, must be strictly increasing.

There are also some shortcuts, e.g. if the array has at most 2 elements,
or if the “first decrease” is at the end of the array.

An implementation could look like this (it gives the correct result
in all your test cases):

const almostIncreasingSequence = a => {
    if (a.length <= 2) {
        return true;
    }

    // Find first i such that a[i - 1] >= a[i]:
    let i = 1;
    while (i < a.length - 1 && a[i - 1] < a[i]) {
        i++;
    }
    if (i >= a.length - 1) {
        return true; // Increasing if last element is removed.
    }

    // Can a[i - 1] be removed ?
    let case1 = (i == 1 || a[i - 2] < a[i]) && a[i] < a[i + 1];
    // Can a[i] be removed ?
    let case2 = a[i - 1] < a[i + 1];

    if (!case1 && !case2) {
        return false;
    }

    // Check that a[i + 1], ... is increasing.
    for (let j = i + 2; j < a.length; j++) {
        if (a[j - 1] >= a[j]) {
            return false;
        }
    }

    return true;
};

(JavaScript is not my first language, so take this just as a
suggestion, it surely can be implemented in a more idiomatic way.)


Productivity/Timer app that uses twilio to send SMS-reminders I wrote a timer app in python using both twilio and the timer module; sends an sms to my phone after a certain amount of time has passed. While it works without a doubt, I wanted to gather some ideas/opinions on how to improve my program for efficiency, readability, etc...

I'm open to all suggestions.

import time
from twilio.rest import Client

client = Client("KEY", "KEY")

work_time = 20 * 60
break_time = 5 * 60

def _timer(workState = True):
    messageBody = ["Time to work", "Time to break"]
    if workState == True:
        client.messages.create(to="MYNUMBER", from_="TWILIONUMBER", body="Time To Work!")
        time.sleep(work_time)
        _timer(not workState)
    else:
        client.messages.create(to="MYNUMBER", from_="TWILIONUMBER", body="Time To Break!")
        time.sleep(break_time)
        _timer(not workState)
_timer(False)

Please note: Several values have been changed for privacy sake (e.g. MYNUMBER, TWILIONUMBER, KEY)

Ways of improving:

work_time and break_time as constant values deserve to be defined as CONSTANTS
_timer function:

workState should be renamed to work_state to follow naming conventions
messageBody = ["Time to work", "Time to break"] is not actually used (can be eliminated)
if workState == True: is just a verbose version of if workState:
both conditional branches if/else have the same common statement _timer(not workState) but differ in action name work/break and delay time work_time/break_time.The whole condition can be restructured to reduce repetitive code and move the common statement(s) out

The final optimized version:

import time
from twilio.rest import Client

client = Client("KEY", "KEY")

WORK_TIME = 20 * 60
BREAK_TIME = 5 * 60

def _timer(work_state=True):
    if work_state:
        action, delay_time = 'work', WORK_TIME
    else:
        action, delay_time = 'break', BREAK_TIME

    client.messages.create(to="MYNUMBER", from_="TWILIONUMBER",
                           body=f"Time To {action.title()}!")
    time.sleep(delay_time)
    _timer(not work_state)

_timer(False)


Try and except clauses for appending elements in lists I have a list that can have up to three elements, and I want to append them to three list, because I get this list from another element within a loop. My final goal is to have a dataframe with the data of all elements. I have coded this:

affected_projects_1 = []
affected_projects_2 = []
affected_projects_3 = []

for issue in project:
    affected_projects = issue.affected_projects
    try:
        affected_projects_1.append(affected_projects[0])
    except IndexError:
        affected_projects_1.append('')
    except:
        raise Exception
    try:
        affected_projects_2.append(affected_projects[1])
    except IndexError:
        affected_projects_2.append('')
    except:
        raise Exception
    try:
         affected_projects_3.append(affected_projects[2])
    except IndexError:
        affected_projects_3.append('')
    except:
        raise Exception

 df = pd.DataFrame(zip(affected_projects_1, affected_projects_2, affected_projects_3))

This piece of code seems pretty verbose and almost repetitive (the only thing that changes is the number). How could I make it more compact? Is that even possible without losing readability or using antipatterns (i.e using globals variables). Thanks

except: raise Exception is awful.

It converts all errors to Exception.

This means errors that have nothing to do with Exception like BaseException and KeyboardInterupt are now something they semantically are not.
This also means if you have a KeyError then it's magically the same as any other error.

It doesn't even raise an exception instance, Exception().
There's no need for it, just remove it from your code.

If you weren't appending to different lists then we could see that all we need is a basic for loop.

for proj in issue.affected_projects[:3]:
    affected.append(proj)

Since you have to append to other lists you can zip over them.

for affected, proj in zip(affected_projects, issue.affected_projects):
    affected.append(proj)

Since you need to default to '' you can just extend issue.affected_projects to these values.

affected_projects = [[], [], []]
for issue in project:
    for affected, proj in zip(
        affected_projects,
        issue.affected_projects + 3*['']
    ):
        affected.append(proj)

We can remove the need for affected.append by moving the zip up one loop.

affected_projects = zip(*(
    (issue.affected_projects + 3*[''])[:3]
    for issue in project
))
pd.DataFrame(zip(*affected_projects))

Since you are ziping affected_projects and you're zipping issue.affected_projects you don't really need to zip at all.

df = pd.DataFrame(
    (issue.affected_projects + 3*[''])[:3]
    for issue in project
)


Chess Dictionary Validator (Python) (Automate The Boring Stuff) This is an exercise in the Automate The Boring Stuff book. I am supposed to create a function  that takes a dictionary argument that represents a chess board (ex: {'1h': 'bking', '6c': 'wqueen', '2g': 'bbishop', etc.}) and returns True or False depending on if the board is valid.
A valid board will have (A) exactly one black king and exactly one white king. (B) Each player can only have at most 16 pieces, (C) at most 8 pawns, and (D) all pieces must be on a valid space from '1a' to '8h'; that is, a piece can’t be on space '9z'. (E) The piece names begin with either a 'w' or 'b' to represent white or black, followed by 'pawn', 'knight', 'bishop', 'rook', 'queen', or 'king'. This function should detect when a bug has resulted in an improper chess board.
#Example dictionary of coordinates (keys) and pieces (values)
#my_board = {"1a":"wking", "2a":"bking", "8b":"wpawn", "3a":"wpawn"}

def isValidChessBoard(board):
    #Creating a dictionary of pieces (keys) and the number of pieces (values)
    num_pieces = {}
    list_pieces = ["bpawn", "bking", "bqueen", "brook", "bbishop", "bknight", "wpawn", "wking", "wqueen", "wrook", "wbishop", "wknight"]
    for v in board.values():
        num_pieces.setdefault(v, 0)
        num_pieces[v] = num_pieces[v] + 1
    for i in list_pieces:
        if i not in num_pieces:
            num_pieces.setdefault(i, 0)
    #Making sure there is exactly one king on each side
    if (num_pieces["wking"] != 1) or (num_pieces["bking"] != 1):
        return False
    #Making sure there are no more than 8 pawns on each side
    if ("bpawn" or "wpawn" in num_pieces) and (num_pieces["bpawn"] or num_pieces["wpawn"]) > 8:
        return False
    #Making sure every piece is apprpriately named (b/w + king/queen/rook/pawn/knight/bishop)
    for j in num_pieces:
        if j not in list_pieces:
            return False
    #Making sure there are no more than 16 pieces on each side
    if num_pieces["bpawn"] + num_pieces["bking"]  + num_pieces["bqueen"] +  num_pieces["brook"] +  num_pieces["bbishop"]  + num_pieces["bknight"] > 16:
        return False
    if num_pieces["wpawn"] + num_pieces["wking"]  + num_pieces["wqueen"] +  num_pieces["wrook"] +  num_pieces["wbishop"]  + num_pieces["wknight"] > 16:
        return False
    #Making sure that the board coordinates are all between (a-h, and 1-8)
    in_range = 0
    possible_coordinates = []
    for i in range(1, 9):
        for j in ["a", "b", "c", "d", "e", "f", "g", "h"]:
            possible_coordinates.append(str(i)+str(j))
    for k in board.keys():
        if k in possible_coordinates:
            in_range = in_range + 1
    if len(board) != in_range:
        return False
    return True

if isValidChessBoard(my_board) == True:
    print("Valid Board")
else:
    print("Invalid Board")

I know that the code works, but I was wondering if there were any faux-pas that I am implementing, or if there is anything I could obviously do to make it cleaner. Thanks!

As you said the code works - there might be additional conditions for a valid board, that you are not checking for which you will be able to expand on using the same structure. I looked up the chapter from the book you mention and your implementation meets the requirements outlined there.
Since this is Python, there are more than one way to implement the requirements, at a minimum I'd recommend refactoring the code to be self documenting instead of relying on comments. This will require only minor refactoring of your code.
Then building on that may be consider taking a functional approach and utilizing python builtins. YMMV - here is my implementation, it is not perfect, but that is not the intent here.
def checkCriteria(pieces, fn, cnt):
    if len(list(filter(fn, pieces))) > cnt:
        return False

    return True

def checkValidPositions(input_pos):
    positions = set()
    for col in ['a', 'b', 'c', 'd', 'e', 'f', 'g', 'h']:
        for row in range(1, 9):
            positions.add(str(row) + col)

    return all(pos in positions for pos in input_pos)

def checkMaxPiecesByColor(pieces):
    return all(checkCriteria(pieces, lambda piece: len(piece) > 0 and piece[0] == ch, 16) for ch in ['w', 'b'])

def checkKingCount(pieces):
    return all(checkCriteria(pieces, lambda piece: piece == king, 1) for king in ['bking', 'wking'])

def checkPawnCount(pieces):
    return all(checkCriteria(pieces, lambda piece: piece == pawn, 8) for pawn in ['bpawn', 'wpawn'])

def checkValidPieces(pieces):
    valid_pieces = set()
    for color in ['w', 'b']:
        for piece in ['pawn', 'knight', 'bishop', 'rook', 'queen', 'king']:
            valid_pieces.add(color + piece)

    return len(pieces - valid_pieces) == 0

def isValidChessBoard(board):
    pieces = board.values()
    positions = board.keys()

    if not checkValidPositions(positions):
        return False

    if not checkMaxPiecesByColor(pieces):
        return False

    if not checkKingCount(pieces):
        return False

    if not checkPawnCount(pieces):
        return False

    if not checkValidPieces(set(pieces) - set([''])):
        return False

    return True


CSV reader using StreamReader and LINQ Mostly due to readability, I am using the following code to

find a specific csv file on a drive
read that CSV file using a streamReader
parse it into a List<string[]> to be able to use LINQ and/or PLINQ later on

However, the process takes about 4-5 seconds, which is simply put too long. Any suggestions on how to improve the following (or maybe even replace it)?

var query = (from x in Directory.GetFiles(_path, "*.csv").AsParallel()
                     where x.Contains(dateTime)
                     select x).First();

        #region Read CSV File
        List<string[]> _tempList = new List<string[]>();

        FileStream stream = new FileStream(query, FileMode.Open, FileAccess.Read, FileShare.ReadWrite);
        using (StreamReader reader = new StreamReader(stream, Encoding.Default, true, 1024))
        {
            string currentLine;
            while ((currentLine = reader.ReadLine()) != null)
            {
                string[] temp = currentLine.Split(new char[] { ',' }, StringSplitOptions.None);
                _tempList.Add(temp);
            }
        }
        #endregion

The order inside the CSV file is important - the file will contain between (2000-7000) x 25 entries.

The CSV file has several fields which I am not interested in - and I don't necessarily need them in my List<string[]> (or string[][]).  I have tried to filter them with a LINQ statement similar to the following pseudo code:

var query = from x in MyListOfStringArray
            select new {Col1 = x[1] , Col2 = c[4]}.ToArray();

This approach results in a strange type for query - and for that matter was pretty slow (I would have an array with about 9-11 properties).  Any ideas on that second issue?

var query = (from x in Directory.GetFiles(_path, "*.csv").AsParallel()
                     where x.Contains(dateTime)
                     select x).First();

If Directory.GetFiles(_path, "*.csv") doesn't return any files where x.Contains(dateTime), this line will blow up with an InvalidOperationException that might be a bit off-putting / surprising.

I find the query would be clearer with a 100% method syntax - this would be equivalent:

var query = Directory.GetFiles(_path, "*.csv")
                     .AsParallel()
                     .Where(name => name.Contains(dateTime))
                     .First();

The name query is wrong/misleading. It would be a query if it were an IQueryable<string> (i.e. up to right after the .Where() call)... but it's not, var actually stands for string and what you call query is actually a fileName: .First() materializes the query, and returns the first element... or throws an exception.

It would be better to call .FirstOrDefault(), which would make your result null if no files matched your search criteria.

I don't understand why you're using var here, given that you're using explicit types everywhere else.. especially since var is just a string. Don't get me wrong though: I prefer seeing var here... and everywhere else.

#region Read CSV File

Using #region inside a method clearly says your method is doing more than one thing. Whenever you do that, you're missing a refactoring opportunity. Consider extracting a method here:

private string GetFileName(string formattedDateTime)
{
    return Directory.GetFiles(_path, "*.csv")
                    .AsParallel()
                    .Where(name => name.Contains(formattedDateTime))
                    .FirstOrDefault();
}

(lacking context to recommend passing in an actual DateTime)

That leaves you with a method that (hopefully) now only has this #region block, which is now completely redundant and can be discarded (if there's still more to it, extract methods as needed)

That said...

FileStream stream = new FileStream(query, FileMode.Open, FileAccess.Read, FileShare.ReadWrite);
using (StreamReader reader = new StreamReader(stream, Encoding.Default, true, 1024))

Disposing the StreamReader will also cleanly close and dispose the FileStream, but that is far from obvious unless you know it, and if Jon Skeet says it's best practice to put it in its own using block, then I can only agree with him.

Your code can use a variable (declared outside the loop's body) for the value separator: you're creating a new character array for every single row you're reading!

char[] separator = new char[] { ',' };

Or more succinctly:

var separator = new[] { ',' };

This makes the temp assignment even clearer/cleaner:

string[] temp = currentLine.Split(separator, StringSplitOptions.None);

Looking at the currentLine, it's far from obvious what's going on here:

while ((currentLine = reader.ReadLine()) != null)

Using the result of an assignment rarely results in easily readable code. I like how it cleverly combines reading the next line into currentLine and verifying whether that's a null value though... so I'm not sure I'd recommend rewriting that part after all.

It's not clear what happens to your List<string[]> when you're done reading the file, so I'll assume you're just returning it. In that case, you could consider yielding the results instead:

private IEnumerable<string[]> ReadCsv(string fileName)
{
    char[] separator = new[] { ',' };
    string currentLine;

    using (var stream = new FileStream(fileName, FileMode.Open, FileAccess.Read, FileShare.ReadWrite))
    using (var reader = new StreamReader(stream, Encoding.Default, true, 1024))
    {
        while ((currentLine = reader.ReadLine()) != null)
        {
            yield return currentLine.Split(separator, StringSplitOptions.None);
        }
    }
}


Bresenham-like line algorithm I implemented an algorithm that gives me a list of points that should be drawn in order the get a line between two points. Please note: it works only in a coordinate system whose origin (0,0) is in the upper left corner.

I'd like to get some review on my Haskell code, could it be improved in terms of the language constructs used?
Could it be more readable, more straight forward?

module DrawLine where

data Point = Point Float Float deriving (Show)

getRange :: Int -> Int -> [Int]
getRange a b
    | a <= b = [a..b]
    | otherwise = [b..a]
{-|
 - Get the slope of a line described by the two points received as arguments

-}
getSlope :: Point -> Point -> Float
getSlope (Point x1 y1) (Point x2 y2) = (y2-y1)/(x2-x1)

{-|
 - Get a list of points where pixels should be filled to draw a line
 - (Inspired from Bresenham's line algorithm)
 - Takes two Point arguments representing the end points of the line, the points
 - will be located on a line between these two points
 -}
getLinePoints :: Point -> Point -> [Point]
getLinePoints f@(Point fx fy) l@(Point lx ly)
    -- vertical line
    | fx == lx = [(Point fx (fromIntegral y)) | y <- getRange (round fy) (round ly)]
    | otherwise =
        -- generate points by mapping on the "longer side" of the line
        if m > 1.0 || m < -1.0 then
            -- the rise if higher than the run
            map (\y ->
                    (Point
                        ((1/m) * ((fromIntegral y) - fy) + fx)
                        (fromIntegral y)
                    )
                )
                (getRange (round fy) (round ly))
        else
            map (\x ->
                    (Point
                        (fromIntegral x)
                        (m * ((fromIntegral x) - fx) + fy)
                    )
                )
                (getRange (round fx) (round lx))
        where
            m = getSlope f l

Thoughts.

Define a Line type and rename getLinePoints to mkLine. A list of points may represent anything but a Line [Points] implies the Points within the line. Where possible let the type system work for you. Of course you could get cute and have Line (m -> x -> b -> [Points]) or List Point Point.

You could define Eq and Ord instances for the Point and maybe Line types.

What if getSlope is passed a vertical line? Should it perhaps use Maybe?

I'd move the non-vertical line support into its own function. Perhaps add a horizontal line function as well.

Why use a list comprehension for the vertical one and map for the other type of line? It can all be done with a comprehension.


Symlinking git-managed dotfiles into a home directory As a learning exercise, I decided to write a script for helping me maintain my Unix configuration/dot-files under a git repository. I check out a copy of the repository in my home directory called .dotfiles, and, inside that, I have the Python script called linkify, which deals with creating links from my home directory to the real files inside the .dotfiles directory.

For your reference, the respective code is in my GitHub repository.

#!/usr/bin/env python

import errno
import os

to_keep_private = [
    'cifs-credentials',
    'config',
    'fetchmailrc',
    'gitconfig',
    'gnupg',
    'mpoprc',
    'msmtprc',
    'mutt',
    'netrc',
    'offlineimaprc',
    'pim',
    'purple',
    'quodlibet',
    'ssh',
    ]

to_ignore = [
    '.',
    '.git',
    '.gitignore',
    'linkify',
]

def make_private(files=[]):
    if len(files) == 0:
        files = os.listdir('.')
    else:
        # Use the list of files passed by the user, if it is not empty
        pass
    for filename in files:
        if os.path.isdir(filename):
            os.chmod(filename, 0700)
        else:
            os.chmod(filename, 0600)

def linkify(files=[]):
    if len(files) == 0:
        files = os.listdir('.')
    else:
        # Use the list of files passed by the user, if it is not empty
        pass

    os.chdir(os.path.join(os.environ['HOME'], '.dotfiles'))
    for source in files:
        if source not in to_ignore:
            target = os.path.join('..', '.' + source)
            source = os.path.join('.dotfiles', source)
            try:
                os.unlink(target)
            except OSError, e:
                if e.errno == errno.ENOENT:
                    pass
                elif e.errno == errno.EISDIR:
                    print("%s is a directory. Ignoring." % target)
                else:
                    print errno.errorcode[e.errno]
            finally:
                os.symlink(source, target)

def add_hook():
    f = open('.git/hooks/post-commit', 'w')
    f.write('#!/bin/sh\ngit push\n')
    f.close()
    os.chmod('.git/hooks/post-commit', 0755)

def parse_cli():
    import optparse
    parser = optparse.OptionParser()

    parser.add_option('-H',
                      '--install-hook',
                      help='Install git hook for automatic push.',
                      action='store_true',
                      default=False)
    parser.add_option('-n',
                      '--no-linkify',
                      help='Disable creation of symlinks',
                      action='store_true',
                      default=False)
    parser.add_option('-P',
                      '--no-fix-perms',
                      help='Disable fix of permission for private files.',
                      action='store_true',
                      default=False)

    return parser.parse_args()

if __name__ == '__main__':
    opts, args = parse_cli()

    if not opts.no_linkify:
        linkify(args)
    if not opts.no_fix_perms:
        make_private(args)
    if opts.install_hook:
        add_hook()

Some guidelines to follow:

If possible, as it is in your case, work with absolute path.
You'll have at the beginning to do more or less something like:

dir = '/home/rik/'
files = [os.path.join(dir,file) for file in os.listdir(dir)]

But everything should be easier and straighforward later on.
You need to move the default behaviour - files = os.listdir('.') - one level up.
I can see one symptom and one reason for that:

You've wrote two times the exact same code.
The two different functions with that code should ignore what's your default directory, since they operate on list of files.

So, if the user does not provide his own custom list, you need to generate the list of files in . and pass it to both your functions. This also means that the functions should not have a default argument value.
If you can, use argparse instead of optparse.
I'm not sure from which version optparse was deprecated, but the main point is that if you have argpare in your standard library use that. You have only a couple of arguments so switching shouldn't take long.
Be consistent with your variable names.

def make_private(files=[]):
    # [...]
    for filename in files:
        # [...]

def linkify(files=[]):
    # [...]
    for source in files:
        # [...]

Why one time files is a list of filenames and another a list of sources?
If you strongly believe that one function works on filenames and the other on sources maybe you need to change the argument name, otherwise being consistent will help the readability of your code.
As Winston Ewert already told you, you should use capital letter for your constants names. Quoting from PEP8:

  Constants are usually defined on a module level and written in all
  capital letters with underscores separating words.  Examples include
  MAX_OVERFLOW and TOTAL.

Additional notes

Don't do this:

def function(lst=[]):
    # ...

Do this instead:

def function(lst=None):
    if lst is None:
        lst = []
    # ...

The reason for that is well explained here (or google it, because it's a very well known problem in which everyone stumbles sooner or later).

I'd say that by doing this you'll also remove the useless else, but as I explained it above you should remove the whole check and move it one level up against the user arguments.


Insert elements into a list I am a newbie trying to insert elements after the n:th element in the list. Any shorter/smarter ways?

def InsertElements(listName, var1, var2, n):

    listName.insert(n, var1)
    listName.insert(n+1, var2)
    return listName

myList = ["banana", "orange", "apple", "cherry", "grape"]

result = InsertElements(myList, "mango", "papaya", 3)

Many languages (e.g. JavaScript, Perl) have a splice function of some sort to modify list contents. They allow you to insert list items or change existing ones to new sublists.

Python uses a "slice" approach, allowing you to specify many list edits without a specific splice function.

>>> myList = ["banana", "orange", "apple", "cherry", "grape"]
>>> myList[3:3] = ["mango", "papaya"]
>>> myList
['banana', 'orange', 'apple', 'mango', 'papaya', 'cherry', 'grape']

By the way, if this is hard to understand, this cheat sheet may help:

 ["banana", "orange", "apple", "cherry", "grape"]

  0_______  1_______  2______  3_______  4______ 5    # item index
  0:1_____  1:2_____  2:3____  3:4_____  4:5____      # equiv slice notation

  ^         ^         ^        ^         ^       ^    # slice notation for
  0:0       1:1       2:2      3:3       4:4     5:5  # 0_length positions
                                                      # between items

  0:2________________ 2:5_______________________      # misc longer slice
            1:4_________________________              # examples
            1:5_________________________________
            1:__________________________________
  0:____________________________________________
  :_____________________________________________
  :3__________________________
  :1______

There are also negative indices (counting from the end of the list). Let's not get into those right now!


Best way to get the list of XML files I'm a newbie in the use of LinqToXml and I wonder if I can improve this snippet.

 Shared Function GetListByType(ByVal intIndex As Integer) As List(Of FileInfo)

            Dim strWorkingPath As DirectoryInfo = New DirectoryInfo(GetWorkspaceDir)
            Dim lstAllFiles As FileInfo() = strWorkingPath.GetFiles("*.xml", SearchOption.AllDirectories)
            Dim lstFiltredList As New List(Of FileInfo)
            Dim xelement As XElement
            Dim contratTypeIndex As String
            Dim contratVersion As Integer

            For Each fic In lstAllFiles
                xelement = xelement.Load(fic.FullName)
                contratTypeIndex = xelement.Element("TypeIndex").Value
                contratVersion = xelement.Element("Version").Value
                If (contratTypeIndex = intIndex.ToString And contratVersion = 0) Then
                    lstFiltredList.Add(fic)
                End If
            Next

            Return lstFiltredList.Distinct.ToList

        End Function

Option Strict

You should always set Option Strict = On.

Short circuit evaluation

You should use the AndAlso operator instead of the And operator. By using And both conditions will be evaluated. By using AndAlso the second condition will only be evaluated if the first returns true.

List(Of FileInfo)

Instead of returning a List(Of FileInfo) you should consider to return an IEnumerable(Of FileInfo).

Naming

You shouldn't prefix your variables like strWorkingPath or lstAllFiles.

GetFiles() vs EnumerateFiles()

From the docs

  The EnumerateFiles and GetFiles methods differ as follows: When you
  use EnumerateFiles, you can start enumerating the collection of names
  before the whole collection is returned; when you use GetFiles, you
  must wait for the whole array of names to be returned before you can
  access the array. Therefore, when you are working with many files and
  directories, EnumerateFiles can be more efficient.

Refactoring

First we will extract the loading of the file and reading the properties to separate methods and add a private class holding the TypeIndex and Version properties of the xml file.

Private Shared Function DoesFileMatch(file As FileInfo, index As String) As Boolean
    Dim version As XmlVersion = GetXmlVersion(file)
    Return (version.Version = "0" AndAlso version.TypeIndex = index)
End Function

Private Shared Function GetXmlVersion(file As FileInfo) As XmlVersion
    Return New XmlVersion(XElement.Load(file.FullName))
End Function

Private Class XmlVersion
    Public Property TypeIndex As String
    Public Property Version As String
    Public Sub New(element As XElement)
        TypeIndex = element.Element("TypeIndex").Value
        Version = element.Element("Version").Value
    End Sub
End Class

Next we add an overloaded GetListByType() method which takes a DirectoryInfo and a String as input parameter.

Shared Function GetListByType(directory As DirectoryInfo, ByVal typeIndex As String) As IEnumerable(Of FileInfo)

End Function

Which we call in our former method like

Shared Function GetListByType(ByVal typeIndex As Integer) As IEnumerable(Of FileInfo)
    Return GetListByType(typeIndex.ToString(), New DirectoryInfo(GetWorkspaceDir))
End Function

The overloaded method can then be simplified, using all above, to

Private Shared Function GetListByType(directory As DirectoryInfo, ByVal typeIndex As String) As IEnumerable(Of FileInfo)

    Dim filteredFiles As New List(Of FileInfo)

    For Each fic In directory.EnumerateFiles("*.xml", SearchOption.AllDirectories)
        If (DoesFileMatch(fic, typeIndex)) Then
            filteredFiles.Add(fic)
        End If
    Next

    Return filteredFiles.Distinct()

End Function

But wait, we can do this a lot more lambda like

Private Shared Function GetListByType(directory As DirectoryInfo, ByVal typeIndex As String) As IEnumerable(Of FileInfo)

    Return directory.EnumerateFiles("*.xml", SearchOption.AllDirectories) _
        .Where(Function(x) DoesFileMatch(x, typeIndex)) _
        .Distinct()

End Function


Extending numpy array by replacing each element with a matrix Suppose I have a H*W numpy array like this (H=3 and W=2):

[[0 1]
 [2 3]
 [4 5]]

I would like to replace each element with a matrix of M*N repeated element (M=2 and N=3):

[[[[0 0 0]
   [0 0 0]]

  [[1 1 1]
   [1 1 1]]]

 [[[2 2 2]
   [2 2 2]]

  [[3 3 3]
   [3 3 3]]]

 [[[4 4 4]
   [4 4 4]]

  [[5 5 5]
   [5 5 5]]]]

My current code first turn each element into a 2D matrix by calling expand_dims() twice, and then expand these matrices using repeat() twice too:

import numpy as np

H, W, M, N = 3, 2, 2, 3

array = np.arange(H * W).reshape(H, W)

array = np.expand_dims(array, axis=2)
array = np.expand_dims(array, axis=3)
array = np.repeat(array, M, axis=2)
array = np.repeat(array, N, axis=3)

Is there a more straightforward and elegant way to obtain the same result?


Creating valid words on an n-by-n grid of characters using depth-first search I am trying to create a word-finder game. There will be an n-by-n Board filled with Tiles.

class Tile:
    empty = None

    def __init__(self, letter):
        self.letter = letter
        #the dictionary tracks the Tiles that border a given Tile
        self.directionals = {'n':Tile.empty, 's':Tile.empty, 'w':Tile.empty, 'e':Tile.empty, 'nw':Tile.empty, 'ne':Tile.empty, 'sw':Tile.empty, 'se':Tile.empty}

class Board:
    def __init__(self, length, chars=''):
        self.length = length
        self.grid = [[] for _ in range(length)]

        for i in range(length):
            for _ in range(length):
                new_tile = Tile(randomchar()) #assume randomchar() returns a random character each time
                self.grid[i].append(new_tile)

        for i in range(length):
            for j in range(length):
                tile = self.grid[i][j]

                if i - 1 >= 0:
                    tile.directionals['n'] = self.grid[i - 1][j]
                if i + 1 < length:
                    tile.directionals['s'] = self.grid[i + 1][j]
                if j - 1 >= 0:
                    tile.directionals['w'] = self.grid[i][j - 1]
                if j + 1 < length:
                    tile.directionals['e'] = self.grid[i][j + 1]
                if i - 1 >= 0 and j - 1 >= 0:
                    tile.directionals['nw'] = self.grid[i - 1][j - 1]
                if i - 1 >= 0 and j + 1 < length:
                    tile.directionals['ne'] = self.grid[i - 1][j + 1]
                if i + 1 < length and j - 1 >= 0:
                    tile.directionals['sw'] = self.grid[i + 1][j - 1]
                if i + 1 < length and j + 1 < length:
                    tile.directionals['se'] = self.grid[i + 1][j + 1]

Starting from any Tile, the player will be able to create a word by traversing through unvisited Tiles. After successfully creating a word, the Tiles used to compose that word are reset. For example on a 4-by-4 Board:

   start         player makes 'some'   new Tiles represented by '*'
[[L A P S],         [[L A P S],          [[L A P S],
 [R O S E], ------>  [R O S  ],  ------>  [R O S *],
 [T I M E],          [T I   E],           [T I * E],
 [S O F T]]          [    F T]]           [* * F T]]

Throughout the game, I want to ensure the Board has a minimum number of words able to be made. I want to have the functionality to choose the reset Tiles such that they form words with neighboring Tiles, or else the number of available words on the Board may fall below the threshold.

To accomplish this, I implemented the below function.

def dfs(tile, word, min_word_length=2, max_word_length=5):
    visited = []

    def helper(tile, word):
        if len(word) > max_word_length:
            return
        if tile in visited:
            return
        #allwords is a list with all the words in the English dictionary
        if word in allwords and len(word) >= min_word_length:
            print(word)

        visited.append(tile)
        for direction in tile.directionals.values():
            if direction is not Tile.empty and direction not in visited:
                helper(direction, word + direction.letter)
        visited.remove(tile)

    helper(tile, word)

I would create Tiles with random letters for the Tiles needing to be reset, applying the dfs function after each Tile is plugged in to see if a valid word is formed. However, the current implementation is too inefficient: it struggles when max_word_length exceeds 5, exponentially more so on larger boards.

How can I change my implementation so that it'd be more efficient? Ideally, I would like the resetting to happen almost immediately (< 1 second). I was thinking of incorporating memoization, but am not sure how to in this case.

Obfuscation

Tile.empty is a long, verbose way of saying None.  Each time you use it, the Python interpreter must look up Tile in locals(), and then globals(), to find the Tile class object.  Then, it needs to look up empty in the Tile dictionary to find the value None.  If you simply used None, your code would be faster, because None is a keyword; no heroic efforts are needed by the Python interpreter to determine what this value is.

Dictionary of None values

From the minimal code which has been presented, there is no compelling reason to store each direction in self.directionals.  Instead of a direction key returning None (or Tile.empty), that direction could simply not exist in the dictionary.  The top-left corner would only contain directions "e", "se", and "s".  This would simply this code:

        for direction in tile.directionals.values():
            if direction is not Tile.empty and direction not in visited:
                ...

to this:

        for direction in tile.directionals.values():
            if direction not in visited:
                ...

One less check means less work, and faster performance.

Finally, since you keep asking for tile.directional.values(), you might consider storing these values in their own collection:

        tile.valid_directionals = set(tile.directionals.values())

or, if you must store all 8 directions:

        tile.valid_directionals = set(value for value in tile.directionals.values()
                                      if value is not Tile.empty)

and then using:

        for direction in tile.valid_directionals:
            if direction not in visited:
                ...

Using a set instead of a dict_values iterator may not be any faster, but I'll be taking advantage of it later.

List Comprehension

    self.grid = [[] for _ in range(length)]

    for i in range(length):
        for _ in range(length):
            new_tile = Tile(randomchar()) #assume randomchar() returns a random character each time
            self.grid[i].append(new_tile)

is a long, round about way of writing:

    self.grid = [[Tile(randomchar()) for _ in range(length)] for _ in range(length)]

Whenever you create a list, and then append to it in a loop, consider using list comprehension instead.  It is a powerful tool for your tool chest.

Simplify Tests

Consider i - 1 >= 0.  The Python Interpreter must lookup the value i, subtract 1 from it, which involves constructing a new int object, or since all the values are going to be small, looking up an interned version.  Then, the value is compared to zero, and discarded.  Consider the equivalent expression i >= 1, or even i > 0.  Look up the value and compare to a constant.  No subtraction operation required, no new integer object required for the result, and no discarding of the result of the subtraction after being used.  This is still in your executed once initialization code, so performance doesn't matter much, but why be sloppy?

Searching too deep

Your helper function looks approximately like:

    def helper(tile, word):
        if len(word) > max_word_length:
            return

        ...

        for direction in tile.directionals.values():
            if ...:
                helper(direction, word + direction.letter)

Imagine if we've called the helper a few times, and we've just recursed in with word="TRAPS".  We start looping around the tiles around the S, skipping the visited tiles, and call helper(direction, "TRAPS" + direction.letter) with direction.letter taking on the values 'O', 'I', 'M', 'E', 'E' and 'S'.  In each call, the first thing we do is check len(word) > max_word_length, and return if True.  With max_word_length = 5, every one of those calls will immediately return!  There is no point iterating to the len(word) == 6 depth!  If we checked for this earlier, we could eliminate many useless calls, and save time.

    def helper(tile, word):

        ...

        if len(word) < max_word_length:
            for direction in tile.directionals.values():
                if ...:
                    helper(direction, word + direction.letter)

Redundancy

Furthermore:

def helper(tile, word):
    ...
    if tile in visited:
        return
    ...
    for direction in tile.directionals.values():
        if direction not in visited:
            helper(direction, word + direction.letter)

You test direction not in visited, and only if that is true will you call helper(direction, ...).  In that recursive call, the tile in visited test will therefore never be true.  You can remove that redundant check for better performance.

Containment Testing

    #allwords is a list with all the words in the English dictionary
    if word in allwords and len(word) >= min_word_length:
        ...

First of all, English can contain some really long words, like "tetraiodophenolphthalein".  We should shorten this dictionary to words which are max_word_length and less.

possible_words = [word for word in allwords
                  if min_word_length <= len(word) <= max_word_length]

With this, my dictionary size is reduced from 235,886 words down to 17,082.  And a smaller dictionary will speed things up.

But what will speed things up more is getting rid of that list.  Testing if a word is in a list is an \$O(n)\$ operation.  If the words were stored in a set, this reduces search complexity to an \$O(1)\$ operation.

possible_words = set(word for word in allwords
                     if min_word_length <= len(word) <= max_word_length)

Then, containment testing is easy:

    if word in possible_words:
        ...

word in possible_words doesn't look much different from word in allwords, but the speed-up going from x in list to x in set is quite dramatic.

What about the and len(word) >= min_word_length condition?  Where did it go?  possible_words only contains words which are at least min_word_length, so we get that for free!

Pruning the Search Tree

Alright, I'm finally where I actually wanted to be to answer this question.  The fun part.

How many words in the English dictionary start with "RL"?  What about "MS" or "FM" or "TF"?  Plenty of words start with "SM" but no words start with `"SMF".

As you are collecting the tile letters into a candidate word, you test whether you have discovered a word, but not whether you've reached a combination which will not start any word.  Doing so will prune you search tree, and avoid checking around \$6^3\$ candidate words starting with "RL".  Less searching, yields faster performance!

So, how do we prune?

Consider the word "SOME".  If we start with "S", we have a possible word.  If we start with "SO", we have a possible word.  If we start with "SOM", we have a possible word.  And if we start with "SOME" we have a possible word.  So for each word in our possible_words, we just need to collect all the possible prefixes for the word into a set.

valid_prefixes = set(word[:size]
                     for word in possible_words for size in range(1, length(word))

With this set of valid prefixes, if our candidate word fragment is not a valid prefix, we can skip deeper searching of that branch:

    for direction in tile.directionals.values():
        if direction not in visited:
            candidate = word + direction.letter
            if candidate in valid_prefixes:
                helper(direction, candidate)

The tile.valid_directionals set

Almost forgot.  visited is a list.  If we switch this into a set, then we can use set arithmetic to get a set of valid_directionals that haven't been visited with valid_directionals - visited

def dfs(tile, word, min_word_length=2, max_word_length=5):
    visited = set()

    def helper(tile, word):
        if word in possible_words:
            print(word)

        if len(word) < max_word_length:
            visited.add(tile)
            for direction in tile.valid_directionals - visited:
                candidate = word + direction.letter
                if candidate in valid_prefixes:
                    helper(direction, candidate)
            visited.remove(tile)

    helper(tile, word)


Ordering data by boolean I have a bit of EF Lambda code which returns a list of data from the db table, then orders by a field called IsDefault

At first, the code was

var listOfData = Db
    .TableName
    .Where(u => u.UserId == userId)
    .OrderBy(u => u.IsDefault)
    .ToList();

Which when writing down, sounds correct. However is wrong as this will order by 0 > 1, and True = 1.

So I changed the statement to;

var listOfData = Db
    .TableName
    .Where(u => u.UserId == userId)
    .OrderBy(u => u.IsDefault ? 0 : 1)
    .ToList();

However, I could have also written this 2 other ways.

.OrderBy(u => !u.IsDefault)

OR

.OrderByDescending(u => u.IsDefault)

Now in my mind, u.IsDefault ? 0 : 1 reads better, and the other two could be misread as the not wanting the default value first.

What is your view on this?

I wouldn't use either ordering on the server side because it looks like the ordering only matters for displaying the data. I cannot imagine why it should matter from the programmatic point of view.

Let the client, its view, its display control, or whatever sort the items according to its needs.


Issue tokens concurrently with synchronization per client I have an API application with token authorization. A token is valid for one hour. Each business client has its own instance of the API application:

{"client_1": "http://localhost:1"},
...
{"client_n": "http://localhost:n"}

In another application I have a class ApiService to make HTTP requests to the API. I create one instance of ApiService. Requests happen concurrently and I want to be sure that for each client token will be issued only once. I use Flurl to make HTTP requests.

public class TokenDetails
{
    public string Token { get; set; }
    public DateTime Expires { get; set; }
}

public class ClientDetails
{
    public string ClientId { get; set; }
    public string BaseAddress { get; set; }
    public string Username { get; set; }
    public string Password { get; set; }
}

public class ClientService
{
    private readonly ConcurrentDictionary<string, Task<TokenDetails>> _tokens =
        new ConcurrentDictionary<string, Task<TokenDetails>>();

    private readonly ConcurrentDictionary<string, object> _locks =
        new ConcurrentDictionary<string, object>();

    public async Task<string> GetDataAsync(ClientDetails client)
    {
        var token = await GetTokenAsync(client).ConfigureAwait(false);

        return await new Url(client.BaseAddress).AppendPathSegment("/data")
            .WithOAuthBearerToken(token.Token)
            .GetStringAsync().ConfigureAwait(false);
    }

    private Task<TokenDetails> GetTokenAsync(ClientDetails client)
    {
        //try find valid token task
        if (!_tokens.TryGetValue(client.ClientId, out var tokenTask)
            || tokenTask.IsFaulted
            || tokenTask.IsCanceled
            || (tokenTask.IsCompleted && tokenTask.Result.Expires < DateTime.UtcNow))
        {
            //token task not found or token is expired - enter lock to make http call
            var lockObject = _locks.GetOrAdd(client.ClientId, id => new object());
            lock (lockObject)
            {
                //try find valid token task again after aquiring lock
                if (!_tokens.TryGetValue(client.ClientId, out tokenTask)
                    || tokenTask.IsFaulted
                    || tokenTask.IsCanceled
                    || (tokenTask.IsCompleted && tokenTask.Result.Expires < DateTime.UtcNow))
                {
                    //http call
                    tokenTask = new Url(client.BaseAddress).AppendPathSegment("/login")
                        .WithBasicAuth(client.Username, client.Password)
                        .GetJsonAsync<TokenDetails>();
                    _tokens.AddOrUpdate(client.ClientId, key => tokenTask, (key, oldToken) => tokenTask);
                }

            }
        }

        return tokenTask;
    }
}

The pattern looks correct. One disadvantage here is you're blocking threads waiting for the lock. You could instead use a SemaphoreSlim and its WaitAsync method for a nonblocking lock.

Even better, have a look at LazyCache. (Scott Hanselman blogged about this recently.) It basically does all of this for you - caching, asynchronous double-check locking, and guaranteeing refresh only happens once. You could replace most of what you've done here with a GetTokenAsync implementation that looks something like this:

return await _tokenCache.GetOrAddAsync(
    $"tokens-{client.ClientId}",
    () => client.BaseAddress
        .AppendPathSegment("/login")
        .WithBasicAuth(client.Username, client.Password)
        .GetJsonAsync<TokenDetails>(),
    TimeSpan.FromHours(1));

One little caveat here is you need to know ahead of time how long to cache the token, i.e. you can't use the Expires value returned from the server. But in your case it sounds like you know it's an hour, so hopefully that's a non-issue.


Simple Graph in Python Just the beginning of graphs API in Python:

# Simple graph API in Python, implementation uses adjacent lists.
# Classes: Graph, Depth_first_search, Depth_first_paths
# Usage:
# Creating new graph: gr1 = Graph(v) - creates new graph with no edges and v vertices;

# Search object: gr2 = Depth_first_search(graph, vertex) - creates search object,
# gr2.marked_vertex(vertex) - returns true if given vertex is reachable from source(above)

# Path object: gr3 = Depth_first_paths(graph, vertex)- creates a new path object,
# gr3.has_path(vertex) - thee same as above
# gr3.path_to(vertex) - returns path from source vertex (to the given)

class Graph:
    """class graph"""
    def __init__(self, v_in):
        """constructor -  takes number of vertices and creates a graph
         with no edges (E = 0) and an empty adjacent lists of vertices"""
        self.V = v_in
        self.E = 0
        self.adj = []
        for i in range(v_in):
            self.adj.append([])

    def V(self):
        """returns number of vertices"""
        return self.V

    def E(self):
        """returns number of edges"""
        return self.E

    def add_edge(self, v, w):
        """void, adds an edge to the graph"""
        self.adj[v].append(w)
        self.adj[w].append(v)
        self.E += 1

    def adj_list(self, v):
        """returns the adjacency lists of the vertex v"""
        return self.adj[v]

    def __str__(self):
        """to string method, prints the graph"""
        s = str(self.V) + " vertices, " + str(self.E) + " edges\n"
        for v in range(self.V):
            s += str(v) + ": "
            for w in self.adj[v]:
                s += str(w) + " "
            s += "\n"
        return s

class Depth_first_search:
    """class depth forst search, creates an object,
    constructor takes graph and a vertex"""
    def __init__(self, gr_obj, v_obj):
        self.marked = [False] * gr_obj.V
    self.cnt = 0
    self.__dfs(gr_obj, v_obj)

    def __dfs(self, gr, v):
        """void depth first search, proceed recursively,
        mutates marked - marks the all possible to reach
         from given (v) vertices; also mutates cnt - number of visited vert"""
        self.marked[v] = True
        self.cnt += 1
        for w in gr.adj_list(v):
            if self.marked[w] == False:
                self.__dfs(gr, w)

    def marked_vertex(self, w):
        """returns True if given vertex (w) is reachable
        from vertex v"""
        return self.marked[w]

    def count(self):
        """returns number of visited verticles
        (from given in the constructor vertex)"""
        return self.cnt

class Depth_first_paths:
    """class depth first paths, solves
    single paths problem: given graph and a vertex (source vertex), find
    a path to another vertex."""

    def __init__(self, gr_obj, v_obj):
        self.marked = [False] * gr_obj.V
        self.edge_to = [0] * gr_obj.V
        self.s = v_obj
        self.__dfs(gr_obj, v_obj)

    def __dfs(self, gr, v):
        """void recursive depth first search, mutates array marked,
        mutates counter (cnt), and creates a path (filling an array     edge_to)"""
        self.marked[v] = True
        for w in gr.adj_list(v):
            if self.marked[w] == False:
                self.edge_to[w] = v
                self.__dfs(gr, w)

    def has_path(self, v):
    """returns true if there is a path from the source
    vertex to the given, else false"""
    return self.marked[v]

    def path_to(self, v):
        """returns path from source to the given vertex"""
        if self.has_path(v) == False:
            return None
        path = []
        x = v
        while x != self.s:
            path.insert(0, x)
            x = self.edge_to[x]
        path.insert(0, self.s)
        return path

I've used classes not function because there is no need for global variables. How do you think build the rest graph algorithms on it?

I've reviewed your code and I can make the following remarks. I'm only going to review the Graph class for now so lets start:

class Graph:
    """class graph"""

Here the comment is redundant. We know its a Graph and we know its a class. Also note for completeness I like to write class Graph(object)

def __init__(self, v_in):
    """constructor -  takes number of vertices and creates a graph
    with no edges (E = 0) and an empty adjacent lists of vertices"""
    self.V = v_in
    self.E = 0
    self.adj = []
    for i in range(v_in):
        self.adj.append([])

Again the comment is redundant We know its a constructor and what it initializes. However as you may notice the real problem here is not the comment but what it describes. Those V, E, v_in parameters are too short and do not mean anything. If I  were to read the method body and not the class name I wouldn't be able to understand that they denote Vertices and Edges. So a better name for them would be vertices, edges and input_vertices. Note that we use lowercase names for class properties.

def V(self):
    """returns number of vertices"""
    return self.V

def E(self):
    """returns number of edges"""
    return self.E

More redundant notes as we know they return something. However the method names are wrong. Why V and E? And why do they return a number? I would suspect to return a list or an Object that I can query. A better name would be again vertices and edges.

def add_edge(self, v, w):
    """void, adds an edge to the graph"""
    self.adj[v].append(w)
    self.adj[w].append(v)
    self.E += 1

def adj_list(self, v):
    """returns the adjacency lists of the vertex v"""
    return self.adj[v]

That's not too bad although I would remove the void remark and add a better explanation about the input assumptions. For example what happens if  do add_edge(set((1,2,3)), frozenset((4,5,6)))? Traceback error. So its better to specify that it assumes the input is integers.

def __str__(self):
    """to string method, prints the graph"""
    s = str(self.V) + " vertices, " + str(self.E) + " edges\n"
    for v in range(self.V):
        s += str(v) + ": "
        for w in self.adj[v]:
            s += str(w) + " "
        s += "\n"
    return s

Ok nothing wrong here. I would only rename s to output or response so that I understand the meaning.

In general I would say to try to keep the names consistent and meaningful so a reader will not have to guess whats going on.


Right justified table printer function A challenge in the beginner book "Automate the boring stuff":

#! Python3

# Table printer
# Takes a list of lists of strings and displays in right-justified table
# assumption is inner lists contain same number items
# each inner list must be printed as a column

the_list = [["a", "bbbbbbbbb", "tttt"],
            ["d", "eeee", "ggggggggggg"],
            ["g", "hhh", "kkkkkkkkkkkkkkkk"]]

def print_table(a_list):
    """
    Prints a 2d list as a right justified table where each column represents
    an inner list
    """
    rows = len(the_list[0])
    columns = len(the_list)
    column_widths = [0 for i in range(columns)]
    # find longest string in each inner lists(or column)
    for i in range(len(column_widths)):
        longest = 0
        for item in a_list[i]:
            if len(item) > longest:
                longest = len(item)
        column_widths[i] = longest

    # store strings in list
    print_row = ["" for i in range(rows)]
    # iterate over lists to produce strings with correct formatting
    for a in range(rows):
        for i in range(columns):
            print_row[a] = print_row[a] + a_list[i][a].rjust(column_widths[i]
                                                             + 4)
    for item in print_row:
        print(item)

print_table(the_list)

The way that you find rows and columns is good. You can't iterate through the lists so this approach is good.
However using them for your comprehensions is bad.

The name a_list is not descriptive, and quite poor.
table whilst generic is much better.

Your comprehensions and loops are actually rather poor.
Your comprehensions are used as if you are initialize an array in say C.
But this is Python, not C.

You should also use the builtins. The way you find the longest in a list can be changed to use max instead.
And if you use a comprehension to change all the strings to their length then you could make column_widths a comprehension.

column_widths = [max(len(cell) for cell in column) for column in table]

Your print row is also quite strange.
Instead of print_row[a] = print_row[a] + you should use print_row[a] += but this can be done in a list comprehension too.
As you're joining the items by nothing you can 'wrap' the comprehension with ''.join.
And the comprehension would just be the indexing the table and justifying you're already doing.
This can result in:

print_row = ["" for i in range(rows)]
for row in range(rows):
    print_row[row] = ''.join(
        table[col][row].rjust(column_widths[col])
        for col in range(columns)
    )

for item in print_row:
    print(item)

From this you should notice that there is not much point in adding to a list, if you're just going to print it later anyway.
And so you can merge these two loops together and remove the print_row comprehension.

Also I'd change the rjust call to not add 4 spaces, as that indents the table, which looks kinda ugly.
And you can add a space between each column by using ' '.join instead.
This can result in the function:

def print_table(table):
    rows, columns = len(table[0]), len(table)

    column_widths = [max(len(cell) for cell in column) for column in table]
    for row in range(rows):
        print(' '.join(table[col][row].rjust(column_widths[col]) for col in range(columns)))