Export running data from Garmin API to CSV I'm interested in getting better. Current script does the job but I'm really unsure regarding

Quality of overall structure & comments
Error handling
Possibility the add testing (what and how should I test?)

Here is the script:

# This script runs through your Garmin Connect activities (New->Old) and saves them as CSV
import json
import csv
import requests
from http.cookies import SimpleCookie

# To get started open garmin connect in Google Chrome, log in and grab the id of your latest activity as in https://connect.garmin.com/modern/activity/<START_ID>
START_ID = 2399797019 # Paste it here
END_ID = 0 # Id to stop at if you've run this script before, leave 0 otherwise

# Then, press F12, click on "Network", press F5, scroll to the very first element, it will have your <START_ID> as name. Click on it and copy the whole value of "Cookie" in Headers > Request Headers
RAW_DATA = 'SESSION=63497e03-bcbc-4...'

cookie = SimpleCookie()
cookie.load(RAW_DATA)
cookies = {}
for key, morsel in cookie.items():
    cookies[key] = morsel.value

# Crawl politely, leave a way for them to contact you
headers = {
    'User-Agent': 'Some hobbyist runner and pythonista trying to bulk extract CSVs, get in touch if you have an issue with this',
    'From': 'you@domain.com'
}

def get_previous_activity_id(activity_id):
    url = 'https://connect.garmin.com/modern/proxy/activity-service/activity/{}/relative'.format(activity_id)
    r = requests.get(url, headers=headers, cookies=cookies)
    try:
        previous_activity_id =  r.json()["previousActivityId"]
        return(previous_activity_id)
    except KeyError:
        print('Issue getting the key ["previousActivityId"] for activity with id {}'.format(activity_id))
        return KeyError

def export_activity_laps(activity_id):
    url = 'https://connect.garmin.com/modern/proxy/download-service/export/csv/activity/{}'.format(activity_id)
    r = requests.get(url, headers=headers, cookies=cookies)
    try:
        with open('data/activities/{}.csv'.format(activity_id), 'w') as temp_file:
            temp_file.writelines(r.content.decode('utf-8'))
        return True
    except Exception as e:
        print('Issue writing the csv file for activity {}'.format(activity_id))
        return e

def export(activity_id):
    previous_id = get_previous_activity_id(activity_id)
    export_activity_laps(activity_id)
    if previous_id and previous_id != END_ID :
        export(previous_id)
    else:
        print("Finished exporting")

def main():
    export(START_ID)

if __name__ == '__main__':
    main()

Interesting script! Here are a few comments to make it better :)

Documentation

Documenting how to find the required values is a very nice touch, exactly the kind of things that helps a new user or maybe yourself in a few months. However, other parts of the documentation can be improved. Indeed, there is a Convention for Python docstrings. It is definitly worth a read. Among other things, your script documentation should be in """triple-quoted string""". Also, you should try to document your functions : purpose and when relevant parameters and return value (I'll come back to this).

Style

Not much to say about the style, your code looks good and seems to follow PEP 8. A few lines seem "too" long, maybe having the URLs as global constant would make things easier to read, maybe splitting the long literal strings on multiple lines could help as well.

Return value

What your functions return seems a bit unclear which probably makes them hard to use properly.

get_previous_activity_id return an activity_id... or KeyError.
export_activity_laps return True... or an exception.

Maybe, get_previous_activity_id should return None when it fails (or throw an exception depending on how expected this can be).

Maybe, export_activity_laps should not return anything at all and just throw in case of error.

Avoidable recursion

export relies on a recursion to iterate over the different activities.
Python does not handle deep recursions very well because it does not perform Tail Recursion Optimisation. If needed, you can rewrite the function:

def export(activity_id):
    while True:
        previous_id = get_previous_activity_id(activity_id)
        export_activity_laps(activity_id)
        if not previous_id or previous_id = END_ID :
            break
    print("Finished exporting")

This version, just like yours, does not check the validity of the initial activity_id. This may be a desirable check. Then, you'd write something like:

def export(activity_id):
    while activity_id and activity_id != END_ID:
        export_activity_laps(activity_id)
        activity_id = get_previous_activity_id(activity_id)
    print("Finished exporting")

If you want to be a bit fancier and think you might use the logic retrieving the activity identifiers one after the other for a different usage, you could extract it in a generator:

def get_activity_identifiers(activity_id):
    while activity_id and activity_id != END_ID:
        yield activity_id
        activity_id = get_previous_activity_id(activity_id)

def export(activity_id):
    for activity_id in get_activity_identifiers(activity_id):
        export_activity_laps(activity_id)
    print("Finished exporting")

Parameters

It could make sense for export_activity_laps not to be responsible for chosing the file path. This could be provided as a parameter.

Disclaimer: I have run (pun intended) none of the code above.


Replacing XML tag values This function replaces some XML tag values. The tags appear in the XML exactly once.I want to know if this can be done in a better way.

sub editConnConfig{
my ($DOMAIN,$rscDir,$walletDir,$DEFAULTPORT)=@_;
`perl -pi -e 's!<trustStore>.*?</trustStore>!<trustStore>$walletDir/fctrust.jks</trustStore>!sg' "$rscDir/ConnectionConfig.xml" 2>&1`;
`perl -pi -e 's!<keyStore>.*?</keyStore>!<keyStore>$walletDir/fckey.jks</keyStore>!sg' "$rscDir/ConnectionConfig.xml"`;
`perl -pi -e 's!<keyStorePassword>.*?</keyStorePassword>!<keyStorePassword>jkspass1</keyStorePassword>!sg' "$rscDir/ConnectionConfig.xml"`;
`perl -pi -e 's!<directoryName>.*?</directoryName>!<directoryName>$DOMAIN</directoryName>!sg' "$rscDir/ConnectionConfig.xml"`;
`perl -pi -e 's!<dbDaemonPort>.*?</dbDaemonPort>!<dbDaemonPort>$DEFAULTPORT</dbDaemonPort>!sg' "$rscDir/ConnectionConfig.xml"`;
}

First and foremost, XML is not a regular language, and therefore parsing it using regular expressions is not appropriate, and is asking for trouble.

Secondly, invoking perl within perl is clearly an awful idea. Invoking it multiple times is even worse.

As all the s!!! operations are performed on the same file, you could perform all of them at once in the same process, by separating them with semicolons, that is:

perl -pi -e 's!pattern1!replacement1!sg; s!pattern2!replacement2!sg' file

But as I said earlier, you should not call perl from within a perl function in a backtick expansion, but implement properly the operations (open file, read buffer, perform replacements, overwrite the file).

And instead of doing this using regular expressions, consider using an XML parser.


Variant implementation in C++ I've been fiddling with templates and compile time stuff i implemented a variant container and wanted some opinions and recommendations this is just a learning exercise, a couple template classes not included here are tl_type_at which gives the type in a typelist at a given index; and tl_index_of which gives the index of the first occurrence of the given type in a typelist:

template <size_t Index, typename T, typename ... Ts>
struct variant_deleter
{
    variant_deleter<Index + 1, Ts...> next;
    void operator()(size_t HeldIndex, void* HeldPtr) const
    {
        if(Index == HeldIndex)
            delete static_cast<T*>(HeldPtr);
        else
            next(HeldIndex, HeldPtr);
    }
};

template <size_t Index, typename T>
struct variant_deleter<Index, T>
{
    void operator()(size_t HeldIndex, void* HeldPtr) const
    {
        if(Index == HeldIndex)
            delete static_cast<T*>(HeldPtr);
        }
};

struct bad_variant_access {};

template <typename T, typename ... Ts>
struct variant
{
    typedef typelist<T, Ts...> types;

    variant()
        : union_(new T())
        , held_(0)
    {}
    template <typename I>
    variant(I Init)
        : union_(new typename tl_type_at< tl_index_of<I, types>::index, types >::type())
        , held_(tl_index_of<I, types>::index)
    {}
    ~variant()
    { variant<T, Ts...>::deleter_(held_, union_); }

    template <typename P>
    variant<T, Ts...>& operator=(const P& rhs)
    {
        union_ = new(union_) P(rhs);
        held_ = tl_index_of<P, types>::index;
        return *this;
    }

    template <typename G>
    G& get()
    {
        if(held_ == tl_index_of<G, types>::index)
            return *static_cast<G*>(union_);
        else
            throw bad_variant_access {};
    }
private:
    void* union_;
    size_t held_;
    static const variant_deleter<0, T, Ts...> deleter_;
};

Some issues:

the assignment operator doesn't call the destructor of the existing data, before setting a new value (results in UB, likely a resource leak).
by default you do not instantiate an empty variant with the default constructor; the first type in the union may not even be default-constructible.
bad_variant_access should probably inherit from a standard exception (std::runtime_error or std::logic_error are common as base classes).


Calculating large powers of 2 in decimal So, this is my problem to solve:

I want to calculate \$2^n\$ where \$0 < n< 10000\$

I am representing each element of array as a space where 4digit number should be "living" and if extra digit appears, I am replacing it to the next element of this array.

The principle I am using looks like this:

The code I am using is the following:

static string NotEfficient(int power)
{
if (power < 0)
    throw new Exception("Power shouldn't be negative");
if (power == 0)
    return "1";
if (power == 1)
    return "2";

int[] A = new int[3750];

int current4Digit = 0;

//at first 2 is written in first element of array
A[current4Digit] = 2;

int currentPower = 1;

while (currentPower < power)
{
    //multiply every 4digit by 2
    for (int i = 0; i <= current4Digit; i++)
    {
        A[i] *= 2;
    }

    currentPower++;

    //checking every 4digit if it
    //contains 5 digit and if yes remove and
    //put it in next 4digit
    for (int i = 0; i <= current4Digit; i++)
    {
        if (A[i] / 10000 > 0)
        {
            int more = A[i] / 10000;
            A[i] = A[i] % 10000;
            A[i + 1] += more;

            //if new digit should be opened
            if (i + 1 > current4Digit)
            {
                current4Digit++;
            }

        }

    }
}

//getting data from array to generate answer
string answer = "";

for (int i = current4Digit; i >= 0; i--)
{
    answer += A[i].ToString().PadLeft(4,'0') + ",";
}

return return answer.TrimStart('0');
}

How I can make this algorithm better?

Here's how I would make your algorithm better.

First: your principle is sound. You are displaying a large binary number in base 10 by taking advantage of the fact that int already has a method that displays a 32 bit number in base 10.  Essentially your method is:

convert the number to base 10000, by making an int array "digits" from 0 to 9999.
Make a device which can double one of those numbers
Print the result after the given number of doublings.

This technique works, but your implementation could use some improvements. The improvements I would make are:

Go big!  Why base 10000?  You could do base 100000000 in an int no problem or base 1000000000000000000 in a long.  Let's do that.
You allocate a huge number of ints.  That's probably either way too many or way too few for the problem at hand.  Don't limit yourself. Make a solution that grows as you need it, not one that has an arbitrary limit.  Moreover: you make 3750 ints, but you could do the entire problem with only 167 longs.
Abstract away your operations into a type that represents the value
Refactor each part of the algorithm into a helper method that does one thing well.
Stop using mutable state. It's OK to burn a little memory. The garbage collector will deal.
Use a more efficient algorithm to calculate big powers.

How might we put these principles into practice? Let's try.

First off, I'm going to make a struct which represents a number encoded in base 1000000000000000000.  Its implementation will be a list of longs, but it could be any collection type. Lists are just convenient.

(Aside: Why a struct? The implementation is small; it's a wrapper around a single reference. The meaning of the struct is logically a value.  It is immutable.  So this is a good candidate for a value type.  We get the benefits of having a clean interface and implementation hiding, but we only pay the price of a single reference.)

struct Base1000000000000000000
{
    private List<long> digits;
    private Base1000000000000000000(List<long> digits) {
        this.digits = digits;
    }

Note that so far everything is private. I want this to have a very locked-down interface.  We've got to get this party started somewhere, so let's represent one:

  public static readonly Base1000000000000000000 One =
    new Base1000000000000000000(new List<long> { 1L });

You could do Zero similarly.

Now what does our display algorithm look like? That's a one-liner:

public override string ToString() =>
    string.Join("",
        ((IEnumerable<long>)digits)
          .Reverse()
          .Select(d => d.ToString().PadLeft(18, '0')));

Note that we're using the non-destructive reverse, not the destructive reverse on list.

The core of the algorithm is in our adder, which takes a number and returns its double:

  public Base1000000000000000000 DoubleIt()
  {
    // Note, _ separators in constants are new for C# 7.
    const long TheBase = 1_000_000_000_000_000_000;
    var result = new List<long>();
    result.Add(0);
    for (int i = 0; i < this.digits.Count; i += 1)
    {
      // Make sure we have storage.
      if (i == result.Count)
        result.Add(0);
      result[i] += this.digits[i] + this.digits[i];
      // We might have to carry.
      if (result[i] >= TheBase)
      {
        // Again, make sure we have room.
        if (i + 1 == result.Count)
          result.Add(0);
        result[i+1] = result[i] / TheBase;
        result[i] -= TheBase;
      }
    }
    // And we're done.
    return new Base1000000000000000000(result);
  }

Super.  Now the main algorithm is straightforward:

Base1000000000000000000 c = Base1000000000000000000.One;
for (int i = 0; i < n; ++i)
  c = c.DoubleIt();
return c.ToString();

Notice how when I extract each operation to its own method, the code becomes simpler and more elegant.

Now, how can you improve this further?

Exercise: (Easy) Implement operator+ on this type.
Exercise: (Pretty Easy) Implement operator- on this type, and represent negative numbers correctly.
Exercise: (Hard) Implement operator* on this type.
Exercise: (Very hard) Now that you have multiplication and addition, you can get to 2n much faster. Suppose you want to do 2384.  Start with One.  Double it.  Now you have 21.  Multiply it by itself -- square it.  Now you have 22.  Square it again to get 24. Square it again to get 28.  And so on, until you have them up to 2256.  Multiply 2256 by 2128 to get 2384, and you're done.  Your original algorithm does 384 doublings, but you can get the answer with one doubling and nine multiplies. Can you work out the algorithm for the optimal sequence of multiplications and additions?
Exercise (Very hard): Implement % and / on this type.
Exercise (Very very hard): Now do it all over again without using any built-in type for the math. (Obviously use string for the text.)  You are leveraging the ability of int or long to represent a bitfield that can be displayed in base 10.  What if you didn't have that?  Because someone had to write that code. The person who wrote that code didn't have it because it hadn't been written yet.  Suppose C# didn't have any integer type; how would you implement math from scratch?


Solution to Dummy Packet Assembler (Task from r/dailyprogrammer) I tried to solve the Packet Assembler task posted at r/dailyprogrammer.
The Question is this:

When a message is transmitted over the internet, it is split into multiple packets, each packet is transferred individually, and the packets are reassembled into the original message by the receiver. Because the internet exists in the real world, and because the real world can be messy, packets do not always arrive in the order in which they are sent. For today's challenge, your program must collect packets from stdin, assemble them in the correct order, and print the completed messages to stdout.
The point of reading from stdin is to simulate incoming packets. For the purposes of this challenge, assume there is a potentially unlimited number of packets. Your program should not depend on knowing how many packets there are in total. Simply sorting the input in its entirety would technically work, but defeats the purpose of this exercise.

Input Format is this:

Each line of input represents a single packet. Each line will be formatted as X Y Z some_text, where X Y and Z are positive integer and some_text is an arbitrary string. X represents the message ID (ie which message this packet is a part of). Y represents the packet ID (ie the index of this packet in the message) (packets are zero-indexed, so the first packet in a message will have Y=0, the last packet in a message will have Y=Z-1). Z represents the total number of packets in the message.

Here is the Python3 solution I did
from re import split

def add_id(packets, p_id, len):
    packets[p_id] = {}
    packets[p_id]["len"] = len
    packets[p_id]["packets"] = {}

    return packets

def add_packet(packets, p_id, p_num, p_len, msg):
    if p_id not in packets.keys():
        packets = add_id(packets, p_id, p_len)

    if p_num > packets[p_id]["len"]:
        return packets

    packets[p_id]["packets"][p_num] = msg

    return packets

def main():
    packets = {}
    while True:
        try:
            recv = split(r'\s+', input())

            packets = add_packet(packets, int(recv[0]), int(recv[1]),
                                int(recv[2]), " ".join(recv[3:]))
        except EOFError:
            break

    for p_id in sorted(packets):
        for packet in range(packets[p_id]["len"]):
            print("%d %d %d %s" %(p_id, packet, packets[p_id]["len"], packets[p_id]["packets"][packet]))

if __name__ == '__main__':
    main()

Please rate this solution and suggest some improvements that can be made?

I think this code is very straightforward and easy to read. Good job!

Some nitpicks are,

You have a few PEP8 violations none all that concerning.

Should add a second white line between imports and your functions
Line too long, at your print statement
Continuation line under-indented for visual indent
printf style formatting is recommended against

Code suggestions

I would use a namedtuple instead of a dictionary, since it is much cleaner looking and easier to use. When to use a namedtuple!

Using the above suggestions and adhering PEP8 I get the following.

from re import split
from collections import namedtuple

def get_messages():
    Message = namedtuple('Message', 'idx num length message')
    messages = []

    while True:
        try:
            recv = split(r'\s+', input())
            messages.append(Message(recv[0], recv[1], recv[2], ' '.join(recv[3:])))
        except EOFError:
            return messages

def print_messages(messages):
    for message in messages:
        print("{}  {} {} {}"
            .format(message.idx, message.num, message.length, message.message))

def main():
    messages = sorted(get_messages(), key=lambda x: (x.idx, x.num))
    print_messages(messages)

if __name__ == '__main__':
    main()


Creating API clients that are "async agnostic" Python3's async/await syntax is great, but it does create a divide between libraries which are async-based and those which are not. For example, boto3 (AWS API library) currently doesn't work with async. There is a separate project, aiobotocore, attempts to recreate some of this functionality in an async context.

I have been thinking for a while about how to create HTTP API clients which can be run async and sync contexts. I have come up with a strategy that involves creating a separation between the logic (preparation of requests, interpretation of responses), and the sending. The logic is implemented as a generator function, which yields out an object which represents the request, and receives back an object representing the response. This generator is "run" by a runner function, which does the actual sending, and may be sync or async.

You can imagine that if, for example, boto3 had been written this way, it would be easy to re-use the bulk of the code to make an async version, rather than having to do an async rewrite.

I have included a toy example below. I would welcome comments.

from typing import Iterable, NamedTuple, Dict, Any, Optional

import requests
from aiohttp import ClientSession

URL_TEMPLATE = "https://api.icndb.com/jokes/{id}/"

class Request(NamedTuple):
    method: str
    url: str
    json: Optional[Dict[str, Any]] = None

class Response(NamedTuple):
    status: int
    json: Optional[Dict[str, Any]] = None

def get_joke(id: int) -> Iterable[Request]:

    response = yield Request("GET", URL_TEMPLATE.format(id=id))

    try:

        if response.status != 200:
            raise JokeApiError("API request failed")

        data = response.json
        if data.get("type") == 'NoSuchQuoteException':
            raise NoSuchJoke(data.get("value", ""))
        if data.get("type") != "success":
            raise JokeApiError("API request failed")

        raise Return(data["value"]["joke"])

    except (JokeApiError, Return, StopIteration):
        raise
    except Exception as e:
        raise JokeApiError() from e

def call_api_sync(it):
    try:
        for req in it:
            response = requests.request(req.method, req.url, json=req.json)
            it.send(
                Response(status=response.status_code, json=response.json())
            )
    except Return as e:
        return e.value

async def call_api_async(it):
    async with ClientSession() as session:
        try:
            for req in it:
                async with session.request(
                        method=req.method,
                        url=req.url,
                        json=req.json) as res:
                    json = await res.json()
                    response = Response(status=res.status, json=json)
                    it.send(response)
        except Return as e:
            return e.value

class JokeApiError(Exception):
    pass

class NoSuchJoke(JokeApiError):
    pass

class Return(Exception):

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

The code is admirably clear.
Nice annotations.
With these classes,
Chuck Norris wouldn't need a debugger,
he could just stare down the bug until the code confesses.

class Request(NamedTuple):
    method: str

Maybe list method 2nd, and allow defaulting to 'GET'?

    response = yield Request('GET', URL_TEMPLATE.format(id=id))

    try:

        if response.status != 200:

Ok, nit-picky observation: there's more than one success code,
so one might bury the test in an is_success() helper function.
(I use an API that routinely returns 204 success.)
But this is for pedagogic purposes, and the 200 test is perfect, don't change it.

Broader observation: showing that "stuff which can go wrong after GET" happens
within the try is pretty interesting.
There is a small amount of "ceremony" to access the result.
If this were a production library, rather than pedagogic, I'd be wondering
if we could maybe save clients from looking for 200 status at all,
and just raise appropriate errors if client is asking for bad data.

This would probably need an url_prefix_to_api_name registry map,
so we could identify a given url as being a Joke url,
and could look for a class with the conventional name JokeApiError.
(Clearly the registry would never contain Norris, because:
No statement can catch the ChuckNorrisException.)

The next few lines have boilerplate checks that maybe could be defaulted,
and / or moved into some JokeApi method a user could choose to supply.

The raise in this line:

    raise Return(data['value']['joke'])

is, ummm, slightly surprising.
I'm sure you have your technical reasons for it.
But maybe it could be a return, or a yield, or simply a method call with side effects?
Or a .close()-style cleanup handler of a with resource manager?
Or simply buried in library code that developers typically won't view?
The raise is just a little jarring, given that this is the success case.

I encourage you to keep pushing on this.
Two ways to win:

Write something a little bigger than just joke access, maybe do some CRUD.
Convince one other developer to use your stuff.

Both will push your code in the direction of increased maturity,
increased readiness for a 1.0 release.


Improving the flexibility of my library/what can be improved? How can I make my library more flexible towards programmers? Here is my library. It's a basic library that provides a simple interface to organise your game (with scenes, and engine/game connection). The only problem that I dislike about it, is the fact that you cannot create a custom 'game loop', you must let the GameLoop class deal with that for you. You can only instead, call to update/draw of the loop if you cannot use a while loop and manually clog the main thread (e.g. Ogre3d and it's call backs to update your game).

I'm also curious on what I can improve upon this library. Does my tutorial make sense? Is my code clean?

https://github.com/miguelishawt/pine

A few observations/thoughts from a quick look at the code and accompanying description. Feel free to explain/correct as appropriate:

For starters, the Game class seems to delegate everything to the GameLoop class, why? It seems more logical that GameLoop would drive the Game which would make use of GameEngine do to input/output and GameStateStack (or stacks) to keep track of game-related states. As it stands, I need to think in terms of frame changes/deltas in more than one section of the code.
When would I want to use the GameLoop as-is as opposed to using isRunning()/update() or deriving from GameLoop? Why not place the call to initialize() in the constructor?
Why does GameStateStackListener need to know about the GameStateStack? Does it consume events or produce events? Or both? How do you avoid infinite recursion if both?
Why only one type of GameStateStack per listener? We might want to keep track and be informed of several state types (for instance, a character would want to be notified of changes in gravity and wind).

I think you should make and publish a simple game using this library to illustrate the purpose of each class


Different character outputs using loops and strings I've just given this as a response to an absolute beginner on SO, and I'm wondering how terrible it is.

Output for an odd number of lines:

>
>>>
>>>>>
>>>
>

Output for an even number of lines:

>
>>>
>>>>>
>>>>>
>>>
>

The code:

int lines = 7;
int half = lines/2;
String carat = ">";
int i;

//better to declare first or does it not matter?

if(lines%2==0){boolean even = true;}
else{boolean even = false;}

for(i=1;i<=lines;i++){
        System.out.println(carat + "\n");
        if(i==half && even){System.out.println(carat+"\n");}
        if(((i>=half && even) || (i>=half+1)) && i!=lines){
                carat = carat.substring(0,carat.length()-2);
        }else{
                carat = carat + ">>";
        }
}

This code just seems so ugly, not considering the fact that there is no method that takes number of lines and that it is abusing integer rounding.

better to declare first or does it not matter?

No. These days most people believe variables should be declared as late as possible, right by where they are used.

  This code just seems so ugly.

It is. A little whitespace in there would help a lot. Which is easier to read:

// This:
if(i==half && even){System.out.println(carat+"\n");}

// Or this:
if (i == half && even) {
  System.out.println(carat+"\n");
}

  that it is abusing integer rounding.

There's no abuse, and it's not rounding. Truncation on integer arithmetic is a specified part of the language. Feel free to use it. In this case, it does exactly what you want.

If I were writing this, I'd do:

printCarats(int numLines) {
  for (int line = 0; line < numLines; line++) {
    // Mirror vertically across center line.
    int halfLine = line;
    if (line > numLines / 2) halfLine = numLines - line - 1;

    int numCarats = 1 + halfLine * 2;
    for (int i = 0; i < numCarats; i++) {
      System.out.print(">");
    }
    System.out.println();
  }
}


Given time intervals determine if a person could attend all meetings The task is taken from LeetCode

  Given an array of meeting time intervals consisting of start and end times [[s1,e1],[s2,e2],...] (si < ei), determine if a person could
  attend all meetings.

  Example 1:

Input: [[0,30],[5,10],[15,20]]
Output: false

  Example 2:

Input: [[7,10],[2,4]]
Output: true

My imperative solution:

/**
 * @param {number[][]} intervals
 * @return {boolean}
 */
var canAttendMeetings = function(intervals) {
  intervals.sort((a,b) => a[0] - b[0]);
  for (let i = 1; i < intervals.length; i++) {
    if (intervals[i][0] < intervals[i - 1][1]) { return false; }
  }
  return true;
};

My functional solution:

/**
 * @param {number[][]} intervals
 * @return {boolean}
 */
var canAttendMeetings = function(intervals) {
  return intervals
    .sort((a,b) => a[0] - b[0])
    .flat()
    .every((x,i, src) => i % 2 === 0 || src[i + 1] === void 0 || src[i] < src[i + 1]);
};

These two functions look quite sufficient to solve the task. I will say that the functional solution will likely be slower, not only because it is functional but also because of the call to .flat() and iterating over twice as many elements.

Correct me if this is incorrect, but the call to .flat() could be removed if the call to .every() was changed to a condition similar to the condition in the imperative solution:

.every((x,i, src) => i === 0 || !(src[i][0] < src[i - 1][1]));


Finding if there exists an input for a given automata for which no matter what the starting state is, final state would be same everytime I am solving a puzzle (Finding if there exists an input for a given automata for which no matter what the starting state is, final state would be same everytime) and have written following Python code. A few testcases are written in check method in the code. For these cases program is running fairly fast. However, for testcases where 50 lists (nodes) are present, the program is taking forever to execute. I am storing intermediate results to use further as well. Can someone please review the code and give suggestions on how to increase the performance of the code?

from itertools import product
from copy import deepcopy

class Node:
    def __init__(self,id):
        self.id = id
        self.dict = {}

    def __str__(self):
        return str(self.id) + " : " + str(self.dict)

    def __repr__(self):
        return str(self.id) + " : " + str(self.dict)

def tryDelete(nodes,_len):
    for i in range(len(nodes)):
        y = nodes[:]
        x = y[i]
        del y[i]
        tNodes = []
        for node in y:
            for input,result in node.dict.items():
                if result == x:
                    node.tDict = deepcopy(node.dict)
                    if x.dict[input] == x.id:
                        node.dict[input] = node
                    else:
                        node.dict[input] = x.dict[input]

        if pathPossible(y,_len ,False) == -1:
            return x.id
        for n in tNodes:
            n.dict = n.tDict
            del n.tDict
    return -2

target = {}
def FindTarget(node,p):
    if len(p) == 1:
        return node.dict[p[0]]
    if node not in target or p not in target[node]:
        x = Gnodes[FindTarget(node,p[:-1])].dict[p[-1]]
        if node not in target:
            target[node] = {}
        target[node][p] = x
    return target[node][p]

def allSatisy(nodes,p):
    x = None
    for node in nodes:
        if x is None:
            x = FindTarget(node,p)
        elif FindTarget(node,p) != x:
            return False
    return True

def allPossiblePaths(l,n):
    #x = int(((l+1)*(l+2))/2)
    for i in range(1, n + 1):
        for p in product(range(l),repeat=i):
            yield p

def pathPossible(nodes,_len ,isItereate=True):
    i = 1
    isFound = False
    for p in allPossiblePaths(_len,len(nodes)):
        if allSatisy(nodes,p):
            isFound = True
            break

    if isFound:
        return -1
    elif not isItereate:
        return -2
    else:
        return tryDelete(nodes,_len)

Gnodes = []
def answer(li):
    Gnodes[:] = []
    for i in range(len(li)):
        Gnodes.append(Node(i))#[i] = Node(i)
    for i in range(len(li)):
        for j in range(len(li[i])):
            Gnodes[i].dict[j] = li[i][j]
    return pathPossible(Gnodes,len(Gnodes[0].dict))

def check(li,ans):
    x = answer(li)
    print(str(li) + " : " + str(ans) + " : " + str(x))

def main():
    check([[2,1],[2,0],[3,1],[1,0]],-1)
    check([[1,2],[1,1],[2,2]],1)
    check([[1,3,0],[1,0,2],[1,1,2],[3,3,3]],-1)

if __name__ == '__main__':
    main()

There's no need to define both __str__ and __repr__ for Node when they're both the same functionality. If you just define __repr__ it will be called in place of __str__ when necessary. (though the reverse is not true). See:

>>> class A:
    def __repr__(self):
        return "self"

>>> str(A())
'self'

Instead of assigning a value to x then deleting it with del, use pop.

x = y.pop(i)

This simultaneously passes y[i] to x while removing it from y.

x = y[i]

This is also quite a confusing function, adding a docstring would make it a lot clearer what it actually is/does. As would better names. y could be nodes_copy and x could be... node I guess? I'm not entirely clear on what x is meant to be for, since the name is meaningless. Also what's tNodes? Does the t stand for something? Traversed possibly? Except I don't see you adding anything to the empty list so I don't see what it does at all.

In FindTarget you're calculating node not in target twice. You do need to use this for two separate if statements, but you could just store the boolean to save running the test twice:

def FindTarget(node,p):
    if len(p) == 1:
        return node.dict[p[0]]

    node_in_target = node in target
    if not node_in_target or p not in target[node]:
        x = Gnodes[FindTarget(node,p[:-1])].dict[p[-1]]
        if not node_in_target:
            target[node] = {}
        target[node][p] = x
    return target[node][p]

As @JoeWallis pointed out, a dictionary is fast at testing these look ups, but that doesn't mean this wont still save time.

In allSatisy, isn't x just being set to the result of FindTarget(nodes[0], p)? Because FindTarget never returns None, x will always be set to some result and therefore will only be set on the first iteration. Unless I'm wrong and the node values returned from FindTarget can be None. If that's not the case though, this would make your function clearer:

def allSatisy(nodes,p):
    x = FindTarget(nodes[0], p)
    for node in nodes[1:]:
        if FindTarget(node,p) != x:
            return False
    return True

And if this is the same as your function, then there's a faster way to do this using Python's all function. It can take a generator expression to evaluate every element in a list for a condition and return the result as a single boolean. In your case, you want to know if all the results of FindTarget(node, p) are equal to x, for each node in nodes (after the first node). Thankfully that's practically the syntax:

def allSatisy(nodes,p):
    x = FindTarget(nodes[0], p)
    return all(FindTarget(node, p) == x for node in nodes[1:])


Should I put default values of attributes on the prototype to save space? Suppose I've got a JavaScript function that I'm treating like a class - that is, I want to make many instances of it:
function Blerg() {
  this._a = 5;
}

Blerg.prototype.getA = function() {
  return this._a;
}

Blerg.prototype.setA = function(val) {
  this._a = val;
}

This class has one attribute, a, which the constructor instantiates to a default value of 5. It is accessed with the getter and setter.
Now suppose I have such a class but it has 30+ attributes (a, b, c, etc), and suppose like a that these all have unique default values, but also that it is uncommon for them to be changed from the default.
Suppose also that I need to make 10,000 or more Blerg instances, and so a design goal is to save space.
I'm wondering if it is a good idea to put all of the default values for my 30+ attributes on the prototype of my class instead. That way, when I create 10,000+ instances of my class, none of them have a, b, c, etc attributes, but calling getA() will still return the correct default value.
So I present this modified Blerg function, Blerg2:
function Blerg2() {
  // nothing!
}

Blerg2.prototype._a = 5;

Blerg2.prototype.getA = function() {
  return this._a;
}

Blerg2.prototype.setA = function(val) {
  this._a = val;
}

Are there downsides to taking this approach?

Some notes
The prototype way seems faster to create, see: http://jsperf.com/blergs
For that matter, the worst-case scenario does not look that bad: http://jsperf.com/blergs/2
And creating many of them using the code in the jsperf test (in its own html page) and adding:
var a = [];
for (var i = 0; i < 400000; i++) {
  a.push(new Blerg()); // or Blerg2
}

Suggests that the heap size for the objects in question is cut in half by using Blerg2.

related jsperf http://jsperf.com/12312412354

Well this is a really bad idea. Objects are always considered having different class if they don't have exactly the same set of properties in the same order. So
a function that accepts these objects will in best case be polymorphic and in worst case megamorphic all the while you are thinking
you are passing it same class of objects. This is fundamental to all JS engines although the specifics that follow focus on V8.

Consider:

function monomorphic( a ) {
    return a.prop + a.prop;
}

var obj = {prop: 3};
while( true ) {
    monomorphic( obj );
}

Now, since the passed object a always has the same class, we will get really good code:

    ; load from stack to eax
15633697    23  8b4508         mov eax,[ebp+0x8]
    ;; test that `a` is an object and not a small integer
1563369A    26  f7c001000000   test eax,0x1
156336A0    32  0f8485000000   jz 171  (1563372B) ;;deoptimize if it is not
    ;; test that `a`'s class is as expected
156336A6    38  8178ffb9f7902f cmp [eax+0xff],0x2f90f7b9
156336AD    45  0f857d000000   jnz 176  (15633730) ;;deoptimize if it is not
    ;; load a.prop into ecx, as you can see it's like doing struct->field in C
156336B3    51  8b480b         mov ecx,[eax+0xb]
    ;; this will untag the tagged pointer so that integer arithmetic can be done to it
156336B6    54  d1f9           sar ecx,1
    ;; perform a.prop + a.prop
    ;; note that if it was a.prop + a.prop2
    ;; then it wouldn't need to do all those checks again
    ;; so for one time check inside the function we can load all the properties
    ;; quickly
156336B8    56  03c9           add ecx,ecx

Notice what happened here, V8 saw that we always pass the same class of object to the function
monomoprhic and generated really tight code that assumes we will always get that class of object
in the future as well.

Now let's do:

function polymorphic( a ) {
    return a.prop + a.prop;
}

var obj = {prop: 3};
var obj2 = {prop: 3, prop2: 4};
while( true ) {
    polymorphic( Math.random() < 0.5 ? obj : obj2 );
}

Now the function must consider 2 different classes of objects. The classes are different but similar enough
that the client code can stay as it is as both classes contain a field prop.

Let's see:

    ; load from stack to eax
04C33E17    23  8b4508         mov eax,[ebp+0x8]
    ;; test that `a` is an object and not a small integer
04C33E1A    26  f7c001000000   test eax,0x1
04C33E20    32  0f8492000000   jz 184  (04C33EB8) ;; deoptimize if not
    ;; test that `a`'s class is one of the expected classes
04C33E26    38  8178ffb9f7401c cmp [eax+0xff],0x1c40f7b9
04C33E2D    45  0f840d000000   jz 64  (04C33E40) ;; if it is, skip the second check and go to the addition code
    ;; otherwise check that `a`'s class is the second one of the expected classes
04C33E33    51  8178ff31f8401c cmp [eax+0xff],0x1c40f831
04C33E3A    58  0f857d000000   jnz 189  (04C33EBD) ;; deoptimize if not
    ;; load a.prop into ecx
    ;; if you are still reading this you will probably notice that this
    ;; is actually relying on the fact that both classes declared the prop field
    ;; first
04C33E40    64  8b480b         mov ecx,[eax+0xb]
    ;; this will untag the tagged pointer so that integer arithmetic can be done to it
04C33E43    67  d1f9           sar ecx,1
    ;; do the addition
04C33E45    69  03c9           add ecx,ecx

Ok so the situation is still pretty good but here we are relying on the fact that properties are in same order
and that there are only 2 different classes.

Let's do the same in different order so that V8 can't use the same instruction (mov ecx,[eax+0xb]) for both objects:

function polymorphic( a ) {
    return a.prop + a.prop;
}

var obj = {prop: 3};
var obj2 = {prop2: 4, prop: 3};
while( true ) {
    polymorphic( Math.random() < 0.5 ? obj : obj2 );
}

And:

06C33E84    36  8b4508         mov eax,[ebp+0x8]
    ;; small integer check
06C33E87    39  f7c001000000   test eax,0x1
06C33E8D    45  0f84d3000000   jz 262  (06C33F66)
    ;; class check 1
06C33E93    51  8178ffb9f75037 cmp [eax+0xff],0x3750f7b9
06C33E9A    58  7505           jnz 65  (06C33EA1)
06C33E9C    60  8b480b         mov ecx,[eax+0xb]
06C33E9F    63  eb10           jmp 81  (06C33EB1)
    ;; class check 2
06C33EA1    65  8178ff31f85037 cmp [eax+0xff],0x3750f831
06C33EA8    72  0f85bd000000   jnz 267  (06C33F6B)
06C33EAE    78  8b480f         mov ecx,[eax+0xf]

06C33EB1    81  f6c101         test_b cl,0x1
06C33EB4    84  0f851e000000   jnz 120  (06C33ED8)
06C33EBA    90  d1f9           sar ecx,1
06C33EBC    92  89ca           mov edx,ecx
06C33EBE    94  03d1           add edx,ecx

Ok just as expected, just using different offsets depending on the class.

So you can see where this is going, if you end up with 10 different classes then you will
just get 30 instructions (instead of 3) whenever a function will need to lookup a property. Which is still
much better than a hash table (100s of instructions?) lookup.

Well no, turns out there is a limit of 4 different classes
and then you go into megamorphic mode.

So with this, we should see radically different code output if we use 5 or more different classes:

function megamorphic( a ) {
    return a.prop + a.prop;
}

var objs = [
    {prop: 3},
    {prop3: 4, prop: 3, prop2: 4},
    {prop4: 4, prop2: 4, prop: 3, prop5: 6},
    {prop: 3, prop12: 6},
    {prop7: 15, prop30: 12, prop314: 4, prop34: 15, prop: 3}
];

while( true ) {
    var index = Math.random() * objs.length | 0;
    megamorphic( objs[index] );
}

Indeed:

3D3342E7    39  8b5508         mov edx,[ebp+0x8]
3D3342EA    42  b9f5e2a115     mov ecx,15A1E2F5
3D3342EF    47  e84c6ffeff     call LoadIC_Initialize  (3D31B240)
3D3342F4    52  8945ec         mov [ebp+0xec],eax
3D3342F7    55  8b75f0         mov esi,[ebp+0xf0]
3D3342FA    58  8b5508         mov edx,[ebp+0x8]
3D3342FD    61  b9f5e2a115     mov ecx,15A1E2F5
3D334302    66  e8396ffeff     call LoadIC_Initialize  (3D31B240)
3D334307    71  8b4dec         mov ecx,[ebp+0xec]
3D33430A    74  f6c101         test_b cl,0x1
3D33430D    77  0f8527000000   jnz 122  (3D33433A)
3D334313    83  d1f9           sar ecx,1
3D334315    85  89c2           mov edx,eax
3D334317    87  f6c201         test_b dl,0x1
3D33431A    90  0f8546000000   jnz 166  (3D334366)
3D334320    96  d1fa           sar edx,1
3D334322    98  03d1           add edx,ecx

That actually looks a lot like code that loads properties from objects that are in hash table mode. Is it fast? Unfortunately jsperf is down now
so you have to run it yourself with code here:

Megamorphic 492
Monomorphic 30

So the same function body ran 15 times faster because the object passed to it always had the same class. It is also hard to predict if you will actually save memory
if all those 10000 objects allocate a different class for instance.

It is also complicated to really look at all the downsides but having optional properties is one of those things that it is easy to say is very bad.


Clean implementation of FTP File Cleaners I was refactoring some of my utility code modules used in my test projects, and wanted to apply my knowledge of clean OOP design patterns and SOLID principles to make these modules more useful and easier to use.

I have an interface called IFtpFileCleaner:

/// <summary>
/// Responsible for deleting a single file located in a remote server
/// </summary>
public interface IFtpFileCleaner
{
    /// <summary>
    /// Deletes a remote file
    /// </summary>
    /// <param name="remotePath">
    /// The full remote path.
    /// </param>
    void Clean(string remotePath);
}

I have an abstract class called RemoteFileCleaner, which implements IFtpFileCleaner:

/// <summary>
/// The file cleaner base class provides common code for all the implementations to reduce code duplication
/// </summary>
public abstract class RemoteFileCleaner : IFtpFileCleaner
{
    /// <summary>
    /// The FTP _client.
    /// </summary>
    private readonly IFtpSecureClient _client;

    /// <summary>
    /// Initializes a new instance of the <see cref="RemoteFileCleaner"/> class.
    /// </summary>
    /// <param name="client">
    /// The FTP client.
    /// </param>
    protected RemoteFileCleaner(IFtpSecureClient client)
    {
        _client = client;
    }

    /// <summary>
    /// Deletes a single file located on a remote server
    /// </summary>
    /// <param name="remotePath">
    /// The full path of the remote file.
    /// </param>
    public void Clean(string remotePath)
    {
        try
        {
            _client.Init();
            _client.DeleteFile(remotePath);
        }
        catch (Exception e)
        {
            Console.WriteLine($"Failed to clean file {remotePath}.{Environment.NewLine}{e}");
            throw;
        }
        finally
        {
            _client.Disconnect();
        }
        Console.WriteLine($"Remote File {remotePath} was deleted.");
    }
}

Now, I can have concrete implementations of my file cleaners. Like SFTPFileCleaner, FTPFileCleaner, and FTPSFileCleaner. They all look very similar. For instance, SFTPFileCleaner looks like this:

/// <summary>
/// The SFTP implementation of <see cref="RemoteFileCleaner"/>.
/// </summary>
public class SFTPFileCleaner : RemoteFileCleaner
{
    /// <summary>
    /// Initializes a new instance of the <see cref="SFTPFileCleaner"/> class.
    /// </summary>
    public SFTPFileCleaner()
        : base(new FtpSecureClientTestFactory().CreateSFTPClient())
    {
    }
}

As you can see, I am performing "Bastard Injection" here in the concrete implementation because I would like to easily instantiate these cleaners in my test methods. Here is an example:

[TearDown]
public void Cleanup()
{
    _ftpCleaner = new SFTPFileCleaner();
    _ftpCleaner.Clean(_expectedRemotePath);
}

What do you guys think about this design? Is it clean and easy to use?
Can it be improved any further?
Is there a name for this design pattern? To me is seems to be the mixture of Facade Pattern (Bastard Injection) and Strategy Pattern. Am I correct?

Wouldn't it make more sense to have IRemoteFileCleaner as the base class, and have FTPFileCleaner (which inherits from IRemoteFileCleaner as the class which all the others inherit from?


C# Method to determine database status I have a task that I am trying to complete and would appreciate direction, to know which is the most efficient path to take for future requirements.

Use Case: A background process that determines if a specific database is broken. This task will be run every two minutes as we must know ASAP if there is a problem with the database.

My Solution: Opening a connection with said database, if the conenction is refused - an email is then sent notifying the appropriate individuals.

Here is the method I built to determine the DB state:

    public bool DatabaseConnection()
    {
        bool statusUp = true;

        using (var databaseConnection = new SqlConnection(ConfigData.ConnectionStrings.DatabaseConnectionString))
        {
            try
            {
                databaseConnection.Open()
            }
            catch (SqlException ex)
            {
                const string message = "Could not establish a connection with Database.";
                Log.DatabaseStatusDown(message, ex);
                statusUp = false;
            }
            finally { databaseConnection.Close(); }
        }

        return statusUp
    }

I am aware that the Using statement leverages the IDisposable class and will auto-dispose the connection, but I wanted to be extra cautious and included the finally statement out of paranoia.

If anyone could please give me criticism, suggestions, recommendations or really anything that can help me improve this code I would greatly appreciate it. I am also open to different ideas on how I can determine if a DB is broken.

Closing the database connection in the finally will cause an exception if the Open () call throws an exception, so you better close the connection  immediately after you opened it in the try block.

To overcome this problem in a clean way you should let the using block handle it to close the database connection.

Method's should be named using a verb or verb phrase. So better name it e.g IsDatabaseServerAlive ().

Otherwise it looks ok for me.


Perfect-forwarding while implementing emplace I am trying to write a class similar to std::set, and I was wondering if I handled the perfect forwarding right in implementing the emplace member function below. Basically, I forwarded calls to my class to a member set. But I wasn't sure whether I used std::forward enough.

Also, is there an easy way to tell when something is not forwarded as desired, and values are copied instead?

#include <set>
using namespace std;

template<typename T>
class myset {
  std::set<T> m_s;
  typedef typename std::set<T>::iterator iterator;
  int cnt;
public:
  template <class... Args>
  pair <iterator,bool> emplace ( Args&&... args ) {
    pair<iterator,bool> p = m_s.emplace(std::forward<Args>(args)...);
    if (p.second) //true if a new element was inserted
      cnt ++; //house keeping
    return p;
  }
};

int main() {myset<int> s;}

You are using std::forward correctly. However, there are several other things worth noting about your code:

why do you need a private counter instead of re-using m_s.size() that will automatically update itself after each emplace_back()?
are you sure that you don't need additional template arguments Compare and Allocator instead of relying on their defaults std::less<T> and std::allocator<T>?
since you are using C++11 already, please use auto to get rid of the pair<itererator, bool> (and if you are using C++14, you can also use auto for the return type).
try also to write using iterator = typename std::set<T>::iterator; because left-to-right is much easier to parse than right-to-left of the current typedef.


Find the longest word in a sentence This is my second attempt after I started to learn about STL. I am oblivious and need suggestions/advices on if there can be improvements made on this code.

#include <iostream>
#include <string>
#include <sstream>
#include <algorithm>
#include <iterator>
#include <vector>
using namespace std;

string LongestWord(string sen) {

  vector<string> coll; //initialize vector

  istringstream iss(sen); //read the string "sen"

  copy(istream_iterator<string>(iss), //copy from beginning of iss
    istream_iterator<string>(), //to the end of iss
    back_inserter(coll)); //and insert string to vector
  //istream_iterator by default separates word by whitespace

  string longestWord = coll.at(0);
  int longestCount = longestWord.length();

  for(auto element : coll)
  {
      if(element.length() > longestCount)
      {
          longestWord = element;
      }
  }
    return longestWord;

}

As @user1118321 points out: just using your implementation, I believe you're missing a crucial line (inside your for loop and if-statement):

longestCount = element.length();

Which becomes:

#include <iostream>
#include <string>
#include <sstream>
#include <algorithm>
#include <iterator>
#include <vector>
using namespace std;

string LongestWord(string sen) {
    vector<string> coll; //initialize vector

    istringstream iss(sen); //read the string "sen"

    copy(istream_iterator<string>(iss), //copy from beginning of iss
        istream_iterator<string>(), //to the end of iss
        back_inserter(coll)); //and insert string to vector
    //istream_iterator by default separates word by whitespace

    string longestWord = coll.at(0);
    int longestCount = longestWord.length();

    for(auto element : coll)
    {
        if(element.length() > longestCount)
        {
            longestWord = element;
            longestCount = element.length();
        }
    }

    return longestWord;
}

Don't use using namespace std, it's better to use std:: as needed. While it's insignificant here, if you work on larger projects it will cause headaches. Especially if you're learning STL, it's a good idea to learn what goes in the STL and the C++ Standard Library (ie within namespace std).

Instead of iterating over the string (when tokenizing into words) and again over the vector, you could just iterate once:

Below is a common way to find the maximum of a container of elements, in this case std::vector<std::string>, while taking advantage of std::istringstream:

#include <sstream>
#include <string>

std::string LongestWord(std::string str){
    std::string::size_type max_len = 0;
    std::string longest_word;
    std::string word;

    std::istringstream stream(str);

    while(stream >> word) {
        if(max_len < word.length()) {
            max_len = word.length();
            longest_word = word;
        }
    }

    return longest_word;
}

This function could be improved depending on how you wish to use it. You may wish to not count punctuation (recommended), you may wish to return a vector of strings if there are words of the same length, or you may wish to throw an error when given an empty string. These are all fairly easy to implement as needed.


BigInteger prime testing import java.math.BigInteger;
import java.util.Random;
public class Primetest {

    private static int size=15;
    private static Random r=new Random();
    private static BigInteger two=new BigInteger("2");
    private static BigInteger three=new BigInteger("3");
    public static void main(String[] args) {

        while(true)
        {
            BigInteger p=new BigInteger(size,r);
            if(isprime(p)==true)
            {
                System.out.println("prime="+p);
                break;
            }
        }
    }

    public static boolean isprime(BigInteger n)
        {
             if(n.compareTo(BigInteger.ONE)==0 || n.compareTo(two)==0)
            {
                return true;
            }
          BigInteger half=n.divide(two);

           for(BigInteger i=three; i.compareTo(half)<=0;i=i.add(two))
            {

                if(n.mod(i).equals(BigInteger.ZERO))
                {
                  return false;
                }

            }
             return true;

        }
 }

This code selects a random prime BigInteger number. I want a 2048 bit BigInteger prime number, but it only works with 15 bit. Can anybody help me?

Edit: Your code is working beyond 15 bits, it's just really slow..... More about that later.

I am not sure why your code does not work beyond 15-bit BigIntegers either.... it's not something daft like the fact you have size = 15, now, is it?

Two suggestions

Apart from that, there are two items that may interest you:

you can probably improve performance by using the two methods:

nextProbablePrime()
isProbablePrime()

With these methods you can check your candidate prime before you do the hard work... i.e. your first call in the isprime() method can be:

if (!n.isProbablePrime()) {
     return false;
}

Then, if it is probably a prime, you will need to check it, and you can do that in a way that may be faster with:

for(BigInteger i=three; i.compareTo(half)<=0;i=i.nextProbablePrime()) {
    ....
}

This may, or may not be faster. Check it.
What will be faster is if you only check to the square-root of n, instead of 'half'. BigInteger does not have a method square-root, but you can get an approximate limit quite fast with just a few iterations through the Babylonian method. You don't need to find the actual square-root, just some value that is slightly larger than the root. This will save many, many, many potential iterations for when the input value really is prime.

Test implementation

I have put together some example code which shows a few things:

your code is, in fact working....
when you have large numbers, your prime factoring guesses are going to pretty much take forever.... and I am not sure if forever-running code is working code...
It uses the faster isProbablyPrime(int) method to short-circuit the slow math.
It computes an approximate square-root of the input value.
It uses nextProbablePrime() instead of checking every odd number in the loop.

Note that I think your algorithm could also be optimized by changing the outer loop. If the goal is to find a large random number, then the better way to do it would be to choose a large number randomly, make sure it is odd, and then test it for prime-ness. If it is not prime, then keep adding 2 until the value is prime. This will significantly reduce the random hunting for prime values.

Here is the code:

private static int size = 1024;
private static Random r = new Random(1);
private static BigInteger two = new BigInteger("2");
private static BigInteger three = new BigInteger("3");

public static void main(String[] args) {

    while (true) {
        BigInteger p = new BigInteger(size, r);
        System.out.println("Processing prime " + p + " with input " + size
                + " bits");

        if (isprime(p) == true) {
            System.out.println("prime=" + p);
            break;
        }
    }
}

public static boolean isprime(BigInteger n) {

    if (!n.isProbablePrime(10)) {
        return false;
    }

    if (n.compareTo(BigInteger.ONE) == 0 || n.compareTo(two) == 0) {
        return true;
    }
    BigInteger root = appxRoot(n);
    System.out.println("Using approximate root " + root);

    int cnt = 0;
    for (BigInteger i = three; i.compareTo(root) <= 0; i = i
            .nextProbablePrime()) {
        cnt++;
        if (cnt % 1000 == 0) {
            System.out.println(cnt + " Using next prime " + i);
        }
        if (n.mod(i).equals(BigInteger.ZERO)) {
            return false;
        }

    }
    return true;

}

private static BigInteger appxRoot(final BigInteger n) {
    BigInteger half = n.shiftRight(1);
    while (half.multiply(half).compareTo(n) > 0) {
        half = half.shiftRight(1);
    }
    return half.shiftLeft(1);
}

And, here is the output

Processing prime 98329376886274465203040498066651959157640182809094020631664385777551847591123638109144698149095205744886312777380124671380640351550097713803874959847367838878126620008205910502904640512240477769179755530216157310355581410760731210073213721673064659778570224177036380289355437254781245378941806708640626915896 with input 1024 bits
Processing prime 37646605107187235792943122848905725816642680759921726391069038948580233504043310475106107937453362419670194263133240792533036542339813188374350574945196545621538046755833078747457686689427373500864049860688062396137214885740182673993779980005699518336257562987964318771232237070594662691795239179395907746381 with input 1024 bits
Processing prime 125205778308571596366364664578026315265996047681538536772575342694310454438809896771367811860534210488218628994364991251294455216205362342490618659150824104797110134588899854049460243713711039304748274575425927846133394573799688746650452867761117377057477970221094526444104587198668516522314203910650060775297 with input 1024 bits
Processing prime 29927154392784343780961536475173096429548596167420203155112563329853022850079385324824305227046118707802338722443720119949840770560558670171973682574167343679095711827324930773450204678598945005042160247713118600168856758838352856602314407651532633576632941646038272135258001495947267516367654449573770447978 with input 1024 bits
Processing prime 139243961902015178358905340711072637999780823861985238094201532190345570854225223205633490036395897038352240416702384303956261085630258304397095684303936086294128301544469715824139689002062508603332475411268227340511899314982630226967627021263463712185132048978641948260688280239972303621025851700017991795656 with input 1024 bits
Processing prime 122363695497135362876334096986750092237187818163935289528269643131788940948676195886148173694943587069178270471951802290191005226421476324835369279161430725594938044767129988205836293228618396536013056503441398180392306035348529131293104302985721904196513857471448257453900973803574632148674525698706255968469 with input 1024 bits
Processing prime 6766841734279494121695202528813564426876102386393672096192548380596947610679150306271345703101761193843898532659901726349258449119163161846636750914151571614552775446973414312295356121082306016346670297986250268817217032557516075488000173517339258842372202954513367225234606811784912645921661651252301210439 with input 1024 bits
Processing prime 1244168157289385582197158692756366939639032807157147509726417844315011285784938824776953893721289756574282393714469772808628985336160381884771437651533388861457165174027710304927221871555060351769769251837444763151883211741263529676654328058488380773866083064897587855677058477160609886235025932428799595897 with input 1024 bits
Processing prime 132158916583424296773989292524518115815097623427134119856232403777863313269411230253297504275469503273779555561857087644449894885032508020437681301637184423517233528793784318637110731993459739195119489689739447166431101193405592878819600262595536763785155730768337651779977312858406291899622027075197339272533 with input 1024 bits
Processing prime 69992427855596672916046274609831358925004102231151217146504934042278719263838690406603474915165537361784459455893083094877744290705302384633321340170916902155546529977673369310451021181970658193490668498069273542381636608842630759935789234533400192866934709438808312038202012904797132388392567889891155748450 with input 1024 bits
Processing prime 21158035183654230873794938382538380103022588111151171726822340627470812338126036440129555658534288545561323627229661551179395796822536779689099195780624577722259563387380039125533382285521378162912552760478091032365155104156393389243223430957343814123541441495493412606366108994001798740793143149348566611849 with input 1024 bits
Processing prime 15252401507860116139044993701109820472568933087608994119874588139374351833868522812721998308899653583211533913737970680874956499213716249151797416323727562426289207703789447617206459217803392887453871279176965603731348358876019227478462392987492580262414905238078166067132705908043224132942658110708655117575 with input 1024 bits
Processing prime 79207591965644170299922757157810677225143754875497695785333916966745459024878221535386104794074683912029219571613935837926395463393267459748825400976839505450111313617312314479345019735666163348592463847785881480127339787201145529464779061029020193892147298882124349322947190382379671430079191833914102173464 with input 1024 bits
Processing prime 61030497376329633328073316647655267262303694060711488520514344109326325543945802124423073251577269169186781440230219553617205658081336472411027108746182826458446589150653587257149674878699689258298746835269894603933930803472419590290016677183062036325055276717300539131443014986954057041180792801863470921160 with input 1024 bits
Processing prime 169114037779361593124142841783629182425924353804019723914654438093025578377446888507229925252227113606029383048794273249003173542629081468619397107139204307145864334149929493619281837363125251731861465625382815511697458723930987915758447225669236647982784800507786813022174334313252476414285766502413871475837 with input 1024 bits
Using approximate root 25226202323750862638468707057345770204269264134797015774704159175996067529790533462300444165020098361223682946889361283140757213132269147109605325588573982
1000 Using next prime 7927
2000 Using next prime 17393
3000 Using next prime 27457
4000 Using next prime 37831

by my estimates, on my machine, it will probably take a few years to test all primes up to 25226202323750862638468707057345770204269264134797015774704159175996067529790533462300444165020098361223682946889361283140757213132269147109605325588573982 .... but, that is much faster than what it would to calculate all the primes up to half of the test value which is 169114037779361593124142841783629182425924353804019723914654438093025578377446888507229925252227113606029383048794273249003173542629081468619397107139204307145864334149929493619281837363125251731861465625382815511697458723930987915758447225669236647982784800507786813022174334313252476414285766502413871475837

If you follow my suggestion and loop to find primes using +2 instead of random, then your main method could be:

public static void main(String[] args) {

    BigInteger p = new BigInteger(size, r);
    if (testBit(0)) {
        p = p.add(BigInteger.ONE);
    }
    while (true) {
        System.out.println("Processing prime " + p + " with input " + size
                + " bits");

        if (isprime(p) == true) {
            System.out.println("prime=" + p);
            break;
        }
        p = p.add(TWO);
    }
}


Using multiple popup windows with bPopup I'm using a lightweight jQuery popup plugin called 'bPopup'. I'm using it on my website at the moment to load multiple popup windows when clicked. I was recently told that my code was inefficient as I was loading multiple popups with multiple JavaScript 'listeners':

He is right.  Think about maintenance.

If you wanted to add another link that popped up another element you would need to add yet another listener function.  Removing one would be the same thing.  Too many things to keep track of.

Change the code to add the event according to a class name and then you can bind to all at once.  Then include the popup element ID within the attributes of the opener link.

$('.popup-opener').bind('click', function(e) {
    e.preventDefault();
    $($(this).attr('popup-element')).bPopup();
});

Popup 1

// ...

Better yet, use the on() function to bind the event.  http://api.jquery.com/on/

Adding new openers now requires no changes to the javascript.


Pattern matching deeply nested arguments in Elixir I've recently started working with Elixir and am writing an app that queries various URLs and parses the response. I am having issues with my parse method, which is included below. The code works just fine, but I am concerned that I may have taken pattern matching too far. The method signature looks like spaghetti.

This is the code that sets the whole thing in process. I think it's fine, but include it to give you some background:

  def run(job) do
    HTTPStatusCheck.query(job)
    |> HTTPStatusCheck.parse
    |> update_db
    |> send_alerts
  end

The query method returns this "object":

%HTTPCheckResult{parsed: false, http_response: resp}

# resp is a %HTTPoison.Response{} "object"

Here is the parse method I need help with:

  def parse(%HTTPCheckResult{
    parsed: false, http_response: {:ok, %Response{status_code: c}}} = result)
    when c in @up do
      %{result | parsed: true, code: c, result: :up}
  end
  def parse(%HTTPCheckResult{
    parsed: false, http_response: {:ok, %Response{status_code: c}}} = result)
    when c in @down do
      %{result | parsed: true, code: c, result: :down}
  end
  def parse(%HTTPCheckResult{
    parsed: false, http_response: {:ok, %Response{status_code: c}}} = result) do
      %{result | parsed: true, result: :error, error: "http code #{c}"}
  end
  def parse(
    %HTTPCheckResult{parsed: false, http_response: {:error, %HTTPoison.Error{reason: reason}}} = result) do
      %{result | parsed: true, result: :error, error: reason}
  end
  def parse(%HTTPCheckResult{parsed: false} = result) do
    %{result | result: :error, error: "unknown"}
  end

My biggest objection is really the size of the parse method signatures, which are three lines in some cases. The important parts also seem pretty deeply nested in there.

How can the parse method can be improved?

I've just taken a whack at cleaning up my original code after a few days.

edit: Now including refactor suggestions from alxndr.

Set everything in motion

  def run(job) do
    HTTPStatusCheck.run(job)
    |> update_db
    |> send_alerts
  end

(The new run method seen here handles what was previously done by query and parse.)

The new HTTPStatusCheck.run/1 method:

  def run(job) do
    method = :head
    url = job.url
    body = ""
    headers = [@user_agent]
    options = @http_options

    HTTPoison.request(method, url, body, headers, options)
    |> parse
  end

(You can see it calls parse internally now, as I cannot find a reason anything else would ever need to work with the unparsed results object.)

The new HTTPStatusCheck.parse/1 definitions:

  defp parse({:ok, %Response{status_code: c}} = resp) do
    %HTTPCheckResult{}
    |> add_http_code(c)
    |> add_http_response(resp)
  end

  defp parse({:error, %HTTPoison.Error{reason: reason}} = resp) do
    %HTTPCheckResult{}
    |> add_http_response(resp)
    |> add_error(reason)
  end

  defp parse(_) do
    %HTTPCheckResult{}
    |> add_error("unknown")
  end

(I feel these are much better, as the method signature and definition code are now much shorter and more readable. The suggestions from alxndr were implemented here and in the next section of helper methods.)

Helper methods for HTTPStatusCheck.parse/1:

  defp add_result(result, val), do: %{result | result: val}
  defp add_http_response(result, response), do: %{result | http_response: response}

  defp add_http_code(result, code) do
    %{result | code: code}
    |> handle_http_code(code)
  end

  defp handle_http_code(result, code) when code in @up do
    result
    |> add_result(:up)
  end
  defp handle_http_code(result, code) when code in @down do
    result
    |> add_result(:down)
  end
  defp handle_http_code(result, code) do
    result
    |> add_error("unhandled http code #{c}")
  end

  defp add_error(result, error) do
    %{result | error: error}
    |> add_result(:error)
  end

%HTTPCheckResult{} is improved too:

There is no longer the need to indicate parsed: false or parsed: true on this struct.


Longest substring with no repetitions This is the code I came up with.It seems correct.

I am interested in:
1) Improvements on code
2) How can it be better than this? i.e. this is O(N^2) Could I have done better in complexity? What should I have read to do it?

Thanks

 public int lengthOfLongestSubstringNoRepetitions(String s) {

            if(s == null || s.trim().equals(""))
                return 0;

            Map<Character,Boolean> seen = new HashMap<Character,Boolean>();

            int max = Integer.MIN_VALUE;

            for(int i = 0; i < s.length(); i++){
                for(int j = i; j < s.length(); j++){
                    char c = s.charAt(j);
                    if(seen.containsKey(new Character(c))){
                        //duplicate in substring
                        int tmp = j - i;
                        if(max < tmp) max = tmp;
                        break;
                    }
                    else{
                        seen.put(new Character(c),true);
                        if(j == s.length() -1){
                            //Reached the end of string.Add one to get the range
                            if(max < (j - i + 1)) max = (j - i + 1);
                        }
                    }
                }
                seen.clear();
            }

            return (max == Integer.MIN_VALUE)?s.length():max;

        }

Here's an O(n) solution. It replaces the Map with a char-indexed vector of the index where each char was last seen.

public static int lengthOfLongestSubstringNoRepetitions(String s) {

    if (s == null)
        return 0;

    // Trimming input even for the non-empty case is more consistent.
    final String str = s.trim();

    if (str.equals(""))
        return 0;

    int seen[] = new int[Character.MAX_VALUE+1];
    for (int i = 0; i <= Character.MAX_VALUE; ++i)
        seen[i] = -1;

    int max = 1;
    int len = 0;

    for (int j = 0; j < str.length(); ++j) {
        char ch = str.charAt(j);
        // If ch was recently seen,
        // counting must restart after the last place it was seen.
        // Otherwise, it adds 1 to the length.
        len = Math.min(j-seen[ch], len+1);
        if (len > max)
            max = len;
        seen[ch] = j;
    }
    return max;
}


String to hash in Ruby without JSON.parse Just for the sake of understanding. I know JSON.parse could do this pretty easily, but without using that, how could this be made cleaner or more efficient?

string = "ounces: 10; contains_toy: false"
result = string.split("; ")
result = result.map{|x| x = x.split(": "); Hash[x.first.to_sym, x.last] }
result = result.reduce(:merge)

Desired result:

{:ounces=>"10", :contains_toy=>"false"}

Chain your calls. Don't update the same variable again and again with very different values.
Just construct a hash straight away with each_with_object or construct an array of pairs, and turn that into a hash.
Any sort of splitting/scanning is prone to errors if the string contains tokens that aren't part of the syntax. I.e. a nested string with a semicolon in it will be split as though it's a delimiter.

Anyway, you can do this, for instance:

result = Hash[ string.scan(/(\w+):\s+([^;]+)/) ]

though that'll give you string keys.

To avoid that, you could do:

pairs = string.scan(/(\w+):\s+([^;]+)/).map { |k,v| [k.to_sym, v.strip] }
result = Hash[pairs]

or

result = string.scan(/(\w+):\s+([^;]+)/).each_with_object({}) do |(k,v), hash|
  hash[k.to_sym] = v.strip
end

Another, much less robust way would be:

pairs = string.split(/[:;]/)
  .map(&:strip)
  .each_slice(2)
  .map { |k,v| [k.strip.to_sym, v.strip] }
result = Hash[pairs]

Edit: And the totally unsafe option would be

result = eval("{#{string.tr(';', ',')}}")

On the plus side, your values have proper types (not all strings). On the minus side, eval is evil, and should always be avoided. So, really, don't do this.


Checking for a value on selectors I've got some pretty simple validation that I'm working on, but it seems like I'm repeating myself a bunch of times. There has to be a better way of doing this. I'm simply checking if there is a value filled in for specific fields and apply a color if the value isn't filled in. There are also other areas where the validation is different where I'm checking a valid phone number. So I couldn't simply apply this to all inputs. Here's some of my code:

var x = $();
var pass = true;

function validateForm(){
        x = $('input#zip').val();
        if (x == null || x == "") {
            $('#ziplabel').css('color','#fff');
            pass = false;
        } else {
            $('#ziplabel').css('color','#444');
        }
        x = $('input#fname').val();
        if (x == null || x == "") {
            $('#fnamelabel').css('color','#fff');
            pass = false;
        } else {
            $('#fnamelabel').css('color','#444');
        }
        x = $('input#lname').val();
        if (x == null || x == "") {
            $('#lnamelabel').css('color','#fff');
            pass = false;
        } else {
            $('#lnamelabel').css('color','#444');
        }

}

Yup, you can iterate the input fields. For this sort of thing you should also be using classes, instead of inline CSS. Using jQuery .each and .togggleClass you can create something like this:

function validate () {

  $('#form input').each(function () {
     var $el = $(this)
     $el.prev('label').toggleClass('invalid', $el.val());
  });

}

And your stylesheet has a rule like this;

label {
  color: #444;
}

label.invalid {
  color: #fff;
}

If you want to check if the form was valid then you can check for the existence of a field with that class. $('#form input.invalid').length. Or you can set a boolean in the each function if you want to separate the logic from the HTML.


Loop through different controls and enabled the state of the whole group VBA I got the below code that is looping through all controls and changing the state in a after update event and enables new row for user to use. Everything works fine but i'm just wondering whether there is a better way to achieve the same result as what i have done is long winded. This will only enable the controls if the combobox has data if blank skip enable.

Private Sub Item1_AfterUpdate()
    Call UpdateItemsToClean
End Sub

Private Sub UpdateItemsToClean()
    Dim ctl As MSForms.Control
    Dim lbl As MSForms.Label
    Dim cmb As MSForms.ComboBox
    Dim txtbox As MSForms.TextBox
    Dim optbtn As MSForms.OptionButton
    Dim i As Integer

    For i = 2 To ItemsListFrame.Controls.Count
        For Each ctl In ItemsListFrame.Controls
            If TypeName(ctl) = "Label" Then
                If ctl.Tag = "GroupItem" & i - 1 Then
                    Set lbl = ctl
                    If Controls("Item" & i - 1).Value <> vbNullString Then
                        Controls("OrderLbl" & i).Enabled = True
                    End If
                End If

            ElseIf TypeName(ctl) = "ComboBox" Then
                If ctl.Tag = "GroupItem" & i - 1 Then
                    Set cmb = ctl
                    If Controls("Item" & i - 1).Value <> vbNullString Then
                        Controls("Item" & i).Enabled = True
                    End If
                End If

            ElseIf TypeName(ctl) = "TextBox" Then
                If ctl.Tag = "GroupItem" & i - 1 Then
                    Set txtbox = ctl
                    If Controls("Item" & i - 1).Value <> vbNullString Then
                        Controls("Qty" & i).Enabled = True
                        Controls("UnitPrice" & i).Enabled = True
                        Controls("SubTotal" & i).Enabled = True
                        Controls("Comments" & i).Enabled = True
                    End If
                End If
            ElseIf TypeName(ctl) = "OptionButton" Then
                If ctl.Tag = "GroupItem" & i - 1 Or ctl.Tag = "InOut" & i - 1 Then
                    Set optbtn = ctl
                    If Controls("Item" & i - 1).Value <> vbNullString Then
                        Controls("OptionIn" & i).Enabled = True
                        Controls("OptionOut" & i).Enabled = True
                    End If
                End If
            End If
        Next ctl
      Next i
End Sub

I rows are being added dynamically I would write a class to wrap each row of controls and respond to the combobox events.

Since there appears to be is a fixed number of rows, there is no reason to iterate over all the Controls in the userform.

These variables seem legacy code because they are set but never used.

Dim ctl As MSForms.Control
Dim lbl As MSForms.Label
Dim cmb As MSForms.ComboBox
Dim txtbox As MSForms.TextBox
Dim optbtn As MSForms.OptionButton

This code is repeated several times.

If Controls("Item" & i - 1).Value <> vbNullString Then
    Controls("OrderLbl" & i).Enabled = True
End If

The If statements can usually be eliminated when setting Boolean values.

Controls("OrderLbl" & i).Enabled = Controls("Item" & i - 1).Value <> vbNullString

A helper variable would make the code even easier to read.

Enabled = Controls("Item" &  i - 1).Value <> vbNullString
Controls("Qty" & i).Enabled = Enabled
Controls("UnitPrice" & i).Enabled = Enabled
Controls("SubTotal" & i).Enabled = Enabled
Controls("Comments" & i).Enabled = Enabled

Refactored Code

Private Sub UpdateItemsToClean()
    Const ItemCount As Long = 5
    Dim Enabled As Boolean
    Dim Index As Long
    Enabled = True
    For Index = 2 to ItemCount
        If Enabled Then Enabled = Controls("Item" & Index - 1).Value <> vbNullString
        Controls("OrderLbl" & Index).Enabled = Enabled
        Controls("Item" & Index).Enabled = Enabled
        Controls("Qty" & Index).Enabled = Enabled
        Controls("UnitPrice" & Index).Enabled = Enabled
        Controls("SubTotal" & Index).Enabled = Enabled
        Controls("Comments" & Index).Enabled = Enabled
        Controls("OptionIn" & Index).Enabled = Enabled
        Controls("OptionOut" & Index).Enabled = Enabled
    Next
End Sub


Idiomatic Leap years in haskell I wrote a simple function to determine if a year is a leap year or not. It works, but does not feel idiomatic.

isLeapYear :: Int -> Bool
isLeapYear year =
    (mod year 400) == 0 || ((mod year 4) == 0 && not ((mod year 100) == 0))

Is there a more "haskelly" way to write this function ?

One thing that always comes to my mind when seeing code like that is one of the first principles in programming that everyone learns.

Don't repeat yourself.

In your code, the part that stands out is mod year n == 0; let's get away with it first:

isLeapYear :: Int -> Bool
isLeapYear year = divisibleBy 400 || (divisibleBy 4 && not $ divisibleBy 100)
    where
        divisibleBy x = mod year x

Using such short helpers is very convenient in Haskell. In my opinion it's one of the most important reasons for the code staying readable (contrary to some newcomers' opinions who just pile up code in one giant expression).

Now, what I don't like here is the full name for the year; I know, I know, it's descriptive, but bear with me:

type Year = Int

isLeapYear :: Year -> Bool
isLeapYear y = divisibleBy 400 || (divisibleBy 4 && not (divisibleBy 100))
    where
        divisibleBy x = mod y x == 0

Now, this is the point where I'd really, really leave it be.


Linkedlist visualization using html5 canvas I have written a simple linked list visualization program in JavaScript:

I am just a beginner in html5 canvas. I am pretty much satisfied with how it works, but I´d like to know if I made some things, that could have been done better.

<!DOCTYPE html>
<html>
<head>
  <meta charset="utf-8">
  <meta name="viewport" content="width=device-width">
  <title>JS Bin</title>
</head>
<body>
<canvas id="canvas"></canvas>
<script>

var Shape=Shape || {
 x:0,
 y:0,
 w:0,
 h:0,
 t:0,
 rects:[],
 create:function(){
  var obj=Object.create(this);
  return obj;
 },
 add:function(x,y,w,h,t){
  this.x=x;
  this.y=y;
  this.w=w;
  this.h=h;
  this.t=t;
  this.rects.push({x,y,w,h,t});
 },
 draw:function(context,text){
  for (var i in this.rects) {
        oRec = this.rects[i];
        context.fillStyle = 'red'
        context.fillRect(oRec.x, oRec.y, oRec.w, oRec.h);
      context.fillText(oRec.t, oRec.x,oRec.y);
        context.beginPath();
       context.moveTo(oRec.x+oRec.w/2,oRec.y+oRec.h/2);
       context.lineTo(oRec.x+100,oRec.y+12);
       context.stroke();
       context.closePath();
       if(i==this.rects.length){

       }

    }
 }

};
window.onload=function(){
   var canvas=document.getElementById("canvas");
   var ctx=canvas.getContext("2d");
   var h=canvas.width=window.innerWidth;
   var w=canvas.height=window.innerHeight;
   var node=Shape.create();
   node.add(10, 100, 25, 25,1)
   node.add(100,100, 25, 25,2)
   node.add(200,100, 25, 25,3)
   node.add(300,100, 25, 25,4)
   node.add(400,100, 25, 25,5)
   node.add(500,100, 25, 25,6)
   node.draw(ctx);

};

</script>
</body>
</html>

I'm not sure why are you using Object.create to instantiate your shape. You should know that operator instanceof only works with real javascript pseudoclasses (that is, if you even think you need pseudoclass for this), which look like this:

function Shape() {
  this.x = 0;
  this.y = 0;
  this.w = 0;
  this.h = 0;
  this.t = 0;
  this.rects = [];
}
Shape.prototype = {
  add:function(x,y,w,h,t) {
    this.x=x;
    this.y=y;
    this.w=w;
    this.h=h;
    this.t=t;
    this.rects.push({x,y,w,h,t});
  },
  draw:function(context,text) {
    for (var i in this.rects) {
      oRec = this.rects[i];
      context.fillStyle = 'red'
      context.fillRect(oRec.x, oRec.y, oRec.w, oRec.h);
      context.fillText(oRec.t, oRec.x,oRec.y);
      context.beginPath();
      context.moveTo(oRec.x+oRec.w/2,oRec.y+oRec.h/2);
      context.lineTo(oRec.x+100,oRec.y+12);
      context.stroke();
      context.closePath();
      if(i==this.rects.length){
        // wtf is this exactly?
      }
    }
  }
}

There's also one very important difference, which is that in your case rects property is shared between all instances!. Here's a demonstration using your Shape. Here is the same example with my code.

Except for that, there's nothing much to say about the code. I'd be more interested how does it actually handle a linked list. Does it check for cycles? Does it show whether the list is circular or null-terminated?

Maybe just make the text black and put it one pixel up for readability. It looks better.

      context.fillStyle = 'black';
      context.fillText(oRec.t, oRec.x,oRec.y-1);


Provide functional-programming style solution for 2D Array - DS challenge Problem Statement:

  Given a 6×6 2D Array, arr:

1 1 1 0 0 0
0 1 0 0 0 0
1 1 1 0 0 0
0 0 0 0 0 0
0 0 0 0 0 0
0 0 0 0 0 0

  We define an hourglass in A to be a subset of values with indices
  falling in this pattern in arr's graphical representation:

a b c
  d
e f g

  There are 16 hourglasses in arr, and an hourglass sum is the
  sum of an hourglass' values. Calculate the hourglass sum for every
  hourglass in arr, then print the maximum hourglass sum.

  For example, given the 2D array:

-9 -9 -9  1 1 1
 0 -9  0  4 3 2
-9 -9 -9  1 2 3
 0  0  8  6 6 0
 0  0  0 -2 0 0
 0  0  1  2 4 0

  We calculate the following 16 hourglass values:

-63, -34, -9, 12,
-10, 0, 28, 23,
-27, -11, -2, 10,
9, 17, 25, 18

  Our highest hourglass value is 28 from the hourglass:

0 4 3
  1
8 6 6

  Note: If you have already solved the Java domain's Java 2D Array
  challenge, you may wish to skip this challenge.

  Function Description

  Complete the function hourglassSum in the editor below. It should
  return an integer, the maximum hourglass sum in the array.

  hourglassSum has the following parameter(s):

  arr: an array of integers

  Sample Input

1 1 1 0 0 0
0 1 0 0 0 0
1 1 1 0 0 0
0 0 2 4 4 0
0 0 0 2 0 0
0 0 1 2 4 0

  Sample Output

19

  Explanation

  arr contains the following hourglasses: ![enter image
  description here]2 The hourglass with the maximum sum (19) is:

2 4 4
  2
1 2 4

Following is my imperative style Solution:

def hourglassSum(arr: Array[Array[Int]]): Int = {
  def sum(rowNumber: Int, columnNumber: Int): Int = {
    var oneHourglassSum = 0
    for {
      i <- 0 to 2
      j <- 0 to 2
    } yield {
      val cumulativeSum = if (i == 1 && (j == 0 || j == 2)) 0
      else arr(i + rowNumber)(j + columnNumber)
      oneHourglassSum += cumulativeSum
    }
    oneHourglassSum
  }

  def max(x: Int, y: Int): Int = if (x > y) x else y

  var hourglassMaxSum = 0
  for {
    rowOffset <- 0 to 3
    columnOffset <- 0 to 3
  } yield {
    hourglassMaxSum = max(sum(rowOffset, columnOffset), hourglassMaxSum)
  }
  hourglassMaxSum
}

There's no need to write a max() method. The Standard Library provides for that.

A for comprehension, with a yield clause, produces a result. So instead of using a mutable var to capture and collect the results of the yield you should capture them directly.

val res = for {...} yield {...}

Or, if you want to process the results more directly, i.e. without the intermediate variable, you can use the somewhat awkward parentheses construct.

def hourglassSum(arr: Array[Array[Int]]): Int = {
  (for {
    x <- 0 to 3
    y <- 0 to 3
  } yield {
    arr(y)(x)   + arr(y)(x+1)   + arr(y)(x+2) +
                  arr(y+1)(x+1) +
    arr(y+2)(x) + arr(y+2)(x+1) + arr(y+2)(x+2)
  }).max
}


Deleting spaces in cells I have a VBA code sample that I use on a table to delete spaces in cells. I want to ask if someone has a better and quicker way to the Trim function on a table than what I have:

This is the sample:

If Sheets("Main").Cells(8, 6) = 1 Then
    strName = Sheets("Main").Cells(8, 3)
    Workbooks.Open Filename:="V:\á÷øåú ùëø\ãåçåú\ãåçåú çæåúééí\ãåçåú ìäîøä\exhr0101.csv"
    Columns(7).Delete
    Cells.Select
    Selection.Copy
    Windows("Trim all cells.xlsm").Activate
    Sheets("Data").Select
    Cells.Select
    Selection.PasteSpecial Paste:=xlPasteValues, Operation:=xlNone, SkipBlanks _
        :=False, Transpose:=False
    'ñâéøú çæåúé î÷åø
    Windows("exhr0101.csv").Activate
    Application.DisplayAlerts = False
    ActiveWindow.Close
    Application.DisplayAlerts = True
    'áéöåò Trim
    Windows("Trim all cells.xlsm").Activate
    Sheets("Data").Select
    For Each cell In ActiveSheet.UsedRange.SpecialCells(xlCellTypeConstants)
    cell = WorksheetFunction.Trim(cell)
    Next cell
    'äãá÷ä áâéìéåï äãá÷ä
    Cells.Select
    Selection.Copy
    Sheets("Target").Select
    Cells.Select
    Selection.PasteSpecial Paste:=xlPasteValues, Operation:=xlNone, SkipBlanks _
        :=False, Transpose:=False

Here is my specific trim:

For Each cell In ActiveSheet.UsedRange.SpecialCells(xlCellTypeConstants)
cell = WorksheetFunction.Trim(cell)
Next cell

You can avoid loop altogether.

 With Activesheet.UsedRange
     .Value = Application.Trim(.Value)
 End With

Warning: will give error if any cell has more than 255 characters.


Iter 2: Reusable, robust c++ std::hash for GMP's big integer type This is the 2nd iteration of a code review. The 1st iteration (completed) is at
Iter 1: Reusable, robust c++ std::hash<mpz_class> for GMP's big integer type
1. Goal
My intention is to provide a fast hashing algorithm to hash GMP's big integer type mpz_class and mpz_t so I can use these types as keys for an unordered_map. The code shall be reusable for others.
2. Current Approach
Since C++17, the standard library provides the specialization hash<string_view> which is used to produce the initial hash value.
First, the magnitude data of the big integer is wrapped into a string_view and then its hash value is calculated using hash<string_view>. This produces an initial hash value which only depends on the magnitude, but not on the sign, of the big integer.
To keep the hashes of positive and negative big integers different, the initial hash value is scrambled once for negative big integers only.
3. Code
File hash_mpz.h:
#ifndef HASH_MPZ_H_
#define HASH_MPZ_H_

#include <gmpxx.h>

namespace std {

template<> struct hash<mpz_srcptr> {
    size_t operator()(const mpz_srcptr x) const;
};

template<> struct hash<mpz_t> {
    size_t operator()(const mpz_t x) const;
};

template<> struct hash<mpz_class> {
    size_t operator()(const mpz_class &x) const;
};

}

#endif /* HASH_MPZ_H_ */

File hash_mpz.cpp:
#include "hash_mpz.h"
#include <cstddef>
#include <string_view>

constexpr size_t pi_size_t() {
    if (sizeof(size_t) == 4) {
        return 0xc90fdaa2; // floor(pi/4 * 2^32)
    } else if (sizeof(size_t) == 8) {
        return 0xc90fdaa22168c234; // floor(pi/4 * 2^64)
    } else {
        throw std::logic_error(
                "sizeof(size_t) not supported. only 32 or 64 bits are supported. you can easily add the required code for other sizes.");
    }
}

inline size_t scramble(size_t v) {
    return v ^ (pi_size_t() + (v << 6) + (v >> 2));
}

namespace std {

size_t std::hash<mpz_srcptr>::operator()(const mpz_srcptr x) const {
    string_view view { reinterpret_cast<char*>(x->_mp_d), abs(x->_mp_size)
            * sizeof(mp_limb_t) };
    size_t result = hash<string_view> { }(view);

    // produce different hashes for negative x
    if (x->_mp_size < 0) {
        result = scramble(result);
    }

    return result;
}

size_t hash<mpz_t>::operator()(const mpz_t x) const {
    return hash<mpz_srcptr> { }(static_cast<mpz_srcptr>(x));
}

size_t hash<mpz_class>::operator()(const mpz_class &x) const {
    return hash<mpz_srcptr> { }(x.get_mpz_t());
}

}

File main.cpp:
#include <iostream>
#include <gmpxx.h>
#include <unordered_map>

#include "hash_mpz.h"

using namespace std;

int main() {
    mpz_class a;

    mpz_ui_pow_ui(a.get_mpz_t(), 168, 16);

    cout << "a      : " << a << endl;
    cout << "hash( a): " << (hash<mpz_class> { }(a)) << endl;
    cout << "hash(-a): " << (hash<mpz_class> { }(-a)) << endl;

    unordered_map<mpz_class, int> map;
    map[a] = 2;
    cout << "map[a] : " << map[a] << endl;

    return 0;
}

4. Question
Is there anything which can benefit from further improvement?

Make functions that should not be exported static
Functions that should only be available locally should be marked static. This applies to pi_size_t() and scramble() in hash_mpz.cpp.
Avoid using std::endl
Use "\n" instead of std::endl, the latter is equivalent to "\n", but also forces a flush of the output. This is rarely necessary, and might hurt performance, especially when writing to a file, or when standard output is redirected to a file.
Consider not using namespace std
It's very good that you are not using namespace std in the header files. But consider not using it at all, since even if only used in .cpp files it can result in namespace conflicts that are hard to debug. If you do find yourself typing std:: a lot and want to avoid it, consider only importing the names that you do use from std::, like so:
using std::cout;
using std::unordered_map;

Final nitpicks

There's still one unnecessary std:: inside the namespace std block in hash_mpz.cpp.
You don't need return 0 at the end of main().