Prime Number calculation bottleneck Inspired by another question here: Sum of Prime numbers under 2 million, I decided to try and implement Eratosthenes' Sieve from scratch using the algorithm described in the article.

unsigned int CalculateSumOfPrimes(const unsigned int number) {
    if(number <= 1) return 0;
    if(number == 2) return 2;
    std::vector<unsigned int> listOfPrimes;
    CalculateListOfPrimes(listOfPrimes, number);
    unsigned int sum = 0;
    for(unsigned int i = 0; i < listOfPrimes.size(); ++i) {
        sum += listOfPrimes.at(i);
    }
    return sum;
}

void CalculateListOfPrimes(std::vector<unsigned int>& container, const unsigned int number) {
    if(container.size() != 0) {
        std::cout << "Container must be empty!" << std::endl;
        return;
    }
    unsigned int current_prime_check = 2;
    std::vector<unsigned int> listOfNonPrimes;
    while(current_prime_check < number) {
        for(unsigned int i = current_prime_check; i < number; i += current_prime_check) {
            if(i == current_prime_check) {
                container.push_back(i);
                continue;
            }
            if(std::find(listOfNonPrimes.begin(), listOfNonPrimes.end(), i) != listOfNonPrimes.end()) continue;
            listOfNonPrimes.push_back(i);
        }
        ++current_prime_check;
        while(std::find(listOfNonPrimes.begin(), listOfNonPrimes.end(), current_prime_check) != listOfNonPrimes.end()) {
            ++current_prime_check;
        }
    }

}

Currently the major bottleneck at large numbers (65536 or higher) is CalculateListOfPrimes(...) at 67% of the total 15 second run time.

Switching the usage from a std::vector to a std::list (since I'm only adding to the END of the the list, it seemed like a good idea), only saved 5%.

Looking at it, the inner while loop is going through EVERY possible number from the currently known non-prime number to the next unknown number, is there an algorithm to tell what the next number to check should be or whether or not the next number and potentially every number after it is guaranteed NOT to be in the list of primes?

Your main in-effeciency is here:

if(std::find(listOfNonPrimes.begin(), listOfNonPrimes.end(), i) != listOfNonPrimes.end())

For every non-prime you are a doing a search through the list of non primes. For both std::vector and std::list this is an O(n) operation.

The usual way of implementing this is to have an array where the removed elements are represented by their index into the array and thus represent a complexity of O(1) to both set and check if an element has already been set to false.

std::vector<bool> sieve(number + 1, true);

Then the following lines can be replaced:

if(std::find(listOfNonPrimes.begin(), listOfNonPrimes.end(), i) != listOfNonPrimes.end())
    continue;
listOfNonPrimes.push_back(i);

// Change too
sieve[i] = false;

Other things:

Every time through the loop you do a test:

if (i == current_prime_check) {
    container.push_back(i);
    continue;
}

This is only true the first element.

So hoist it out of the loop. Do the container.push_back(i); outside the loop and start i at the next position:

for (unsigned int i = 2 * current_prime_check; i < number; i += current_prime_check)
             //   ^^^^ Start one place up from where you were.

Replace the other find as well:

++current_prime_check;
while (std::find(listOfNonPrimes.begin(), listOfNonPrimes.end(), current_prime_check) != listOfNonPrimes.end()) {
    ++current_prime_check;
}

If you use the sieve this can be replaced by a simple test into the sieve:

++current_prime_check;
while (!sieve[current_prime_check]) {
    ++current_prime_check;
}

After Edit Comments:

We can make it quicker.

Your function that generates the prime:

void CalculateListOfPrimes(std::vector<unsigned int>& container, const unsigned int number)

You pass an array the function fills the array then you processes the array as the next stage. A better solution would be to pass a function that is applied to each prime as you find it. Then you can have a simple version that pushes the values into a container. Or alternatively you can have a function that just sums up the primes (so you do not even need the container or have to iterate over it).

So try:

template<typename Func>
void CalculateListOfPrimes(Func const& action, const unsigned int number);

Then you could use it like this:

struct PB
{
    std::vector<unsigned int>& cont;
    PB(std::vector<unsigned int>& c): cont(c) {}
    void operator()(unsigned int value) const { cont.push_back(value); }
};
…
std::vector<int> data;
CalculateListOfPrimes(PB(data), 2000000);

Or you can have a specific function

struct Sum
{
   unsigned int& sum;
   Sum(unsigned int& s) : sum(s) {}
   void operator()(unsigned int value) const { sum += value; }
};
…
unsigned int total;
CalculateListOfPrimes(Sum(total), 2000000);

Comments:

Don't use system("pause");. It's plainly platform specific. All you are doing is getting the program to wait. So read a line from the standard input.

std::getline(std::cin, line); // If you have other input you may need to flush first.

Hash define not a good move.

Prefer to use a function (there is no performance penalty as it will probably be inlined). If you must use a hash define then define it like this:

#define PAUSE  do { /* Action To Pause */ } while(false)

atoi()

primes = CalculateListOfPrimes(std::atoi(argv[2]));

Not standard. No way to check failures.

I would use boost::lexical_cast(argv[2]) it will throw an exception on failure.

Don't pass pointers around

DisplayResult(&primes, 0);

Change the function so you pass by const reference. This avoids the copy, stops you from accidentally modifying the array, and you can't accidentally pass a NULL so its always valid.

void DisplayResult(std::vector<bool> const& container, const unsigned int sum);

Truth values:

The result of the == operation is a boolean so you don't need to use a ternary operator on-top of that:

bool isSumArg = (std::strcmp("-s", argv[1]) == 0 ? true : false);

is exactly equivalent to the easier-to-read

bool isSumArg = (std::strcmp("-s", argv[1]) == 0);

Prime specific values:

A quick optimization here. You do not need this loop to go all the way to number.

You just need to loop as far as sqrt(number) after that nothing will affect the result.

while (current_prime_check < number) { … }

Make functions do one thing.

This function does lots of different things depending on the inputs that are not related.

void DisplayResult(std::vector<bool>* container, const unsigned int sum) {
    if(container == NULL && sum == 0) {
        return;
    }

    if(sum != 0) {
        std::cout << "Sum of Primes: " << sum << std::endl;
        return;
    } else if(container != NULL) {
        std::cout << "List of Primes:" << std::endl;
        for(unsigned int i = 1; i < container->size(); ++i) {
            if(container->at(i) == true) std::cout << std::setw(15) << std::right << i << std::endl;
        }
    }
}


JavaScript canvas animation slowing down and hogging memory I'm experimenting with moving shapes in JavaScript with a canvas tag.

I have one half circle (only stroked) which rotates on set intervals.

I've written the code so many more can be added. I can give starting rotations and a lot of other handy parameters.

I've tried this with twelve circles and it looks exactly like I want. Problem is, the animation slows down and the framerate goes down quite fast. And it uses heaps and heaps of memory. Any ideas on how to optimize this code to not consume so much?

Demo: http://jsfiddle.net/xaddict/fLE8z/

JS code:

var kin;
var canvas;
var context;
var pi = Math.PI;
var linearSpeed = 0.5*pi; // pixels / second
var linearDistEachFrame;
var myCircle;

function Circle(x, y, rot, radius, borderWidth, strokestyle){
    this.x = x;this.y = y;
    this.radius = radius;
    this.borderWidth = borderWidth;
    this.rot = rot
    this.goalrot = rot;
    this.strokestyle = strokestyle;
}

function randomizeThis(object){
    object.goalrot += pi;
}

function updateCircle(object){
    if(object.goalrot > object.rot){ //if goal rotation is bigger than current rotation
        object.rot += linearDistEachFrame;
    }else{ // if goal rotation is equal or smaller than current
            if(object.goalrot >= 2* pi){object.goalrot -= 2* pi;}
            object.rot = object.goalrot;
    }
}

function drawCircle(object){
    context.beginPath();
  context.arc(object.x, object.y, object.radius, object.rot, object.rot+pi, false);
  context.fillStyle = "none"; context.fill();
  context.lineCap = "round";
  context.lineWidth = object.borderWidth;
  context.strokeStyle = object.strokestyle;context.stroke();
}

function drawCirc(context,x,y,radius,clip){
    context.beginPath();
    context.arc(x,y,radius, 0, 2*pi, false);
    if(clip == true){context.clip();}
}

window.onload = function(){
    kin = new Kinetic_2d("canvas");
    canvas = kin.getCanvas();
    context = kin.getContext();

    myCircle =         new Circle(421,320,1.0*pi,    153,    12,"#FAA61A");//FAA61A

    kin.setDrawStage(function(){
        //update
        linearDistEachFrame = linearSpeed * kin.getTimeInterval() / 1000;
        updateCircle(myCircle);

        //clear
        kin.clear();

        //draw
        context.save();
        drawCirc(context,281,222,58,true);
        drawCircle(myCircle);
        context.restore();
    });

    kin.startAnimation();

    setTimeout(function(){randomizeThis(myCircle);setInterval("randomizeThis(myCircle)",4000);clearTimeout(this);},6000);
};

Thanks for all help!

There is no need to recalculate the linearDistEachFrame during each update. You may want to precompute 2 * Math.PI as well.


Performance is too low, online compiler wants a faster way to find all possible combinations of k numbers out of 1...n I have to find a method (using backtracking) to list all possible combinations, in lexicographical order,of k numbers out of the interval [1,n].. duplicates are not allowed. i.e: input: 5 3output:1 2 31 2 41 2 5 1 3 41 3 51 4 5 2 3 42 3 52 4 53 4 5

I know using namespace std; is bad practice, but that's the way we've been taught to write code, we always use iostream library and sometimes cmath iomanip and string, so the point is that I don't have a lot of knowledge in C++.. All I would like is a faster method than the one that I implemented, and if you can, don't use advanced syntax.. I understand that "advanced" might mean different things for different people, but maybe the code bellow will give you an uderstanding of the level of C++ knowledge I have

#include <iostream>
using namespace std;
int sol[20], n , el_maxim;
void display()
{
    for (int i = 0; i < n; i++)
        cout << sol[i] << " ";
    cout << endl;
}
int okay()
{
    for (int i = 0; i < n - 1; i++)
        if (sol[i] >= sol[i + 1])
            return 0;
    return 1;
}
void bkt(int poz)
{
    if (poz == n)
    {
        okay();
        if (okay() == 1)
            display();
    }
    else
        for (int i = 1; i <= el_maxim; i++)
        {
            sol[poz] = i;
            bkt(poz + 1);
        }
}
int main()
{
    cin >> el_maxim >> n;
    bkt(0);
    return 0;
}

Preface:

  if you can, don't use advanced syntax

I understand why you say this, but do not shy away from strange looking syntax.  When you encounter something you cannot read, take the time to lookup what the syntax does (even experienced developers do this).  The weird looking syntax may be doing useful stuff.  And after you understand what's going on, the syntax usually doesn't look weird anymore.

Another algorithm:

First off, for a beginner, this is a great start.  It's very simple code which is good.

If you want faster code, the first thing to do is make sure your general approach is reasonable or better yet optimal.  Your algorithm generates all possible sets of length K with elements from {1,2,...N} and then checks if the sets are sorted.  Note that I have used N for the size of the input set and K for the size of the combinations -- this is a convention I have seen in many places but is different from your code.

There are $$K^N$$ possible sets of length K from a set of N objects.  For the same set of N objects, there are $$\frac{N!}{K!(N-K)!}$$ combinations.  Since the combinations are a subset of the possible sets, they are a smaller set and in many cases they are a much smaller set.  Interestingly there are some non integer solutions for K^N < (N choose K) e.g. K = 0.5 and N = 1.5. maybe someone else can shed some light on that.  But they key takeaway is that only generating combinations will be a lot faster.

How can you only generate combinations?  There's more than one way to do this, but I will describe one way.

Consider the n=5, k=3 example.  The set of numbers to maybe print is 1,2,3,4,5, and the set of numbers to definitely print is {} (empty set).  I'll simplify this as ({}, {1,2,3,4,5}).  Some of the combinations have 1 and some don't. So we now have to consider both the possibilities ({1},{2,3,4,5}) and ({}, {2,3,4,5}).  We repeat this recursively, and when we have a set of things of size el_maxim (or, by convention, k), we print.  In psuedo-code:

void recurse(int indexK, int indexN) {
    if (indexK == K) {
        print(result);
        return;
    }

    // "if there more slots to fill than numbers to fill them"
    // could also say `if (indexN == N)` ... correct but slower
    if (N - indexN < K - indexK) {
        return;
    }

    result[indexK] = indexN;

    recurse(indexK + 1, indexN + 1);
    recurse(indexK, indexN + 1);
}

I encourage you to try getting this function to compile and then benchmarking to make sure you see the speedup you would expect.  You'll have to try a few different values of N and K.  For some inputs <10, the improved algorithm should run 1000s of times faster!

Review of your code:

Since my main advice is to use a better algorithm, I haven't given any specific comments on your code.  I'll do that here:

Your program should be based around a function that can generate combinations.  The function should take N and K as input, and it should return all the combinations.  Something like this:

/// This function generates all combinations of integers {1,2,...,N}
/// @param N    the size of the set
/// @param K    the size of the combinations
ResultType generateCombinations(int N, int K) { ... }

Then your main function should call generateCombinations.  This has several advantages over your method:

The compiler will make sure you set all the inputs.  In your version, if you forget to set a global variable, then you'll get the wrong answer.
You have a convenient place to put a big comment about what the function does.  The comment I've placed above is in Doxygen's format so you can generate documentation from your comments.  It also looks pretty good in my opinion.
You can easily call the function in several places.
You don't have to worry about name collisions.
The compiler can optimize arguments better than global variables.  In general, compilers are really good with functional code and not so good with code that uses lots of memory.
I could go on but I think you get the point.

Separate printing the combinations from generating the combinations.  You've sort of done that with the display function, but not really since anyone who calls bkt is forced to print the result.  There are two main ways to do this.  The first way is to return a big data structure, and then print it:

std::vector<std::vector<int>> generateCombinations(int N, int K) { ... }

for (auto& vec : genCombinations(5, 3)) { print(vec); }

This way is pretty easy/simple.  The downside is it uses a lot of memory.

The second way is to accept an argument that says what to do with each result:

void genCombinations(int N, int K, std::function<void(vector<int>&)> func) { ... }

genCombinations(5, 3, [](auto& vec) { print(vec); });

This is probably harder to read for many people, but it saves a large amount of memory in this case!

Your names are a little weird.  el_maxim instead of maximum_element or N as dictated by convention.  n instead of num_elements or K per the convention.  Also bkt and okay seem like they were chosen because they are short rather than because they are good descriptions of what they do.

okay();
if (okay() == 1)

Is this intended?

P.S.

I encourage you to rewrite and post another code review using a faster algorithm!  You may get more comments on your code rather than on your approach.


Searching for voters by address I have 3 tables, 1 where voters information is stored, 2 where their address is stored and 3 where voter ids are matched with address ids. I thought this approach was best as several voters can occupy the same address.

Now I have Query that selects all the voters on a given street / road. Although it works, I'm not sure if I should be using joins or keep using where, and if locate is the best way to search a field.

SELECT `voter_name`, `address_full`
FROM `tr_voter`, `tr_voter_address`, `tr_voter_address_assigned`
WHERE tr_voter_address_assigned.voter_id = tr_voter.voter_id
AND tr_voter_address_assigned.address_id = tr_voter_address.address_id
AND LOCATE('insert street name here', `address_full`)
ORDER BY `address_2`, `address_1`
LIMIT 0 , 200

address_full is the full address, where each component is broken down into address_1, address_2 etc. This does mean however, in some cases with house names, that a street may be stored in 1 2 or 3, so searching the full field seemed the full proof way to do it.

Voter Table

`voter_id` int(11) NOT NULL AUTO_INCREMENT,
`voter_name` varchar(255) COLLATE utf8_unicode_ci NOT NULL,
`voter_number` varchar(11) COLLATE utf8_unicode_ci NOT NULL,
 `voter_title` varchar(20) COLLATE utf8_unicode_ci NOT NULL,
`voter_first_name` varchar(255) COLLATE utf8_unicode_ci NOT NULL,
`voter_last_name` varchar(255) COLLATE utf8_unicode_ci NOT NULL,
`voter_notes` text COLLATE utf8_unicode_ci NOT NULL,
`voter_last_vote_date` datetime DEFAULT NULL,
`voter_entry_date` datetime NOT NULL,
`voter_last_update` timestamp NOT NULL DEFAULT '0000-00-00 00:00:00' ON UPDATE CURRENT_TIMESTAMP,
PRIMARY KEY (`voter_id`),
UNIQUE KEY `number` (`voter_number`)

Address Table

`address_id` int(11) NOT NULL AUTO_INCREMENT,
`address_full` varchar(255) COLLATE utf8_unicode_ci NOT NULL,
`address_1` varchar(255) COLLATE utf8_unicode_ci NOT NULL COMMENT 'address name or number',
`address_2` varchar(255) COLLATE utf8_unicode_ci NOT NULL,
`address_3` varchar(255) COLLATE utf8_unicode_ci NOT NULL,
`address_4` varchar(255) COLLATE utf8_unicode_ci NOT NULL,
`address_5` varchar(255) COLLATE utf8_unicode_ci NOT NULL,
`address_6` varchar(255) COLLATE utf8_unicode_ci NOT NULL,
`address_lat` varchar(255) COLLATE utf8_unicode_ci NOT NULL,
`address_long` varchar(255) COLLATE utf8_unicode_ci NOT NULL,
PRIMARY KEY (`address_id`),
UNIQUE KEY `address_full` (`address_full`)

Assigned Table

`assigned_id` int(11) NOT NULL AUTO_INCREMENT,
`voter_id` int(11) NOT NULL,
`address_id` int(11) NOT NULL,
PRIMARY KEY (`assigned_id`)

OK Assuming you need the "Assigned Table" to support the many to many relationship of a voter to address.

`assigned_id` int(11) NOT NULL AUTO_INCREMENT,
`voter_id` int(11) NOT NULL,
`address_id` int(11) NOT NULL,
PRIMARY KEY (`assigned_id`)

You don't need assigned_id. Remove this. The primary key is (voter_id, address_id)


Rewriting the code without use of GOTO This is a code automatically converted from VB 6.0 to C#.net
What is a safe and correct way to get rid of that GOTO statement?

public bool IsDirty(bool checkChildrenInd)
{
    bool result = false;
    //Determine if this object is 'dirty', or has been updated since it was created.

    if (Parent.NewInd)
    {
        return true;
    }

    if (backup == null)
    {
        goto CheckChildren; //Check any child lists
    }

    if (!String.Equals(MissingDataValue, backup.MissingDataValue))
    {
        return true;
    }

    if (!String.Equals(PreviewDataValue, backup.PreviewDataValue))
    {
        return true;
    }

    if (FieldOption != backup.FieldOption)
    {
        return true;
    }

CheckChildren:
    if (!checkChildrenInd)
    {
        return result;
    }
    return result;
}

EDIT: FIXED

This specific instance can be rewritten as

public bool IsDirty(bool checkChildrenInd) {
  return ! (  Parent.NewId ||
              backup == null ||
              String.Equals(MissingDataValue, backup.MissingDataValue) ||
              String.Equals(PreviewDataValue, backup.PreviewDataValue) ||
              FieldOption == backup.FieldOption );
}

if this pattern is always the same, but there may be other conditions as you note in the comments, I note that CheckChildren

Takes a parameter which is also the function parameter
returns on all code paths
result is never assigned to before the goto statement

If this is always true, you can factor it out. replace the CheckChildren label with a function:

private bool CheckChildren(bool checkChildrenInd){
    bool result = false;
    if (!checkChildrenInd)
    {
        return result;
    }
    return result;
}

and the goto CheckChildren; with return CheckChildren(checkChildrenInd);

if result is possibly set to true before the goto statement, you will need to pass result as a variable to the funcion too (and include it in the signature).


simple and extendable shop My task is to write a simple shop. The requirements are :

It can have multiple items
Each item its own price and quantity
Each customer basic identifier such as full name
Each customer receives clear receipt
Shop keeps the shopping history

It's meant to be flexible, easy to expand, high performance considered. I'd be grateful if someone could review it and point out what can/should be improved or changed. Thanks!

#include <iostream>
#include <vector>
#include <set>
#include <map>
#include <list>

using namespace std;

class receipt {
private:
    string prodName;
    int quant;

public:
    receipt(const string& prdNm, int qnt)
          : prodName(prdNm), quant(qnt)
    {}
    string getProdName() const { return prodName; }
    int    getQuant() const { return quant; }

};

class client {
private:
    string name;
    vector<receipt> receipts;

public:
    client(const string& n): name(n) {}
    string getName() const { return name; }

    void takeReceipt (const string& prodName, int quant){
        receipts.push_back( receipt(prodName, quant) );
    }

    void printReceipts(int num) const {
        // printing receipts
    }

    bool operator< (const client& c) const {
        return name < c.name;
    }
};

class product {
private:
    string name;
    int price;

public:
    product(const string& n, int p): name(n), price(p) {}
    string getName() const { return name; }
    int getPrice() const { return price; }

    bool operator< (const product& p) const {
        return name < p.name;
    }
};

class record {
private:
    string clientName;
    string prodName;
    int quant;

public:
    record(const string& clNm, const string& prdNm, int qnt)
          : clientName(clNm), prodName(prdNm), quant(qnt)
    {}
    string getClientName() const { return clientName; }
    string getProdName() const { return prodName; }
    int    getQuant() const { return quant; }

};

class shop {
private:
    set<client> clients;
    map<product, int> products;
    list<record> records;

public:
    void addClient(const client& cli) {
        clients.insert(cli);
    }

    void removeClient(const client& cli) {
        clients.erase(cli);
    }

    void addProduct(const product& pr, int quant) {
        products[pr] += quant;
    }

    void removeProduct(const product& pr, int quant) {

        if (products[pr] <= quant)
            products[pr] = 0;
        else
            products[pr] -= quant;
    }

    void doTransaction(client& cli, const product& prod, int quant) {

        if ( clients.find(cli.getName()) == clients.end() )
        {
            // client is not on the list of valid clients.
            return;
        }

        if (products[prod] >= quant)
        {
            // client is buying
            products[prod] -= quant;
            records.push_back( record(cli.getName(), prod.getName(), quant) );
            cli.takeReceipt(prod.getName(), quant);
        }
        else
        {
            // product not available in the quantity
        }
    }

    void printClients() const {
        // printing all the clients
    }

    void printRecords(int num) const {
        // Printing last records of previous transactions
    }

    void printProducts() const {
        // Printing all the available products
    }
};

int main(){

    shop s;
    client cm("Mike"), cj("John"), ca("Andy");
    product pot("potatoe", 5), app("apple", 7), nut("nut", 3);

    s.addClient(cm);
    s.addClient(cj);
    s.addClient(ca);
    s.addProduct(pot, 100);
    s.addProduct(app, 50);
    s.addProduct(nut, 20);

    s.removeClient(ca);
    s.removeProduct(pot, 20);
    s.removeProduct(nut, 20);

    s.doTransaction(cm, pot, 30);
    s.doTransaction(cj, app, 10);
    s.doTransaction(cm, nut, 5);
    s.doTransaction(cm, app, 5);

    return 0;
}

Don't do using namespace std;

Qualify names properly with std:: where necessary.

string getProdName() const { return prodName; }
int    getQuant() const { return quant; }

Avoid abbreviations as they harm code clarity. Most IDE's provide auto-completion, so abbreviations don't save typing anyway.

receipt(const string& prdNm, int qnt)
      : prodName(prdNm), quant(qnt)
{}

The initializer list allows you to use the same name for the function argument as for the member variable, so this would be fine:

receipt(std::string productName, int quantity)
      : productName(std::move(productName)), quantity(quantity)
{}

(Since the product name is a "sink argument" (we want to copy and store it locally), we can take it by value and move it into place).

I don't think it's a good idea to overload operator< for these classes. These aren't mathematical types, so there's no inherent ordered relationship. It would make just as much sense to sort by product price as by name.

We're using the operator< for sorting in a particular structure, so we want to associate the ordering with that structure, not with the type itself. We can do that using a "functor" class to define comparison, and specifying it as a template argument:

struct ProductLessThanByName
{
    bool operator()(const Product& a, const Product& b) const {
        return a.getName() < b.getName();
    }
};

...

    std::map<Product, int, ProductLessThanByName> products;

record is a rather ambiguous name. Maybe client_purchases or purchase_record or something might be better.

Since we already have the receipt class, that stores product and quantity, we could perhaps use that internally.

Although encapsulation is important, people often interpret it as "make all member variables private and add getter / setter functions". This is fine for classes that have associated logic in member functions, or where we can't change a member without extra work (we must adhere to the class invariants).

However, it's unnecessary for simple data containers. If we don't want to allow the user to change a member of the product or receipt classes, we can enforce that by giving them a const& to the class. So we can save a lot of work and use plain structs. e.g.:

struct  receipt
{
    std::string productName;
    int quantity;
};

It's probably better to use unsigned types for the quantity and price. We don't expect these to ever be negative, and it's easier to rule out invalid values.

As far as the design goes, I'm not sure we need to add clients outside of a transaction. I don't think it really makes sense to "remove" a client either. We have to keep the transaction history intact.

I think it would be helpful to separate the idea of "adding a product-type to the database" from "adding stock (actual items of a product-type) to the shop".

As Kyy13 notes in the comments, we need to store the price at which we sold an item in the transaction history.


Functional coffee dispenser Inspired by this question, I decided to rewrite this code in Scheme, as I'm working through SICP right now. Any feedback welcome.

#lang racket

;assume cup, is-empty?, refill and drink are operations understood by the environment

(define (coffee cup)
        (if (is-empty? cup)
        (coffee (refill cup))
        (drink cup)))

In a Lisp dialect (Scheme, Racket, or anything else), indentation is rather important.  It doesn't determine anything about how the code works, but being able to visually parse the code is necessary, because the alternative is to count parentheses.

With this indentation, you can visually see that the 2 lines after the if are part of the if statement:

(define (coffee cup)
  (if (is-empty? cup)
      (coffee (refill cup))
      (drink cup)))

I just copy-pasted the code into Emacs, told it to switch to scheme mode, and then told it to reindent.  You don't have to use Emacs, but you should use an editor that understands the Lisp dialect you're using.  If you mess up the parentheses, the editor will indent things oddly, and you can visually see that it's wrong without having to count parentheses.


Matrix OOP implementation using C++11 and C++14 I know that other people already did the same already,
but I'm learning C++ (C++11/14) coming from C (I'm reading Programming: Principles and Practice Using C++, by Bjarne Stroustrup), and this is my OOP implementation of a matrix (N by M size).

At the moment it doesn't use template for generic types and uses double, and all it does is represent a matrix, but doesn't implement any calculation function members yet.
I'm using the C++ PIMPL idiom (using C++11's unique_ptr and C++14's make_unique), and the subscript [] operator overload.

I'd like to know if I'm using a good programming pattern (or a bad antipattern) and in the correct C++ way of doing things (ex. program structure, files and organization, types etc...). Please if you can keep it simple, as I'm still missing parts of the language I haven't learned yet.

File main.cpp:

#include <iostream>
#include <memory>

#include "Matrix.hpp"

using namespace std;
using namespace zgt;

constexpr size_t num_elements{ 3};

/*
 Outputs:
 1, 2, 3
 4, 5, 6
 7, 8, 9
 */
int main() {
    Matrix m{ num_elements }; // Squared matrix

    double inc{ 1 };
    for (size_t i{ 0 }; i < num_elements; i++) {
        for (size_t j{ 0 }; j < num_elements; j++) {
            m[i][j] = inc++;
        }
    }

    for (size_t i{ 0 }; i < num_elements; i++) {
        for (size_t j{ 0 }; j < num_elements; j++) {
            if (j == 0) {
                cout << m[i][j];
            } else {
                cout << ", " << m[i][j];
            }
        }

        cout << endl;
    }

    return 0;
}

File Matrix.cpp:

#include "Matrix.hpp"

using namespace std;
using namespace zgt;

struct Matrix::_impl {
    unique_ptr<unique_ptr<MatrixRow>[]> _m;

    _impl(size_t rows, size_t cols): _m(make_unique<unique_ptr<MatrixRow>[]>(rows)) {
        for (size_t i{ 0 }; i < cols; i++) {
            _m[i] = make_unique<MatrixRow>(cols);
        }
    }
};

MatrixRow& Matrix::operator[](size_t index) {
    return *(pimpl->_m[index]);
}

Matrix::Matrix(size_t rows, size_t cols) : pimpl(make_unique<_impl>(rows, cols)) {}
Matrix::Matrix(size_t size) : Matrix::Matrix(size, size) {}
Matrix::~Matrix() = default;

File Matrix.hpp:

#ifndef Matrix_hpp
#define Matrix_hpp

#include <iostream>
#include <memory>

#include "MatrixRow.hpp"

namespace zgt {
    class Matrix{
        struct _impl;
        std::unique_ptr<_impl> pimpl;
    public:
        Matrix(size_t rows, size_t cols);
        Matrix(size_t size); // [size][size]
        ~Matrix();

        MatrixRow& operator[](size_t index);
    };
}

#endif

File MatrixRow.cpp

#include "MatrixRow.hpp"

using namespace std;
using namespace zgt;

struct MatrixRow::_impl {
    unique_ptr<double[]> _v;

    _impl(size_t cols): _v(make_unique<double[]>(cols)) {}
};

double& MatrixRow::operator[](size_t index) {
    return pimpl->_v[index];
}

MatrixRow::MatrixRow(size_t cols) : pimpl(make_unique<_impl>(cols)) {}
MatrixRow::~MatrixRow() = default;

File MatrixRow.hpp:

#ifndef MatrixRow_hpp
#define MatrixRow_hpp

#include <iostream>
#include <memory>

namespace zgt {
    class MatrixRow{
        struct _impl;
        std::unique_ptr<_impl> pimpl;
    public:
        MatrixRow(size_t cols);
        ~MatrixRow();

        double& operator[](size_t index);
    };
}

#endif

Lets start from the usage:

It is not possible to write range loops with the current version
no rowcount() and columncount() (or anything similar). So it is not possible to actually traverse matrix
operator[]() won't work on const objects, since there is no operator[](std::size_t index) const.

Clarity and cleanliness:

nuke pimpl. I'm serious, it doesn't add much benefit here. In fact, the compilation might be increased by pimpl, since matrix is very lightweight anyway.
#include <iostream> header is not needed, should be removed.
double indirection. _impl should actually be MatrixRow. But if pimpl is gonna get nuked, it will be embedded into matrix itself.
std::vector<> would be much better fit here.

Nitpicks:
destructor is generated for you automatically. defaulting it is redundant.

Some general guidelines:

OOP is usually mixed with some functional programming in C++. For example, templates.
C++ code usually tries to provide some default behavior that is correct and safe, and some access to underlying implementation that when accessed lets programmer to take all of the control to himself (literally everything).

Recommendation for next iteration:

I think that std::vector<T> will be the best fit here. Calculating the index to jump at should be pretty easy (it is what compiler does for us anyway). I think it will be okay to return T* from operator[](), because users know they shouldn't mess with it, but making some small struct would good as well. Something like std::array_view<T> would be nice (though it seems like it is not in the standard yet, even in C++17). Also, you get iterators for free with vector approach.

Just write:

auto begin()
{
    return storage.begin();
}

auto end()
{
    return storage.end();
}

As a result, you get range loop for free as well.


Validating sudoku columns and rows I'm writing a sudoku col/row validator, what is does is:

Reads user input on how many Sudoku instances to validate.
Reads the sudoku matrix grid 9x9 (for me, in my specific case 9x9) into a 2d array. Each line read/row should contain something like: 8 2 1 3 6 4 9 7 5.
Validates each row and column by flipping a 9 bit bitset to test whether all columns and rows contains nums from 1-9.

Please note that the only purpose of this program is to validate sudoku columns/rows as specified and not to completely validate it. i.e also checking each sudoku block.

How can I improve my code? I believe there's lots to it, naming conventions, fundamental approach...etc..etc.

#include <iostream>
#include <sstream>
#include <bitset>

namespace Constants
{
    constexpr int ROW_COL_SIZE = 9;
    const std::string STR_YES = "YES";
    const std::string STR_NO = "NO";
}

template<unsigned int N>
class Sudoku
{
private:
    int m_matrix[N][N] {{0}};
public:
    void ReadRows();
    std::string strIsValid() const;
};

template<unsigned int N>void Sudoku<N>::ReadRows()
{
    for(unsigned int row{0}; row<N; row++)
    {
        static std::string line;
        std::getline(std::cin, line);
        std::istringstream issline(line);
        int readnum {0};
        for(unsigned int i{0}; i<N; i++)
        {
            issline >> readnum;
            m_matrix[row][i] = readnum;
        }
    }
}

template<unsigned int N>std::string Sudoku<N>::strIsValid() const
{
    //9bit default ctor all zeroes, 000000000
    std::bitset<N> bitRow[N];
    std::bitset<N> bitCol[N];
    for(int i{0}; i<N; i++)
    {
        for(int j{0}; j<N; j++)
        {
            bitRow[i].flip(m_matrix[i][j]-1); //verify nums 1-9 row, bitset index is 0 not 1.
            bitCol[i].flip(m_matrix[j][i]-1); //verify nums 1-9 col
        }
        if(!bitRow[i].all() && !bitCol[i].all())
        {
            return Constants::STR_NO;
        }
    }
    return Constants::STR_YES;
}

int main(int, char**)
{
    int instances {0};
    std::cin >> instances;
    std::cin.clear();
    std::cin.ignore();

    for(int i{0}; i<instances; i++)
    {
        std::cout << "Instance " << i+1 << std::endl;
        Sudoku<Constants::ROW_COL_SIZE> sudokuInstance;
        sudokuInstance.ReadRows();
        std::cout << sudokuInstance.strIsValid() << std::endl << std::endl;
    }

    return 0;
}

It is not a complete answer. Just some of my thoughts about this code.

C++

Do not compare integers of different signedness. Loop counters i and j in function strIsValid are declared as int, but they are compared with N which is unsigned int. It could lead to some problems. Make the counters unsigned.
Use strict compilation flags. The compiler would tell you about your signed/unsigned issue if you pass it -Wall key.

You should always achieve that the compiler will not display any diagnostic messages with the most strict compilation keys (-Wall, -Wextra, -pedantic, etc).

But you probably would not want to use Clang's -Weverything.
What happened with your main? Your main function looks very unnatural:

int main(int, char**)

If you do not need argc and argv, write just

int main()

About std::endl. You should avoid globally using std::endl. It is not the same as just \n. There is a good answer on SO about this topic.

And why are you printing std::endl twice?

std::cout << sudokuInstance.strIsValid() << std::endl << std::endl;

You do not have to explicitly return from main. You don't have to explicitly return 0; at the end of main.

Programming

Minimize the scope of local variables. You should always minimize the scope of local variables. In your case, for example, scope of the readnum variable can be reduced. Some reading about this topic.
The predicative function should return boolean. Your strIsValid is a predicative function. It means that this function checks something and returns either true or false. But you return string "YES" (true) or "NO" (false). You should return bool instead.
What the strIsValid function does? The name of this function doesn't answer the question.

Architecture

Validate user's input. At this time you do not validate user's input in ReadRows. What if I'll input a number which is greater than 9? Or less than 0? If I'll input too many numbers?..

In this case, you store invalid data into m_matrix. Then this invalid data will be used as arguments of std::bitset's flip() which will cause std::out_of_range.
Split row and cols validating and reading user's input. The Sudoku class should know nothing about reading user's input. The only thing that it should do is validate Sudoku columns and rows! This is called the Single Responsibility Principle (SRP).


Two squares or not two squares - constant time optimization I have been trying to solve this problem, and after a bit of googling, I realised a \$O(\sqrt{n})\$ time complexity works fine (though the algorithm suggested is different from that I have used). My algorithm is as follows (pseudo code):

for i in range(sqrt(n/2+1)):
    if n-i*i is a perfect square:
        return True
return False

The exact code is as follows:

#include<stdio.h>
#include<stdlib.h>
#include<math.h>
#define MAX 100000000
typedef unsigned long long int llu;
int isSquare(llu n){
    if(n%16!=0 && n%16!=1 && n%16!=9 && n%16!=4)
        return 0;//as a square can only be of the form 16n+r ,where r belongsto {0,1,4,9}
    //The above step prevents use of expensive function sqrtl() in 75% cases
    llu root = sqrtl(n);
    return root*root == n;
}
int isSqSum(llu n){
    if(n%4==3)//as sum of two squares cannot be of the form 4n+3
        return 0;
    llu i;
    for(i=0;i*i<n/2+1;++i)
        if(isSquare(n-i*i))
            return 1;
    return 0;
}
int main()
{
    int c ;
    scanf("%d",&c);
    llu n;
    while(c--){
        scanf("%llu",&n);
        if(isSqSum(n))
            printf("Yes\n");
        else
            printf("No\n");
    }
    return 0;
}

As you can see, I have already applied most of the optimizations I could think of, but I still keep getting TLE. Is there something wrong with my code or is it that the algorithm that I am using has a large constant of proportionality in time complexity?

llu root = sqrtl(n); is not safe.  The precision of a long double is not specified by C to give the exact correct answer for all unsigned long long.
Not sure yet why you get TLE (Time Limit Exceeded).  As the page you point to only has 10 small integers to test, there must be horribly wrong with your compilation or yu have an unstated interesting input.
Spent 30 minutes tracking down a bug because I forgot to a braces when extending the following.  Instead, highly reccomend to add breaces in all for(), if(), etc. except maybe in the the most trivial circumstance.

for(i=0;i*i<n/2+1;++i)
  if(isSquare(n-i*i))
    return 1;

// Better
for(i=0;i*i<n/2+1;++i) {
  if(isSquare(n-i*i)) {
    return 1;
  }
}


Is this form submission validator professional? I have written some jQuery code for AJAX form submission:

jQuery(function() {
    jQuery('#message').click(function() {
        var btn = jQuery(this)
        var Name = jQuery("#Name").val();
        var Email = jQuery("#Email").val();
        var Sub = jQuery("#Sub").val();
        var Message = jQuery("#Message").val();
        if (Name == "") {
            jQuery('#Name').focus();
            jQuery("#Name").after('');
        }
        else if (Email == "") {
            jQuery("#Email").focus();
            jQuery("#Email").after('');
        }
        else if (Sub == "") {
            jQuery("#Sub").focus();
            jQuery("#Sub").after('');
        }
        else if (Message == "") {
            jQuery("#Message").focus();
            jQuery("#Message").after('');
        }
        else if (!validateEmail(Email)) {
            jQuery("#Email").focus();
        }
        else {
            btn.button('loading')
            $.ajax({
                url: '/application',
                dataType: 'text',
                type: 'post',
                data: {querytype: 'form', name: Name, email: Email, sub: Sub, message: Message},
                success: function(data)
                {
                    jQuery('#success').fadeIn();
                }
            });
        }
    });
});

Is this code good to go? Or it can be improved?  I have seen this where they check the validation after form submission, or this.

Should I check validation before or after submission?

Will my approach be good if I need to handle 10+ input elements?

A few things:

Variables in JavaScript should be camelCased. PascalCase is traditionally left for types.
CSS classes are generally lower-case and hyphenated. Instead of #Name, consider #name (and the corresponding HTML attribute, id='name')
Use $ instead of jQuery. $ is syntactic sugar that essentially resolves the same thing, but it's one character instead of 6, and it's more conventionally used (see below)
Consider using the jquery.validate library. This will make your validation significantly less painful. If you're using ASP MVC you can use jquery.validate.unobtrusive to automatically map your validation on your view model from C# to JavaScript.
Keep your semi-colon conventions consistent. Use them at the end of every statement, or don't use them at all.
If you're going to use $('#element') calls a lot, consider calling that $('#element') once and storing it inside of a variable inside the scope you are using that element.

With regards to validation, you should always perform validation on the server side. I cannot overstate that point. You must do it. Client-side validation is a bonus that should be performed in addition to server-side validation, but should not be relied upon and you should not make it core functionality to your site (users can easily disable JavaScript, and some browsers like Tor have JavaScript disabled by default).

Remember that the golden rule in programming is that end-users are stupid. The weakest point to any application is the point that the end-user is allowed to provide input. You should treat the data received from the form as such, even if you already have client-side validation.

Your approach will work with 10+ input elements, but you will be repeating a lot of code. As suggested above it's advisable for you to check out jquery.validate which will allow you to maintain validation rules a lot easier (and DRY up your validation)

I don't think you should be concerned whether or not your code is professional but rather that it signals it's intent to other developers clearly and it is maintainable. Your code is certainly clear in what it tries to do it but strikes me as scaling very badly in terms of maintainability, and I think you should address that.

Example of jQuery Validate

Well, I've only really used jquery.validate.unobtrusive, but here's a link to the library. I'm sorry I can't provide much more info on that, as most of it was done automatically for me behind the scenes by ASP MVC 4.

jQuery in WordPress

  Wordpress automatically no-conflicts jQuery as a matter of courtesy. If you're only using jQuery, you can override this and just put in a $ = jQuery; somewhere. Better yet, you can use jQuery(function($) { ... to alias $ to jQuery inside your ready handler. – Schism

Or, as pointed out by @Pinoniq in his answer, you can wrap it in an IIFE (aka self-executing functions).


To calculate the total CPU usage as a percentage I have been developing the below method to calculate the CPU usage of a Linux system at a point in time, as a percentage of the total number of cores available in the system.

#!/bin/bash

# Get number of cores from proc cpuinfo
CORECOUNT=`grep -c ^processor /proc/cpuinfo`
# Use top, skip the first 7 rows, count the sum of the values in column 9 - the CPU column, do some simple rounding at the end.
CPUUSAGE=`top -bn 1 | awk 'NR>7{s+=$9} END {print s/'$CORECOUNT'}' | awk '{print int($1+0.5)}'`
printf "$CPUUSAGE\n"

Would this be considered a reasonably accurate way of doing this? Any suggestions to increase efficiency/accuracy would be greatly appreciated.

There are a few problems you have here. The first, is the concept of getting the usage at a point in time, which is not possible. But, you can get it for a span of time. You are using top -bn 1 which will get the usage for the span of a second.

Since you are accessing the /proc system to get the number of CPU's, you may as well use the right /proc file for what you want:

cat /proc/stat:

 ~ $ cat /proc/stat
cpu  177206 161827 1274669 3582291272 144435 154779 0 0 0 0
cpu0 32170 48533 914773 894903107 30650 69784 0 0 0 0
cpu1 52790 41088 180295 895685563 76035 14410 0 0 0 0
cpu2 18745 40955 49422 895959636 11295 2234 0 0 0 0
cpu3 73500 31249 130178 895742965 26454 68350 0 0 0 0
intr 339571688 48 10 0 0 0 0 2 0 1 0 0 0 152 0 0 8750527 0 322394 40428015 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0
......
0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0
ctxt 660707018
btime 1401123121
processes 196075
procs_running 1
procs_blocked 0
softirq 322202763 0 136092603 1792784 40737199 4701209 0 6 133008518 51951 5818493

With the above file, I would calculate it manually.... (also using the documentation here: http://www.mjmwired.net/kernel/Documentation/filesystems/proc.txt#1212 )

I have taken the liberty of programming it myself.... since the recommendation is sucha  deviation from your solution, it makes sense:

#!/bin/bash

#CORECOUNT=$(grep cpu /proc/stat | grep -v 'cpu ' | wc -l)
#echo $CORECOUNT cores not needed for calculation though

DELAY=${1:-1}

function getstat() {
    grep 'cpu ' /proc/stat | sed -e 's/  */x/g' -e 's/^cpux//'
}

function extract() {
    echo $1 | cut -d 'x' -f $2
}

function change() {
    local e=$(extract $ENDSTAT $1)
    local b=$(extract $STARTSTAT $1)
    local diff=$(( $e - $b ))
    echo $diff
}

#Record the start statistics

STARTSTAT=$(getstat)

sleep $DELAY

#Record the end statistics

ENDSTAT=$(getstat)

#http://www.mjmwired.net/kernel/Documentation/filesystems/proc.txt#1236
#echo "From $STARTSTAT"
#echo "TO   $ENDSTAT"
#     usr    nice   sys     idle       iowait irq    guest
#From 177834 168085 1276260 3584351494 144468 154895 0 0 0 0
#TO   177834 168085 1276261 3584351895 144468 154895 0 0 0 0

USR=$(change 1)
SYS=$(change 3)
IDLE=$(change 4)
IOW=$(change 5)

#echo USR $USR SYS $SYS IDLE $IDLE IOW $IOW

ACTIVE=$(( $USR + $SYS + $IOW ))
TOTAL=$(($ACTIVE + $IDLE))
PCT=$(( $ACTIVE * 100 / $TOTAL ))

echo "BUSY $ACTIVE TOTAL $TOTAL $PCT %"


In memory data access for CSV files that may have changed I'm reading data from CSV files and store the result in a simple DataContext instance that gets injected throughout my application.

public class DataContext
{
    private static readonly IDictionary<Type, IEnumerable<BaseEntity>> EntityMap =
        Assembly.GetExecutingAssembly().GetTypes()
        .Where(t => t.IsClass && !t.IsAbstract && t.IsSubclassOf(typeof(BaseEntity)))
        .ToDictionary(t => t, t => Enumerable.Empty<BaseEntity>());

    public IEnumerable<T> Get<T>() where T : BaseEntity
    {
        return EntityMap[typeof(T)].Cast<T>();
    }

    public void Set<T>(IEnumerable<BaseEntity> value) where T : BaseEntity
    {
        EntityMap[typeof(T)] = value.ToList();
    }
}

I listen for changes in a folder. As soon as a CSV file gets updated, I'll update the corresponding list. Something like this:

void OnFileChange(string fileName)
{
    // handle in a separate thread to decrease the load of FileSystemWatcher
    Task.Run(() => {
        var parsedFileData = parser.Parse<DataType>(fileName);
        dataContext.Set<DataType>(parsedFileData);
    });
}

I never do a delta update of entities. It's always a complete replacement of the collection, which is by intention.

Some thoughts:

What do you think of this in memory approach with variables vs something like Redis?
Any other comments?

It's always a good idea to try to design LINQ compatible classes. Your  DataContext can be designed this way.

first you need to un-static-fy the field, it shouldn't be static (rarely anything has to be)
implement the ILookup interface as it behaves exactly as your data-context, this is, return an empty collection if the key hasn't been found. Internally you can use a dictionary
there is no need to scan the types and fill the dictionary with empty collections

Example:

public class DataContext : ILookup<Type, BaseEntity>
{
    private readonly IDictionary<Type, BaseEntityGroup> _items = new Dictionary<Type, BaseEntityGroup>();

    public bool Contains(Type type) => _items.ContainsKey(type);
    public int Count => _items.Count;
    public IEnumerable<BaseEntity> this[Type type]
    {
        get => _items.TryGetValue(type, out BaseEntityGroup result) ? result : Enumerable.Empty<BaseEntity>();
        set => _items.Add(type, new BaseEntityGroup(type, value));
    }
    public IEnumerator<IGrouping<Type, BaseEntity>> GetEnumerator() => null;
    IEnumerator IEnumerable.GetEnumerator() => GetEnumerator();
}

This requries another helper class for storing the items, so let's create one. It needs to implement the IGrouping interface:

public class BaseEntityGroup : List<BaseEntity>, IGrouping<Type, BaseEntity>
{
    public BaseEntityGroup(Type type, IEnumerable<BaseEntity> items) : base(items) => Key = type;
    public Type Key { get; }
}

Now you can work with it like this:

var dc = new DataContext();
dc[typeof(string)] = new BaseEntity[]
{
    new BaseEntity2(),
    new BaseEntity2()
};

but you want to use some generics? So why not create a two extensions for it:

public static DataContext Add<T>(this DataContext context, IEnumerable<BaseEntity> items) where T : BaseEntity
{
    context[typeof(T)] = items;
    return context;
}

public static IEnumerable<T> Get<T>(this DataContext context) where T : BaseEntity
{
    return context[typeof(T)].Cast<T>();
}

Should you now need to query your data-context you can easily do it with LINQ.


Specify table data in code, reference objects public final class Descriptors {
public static final EnumValueDescriptor TIME_SETTING = new EnumValueDescriptor(
            R.string.pref_label_time_setting,
            Measures.TIME_SETTING_12, R.string.pref_time_setting_12,
            Measures.TIME_SETTING_24, R.string.pref_time_setting_24);

public static final EnumValueDescriptor MEASURE_BG = new EnumValueDescriptor(
        R.string.pref_label_bg,
        Measures.BG_MMOL_L, R.string.measure_mmol_l,
        Measures.BG_MG_DL, R.string.measure_mg_dl);

public static final EnumValueDescriptor MEASURE_СH = new EnumValueDescriptor(
        R.string.pref_label_ch,
        Measures.CH_G, R.string.measure_g,
        Measures.CH_EXCH, R.string.measure_exch);

    // Is there a better way to specify table data?
public static final NumericValueDescriptor TBG_MMOL_L = new NumericValueDescriptor(
        R.string.pref_label_target_blood_glucose,
        ValueFormat.ONE_SIGNIFICANT_DIGIT,
        0.0f,
        40.0f,
        0.1f,
        R.string.measure_mmol_l);

public static final NumericValueDescriptor TBG_MG_DL = new NumericValueDescriptor(
        R.string.pref_label_target_blood_glucose,
        ValueFormat.INT,
        0f,
        300f,
        1f,
        R.string.measure_mg_dl);

public static final NumericValueDescriptor CF_MMOL_L_U = new NumericValueDescriptor(
        R.string.pref_label_correction_factor,
        ValueFormat.ONE_SIGNIFICANT_DIGIT,
        0.1f,
        99.0f,
        0.1f,
        R.string.measure_mmol_l_u);

public static final NumericValueDescriptor CF_MG_DL_U = new NumericValueDescriptor(
        R.string.pref_label_correction_factor,
        ValueFormat.INT,
        1f,
        99f,
        1f,
        R.string.measure_mg_dl_u);

public static final NumericValueDescriptor MF_G_U = new NumericValueDescriptor(
        R.string.pref_label_meal_factor,
        ValueFormat.ONE_SIGNIFICANT_DIGIT,
        0.1f,
        99.0f,
        0.1f,
        R.string.measure_g_u);

public static final NumericValueDescriptor MF_U_EXCH = new NumericValueDescriptor(
        R.string.pref_label_meal_factor,
        ValueFormat.INT,
        1f,
        99f,
        1f,
        R.string.measure_u_exch);

public static final int TYPE_TBG_MMOL_L = 0;
public static final int TYPE_TBG_MG_DL = 1;
public static final int TYPE_CF_MMOL_L_U = 2;
public static final int TYPE_CF_MG_DL_U = 3;
public static final int TYPE_MF_G_U = 4;
public static final int TYPE_MF_U_EXCH = 5;

    // Actually this is mapping of TYPE_TBG_MMOL_L  to an object, how better link constants above with objects?
public static final NumericValueDescriptor[] NUMERIC_DESCRIPTORS = {
    TBG_MMOL_L,
    TBG_MG_DL,
    CF_MMOL_L_U,
    CF_MG_DL_U,
    MF_G_U,
    MF_U_EXCH
};
 }

This class doesn't have any responsibilities, it just holds a group of static variables.  It is unclear what the purpose of these variables are.  I am sure there is a better way to achieve your goal as I doubt you actually need to be storing these details here.


Concurrent programming with a simple server I am creating a simple server that accepts a int and returns the value received twice:

public class MyServer{

    private static int port = 1234;

    public static void main(String args[]){
        try{
            ServerSocket serversock=new ServerSocket(port);
            while(true){
                Socket  socket=serversock.accept();
                new Thread(new MyClass(socket)).start();
            }
        }catch(IOException e){}
        }
    }

    class MyClass implements Runnable{

        Socket socket;

        public MyClass(Socket s){socket = s;}

        public void run(){
            try{
                DataInputStream in = new DataInputStream(socket.getInputStream());
                DataOutputStream out = new DataOutputStream(socket.getOutputStream());
                int x = in.readInt();
                out.write(2*x);
                socket.close();
            }
            catch(IOException e){}
    }
}

My question is about redesigning the code. Are there any weaknesses in the design?
If yes, then what changes would make the code better?

If you really are planning on creating a thread for each incoming request just use and Executors.newCachedThreadPool().  As long as the incoming requests arent very often, this ExecutorService will create a new thread when you submit to it, but if a thread is idle and not being used, the service will reuse the thread instead of creating a new one.

Edit: Here is the example using your code.

public class MyServer{

    private static int port = 1234;

    public static void main(String args[]){
        try{
            final ExecutorService service = Executors.newCachedThreadPool();
            ServerSocket serversock=new ServerSocket(port);
            while(true){
                Socket  socket=serversock.accept();
                service.submit(new MyClass(socket));
            }
        }catch(IOException e){}
        }
    }

    class MyClass implements Runnable{

        Socket socket;

        public MyClass(Socket s){socket = s;}

        public void run(){
            try{
                DataInputStream in = new DataInputStream(socket.getInputStream());
                DataOutputStream out = new DataOutputStream(socket.getOutputStream());
                int x = in.readInt();
                out.write(2*x);
                socket.close();
            }
            catch(IOException e){}
    }
}

With this you have the possibility of thread reuse.


Using sed regular expression to extract domain name from file I'm learning regex with sed to extract the last field from file named "test". The method I'm trying gives desired output.
Please suggest if this method Im trying is effective way of doing it. Also when should we use "-e" option with sed (please give an example — I couldn't find examples)

~# ] cat test
example.com.        4   IN  NS  b.iana-servers.net.
50times.com.        21556        IN      NS      ns1.50times.com.
example.com.        4   IN  NS  a.iana-servers.net.

~# ] cat test | sed -r 's/^[[:alnum:]]*.[[:alnum:]]*.?[a-z]*.[[:blank:]]+[0-9]+[[:blank:]]+IN[[:blank:]]+[A-Z]+[[:blank:]]+//g' | sed -r 's/\.*.$//'
b.iana-servers.net
ns1.50times.com
a.iana-servers.net

When processing tabular data in columns, awk is often a more appropriate tool to use.  The equivalent command would be

awk '{ sub("\.$", "", $NF); print $NF }' test

… which I think is more readable.

Explanation:

NF is the number of fields: for this text, 5.
$NF is the content of the last (5th) field.
sub("\.$", "", $NF) strips the trailing dot from the last field.
{ commands } executes the commands for every line in the file.


My JavaScript calculator I am playing a bit with vanilla JavaScript and created a simple JavaScript calculator using eval().  Any suggestions for improvements?

var keys = document.getElementsByClassName('number'),
    getTotal = document.getElementById('showTotal'),
    btnReset = document.getElementById('reset'),
    visor = document.getElementById('visor');

for (var i = 0; i < numbers.length; i++) {
  numbers[i].onclick = addNum;
}

getTotal.onclick = showResult;
btnReset.onclick = clearVisor;

function addNum() {
  visor.value += this.value;
}

function showResult() {
  var total = 0;
  total = visor.value;
  visor.value = '';
  visor.value = eval(total);
}

function clearVisor() {
  visor.value = '';
}

<form name="calc">

    <input type="text" name="visor" id="visor">

    <input type="button" class="keys" value="7"> <input type="button" class="keys" value="8"> <input type="button" class="keys" value="9">

    <input type="button" class="keys" value="4"> <input type="button" class="keys" value="5"> <input type="button" class="keys" value="6">

    <input type="button" class="keys" value="1"> <input type="button" class="keys" value="2"> <input type="button" class="keys" value="3">

    <input type="button" class="keys" value="/"> <input type="button" class="keys" value="*"> <input type="button" class="keys" value="-">

    <input type="button" class="keys" value="+"> <input type="button" id="reset" value="c"> <input type="button" id="showTotal" value="=">

  </form>

A few notes:

Shy away from eval(). TL;DR: impacts performance and is a security risk.  I don't know a better method to use for vanilla JavaScript however, so we'll have to stick with it for now.
Put your operators in an array:

var operators = ['+', '-', 'x', '÷'];

Right now your code doesn't handle decimal points very well.  I would add a Boolean flag to track if you have them in the number.
You don't consider the possibility of a user entering x or ÷ into your text field.  You should replace those with their respecting proper operators:

equation = equation.replace(/x/g, '*').replace(/÷/g, '/');

What happens if the last character of the equation is an operator or a decimal?  We should remove it and then process the equation:

if(operators.indexOf(lastChar) > -1 || lastChar == '.')
        equation = equation.replace(/.$/, '');

Let's consider failure cases where we don't want to process the equation.  I've implemented some checks handling these in the final code with comments explaining them.

If we start with an operator other than -
If there is more than one decimal in the number
Two operators appear together

Final Results

// Get all the keys from document
var keys = document.querySelectorAll('#calculator span');
var operators = ['+', '-', 'x', '÷'];
var decimalAdded = false;

// Add onclick event to all the keys and perform operations
for (var i = 0; i < keys.length; i++) {
  keys[i].onclick = function(e) {
    // Get the input and button values
    var input = document.querySelector('.screen');
    var inputVal = input.innerHTML;
    var btnVal = this.innerHTML;

    // Now, just append the key values (btnValue) to the input string and finally use javascript's eval function to get the result
    // If clear key is pressed, erase everything
    if (btnVal === 'C') {
      input.innerHTML = '';
      decimalAdded = false;
    }

    // If eval key is pressed, calculate and display the result
    else if (btnVal === '=') {
      var equation = inputVal;
      var lastChar = equation[equation.length - 1];

      // Replace all instances of x and ÷ with * and / respectively. This can be done easily using regex and the 'g' tag which will replace all instances of the matched character/substring
      equation = equation.replace(/x/g, '*').replace(/÷/g, '/');

      // Final thing left to do is checking the last character of the equation. If it's an operator or a decimal, remove it
      if (operators.indexOf(lastChar) > -1 || lastChar == '.')
        equation = equation.replace(/.$/, '');

      if (equation)
        input.innerHTML = eval(equation);

      decimalAdded = false;
    }

    // Basic functionality of the calculator is complete. But there are some problems like
    // 1. No two operators should be added consecutively.
    // 2. The equation shouldn't start from an operator except minus
    // 3. not more than 1 decimal should be there in a number

    // We'll fix these issues using some simple checks

    // indexOf works only in IE9+
    else if (operators.indexOf(btnVal) > -1) {
      // Operator is clicked
      // Get the last character from the equation
      var lastChar = inputVal[inputVal.length - 1];

      // Only add operator if input is not empty and there is no operator at the last
      if (inputVal != '' && operators.indexOf(lastChar) === -1)
        input.innerHTML += btnVal;

      // Allow minus if the string is empty
      else if (inputVal === '' && btnVal === '-')
        input.innerHTML += btnVal;

      // Replace the last operator (if exists) with the newly pressed operator
      if (operators.indexOf(lastChar) > -1 && inputVal.length > 1) {
        // Here, '.' matches any character while $ denotes the end of string, so anything (will be an operator in this case) at the end of string will get replaced by new operator
        input.innerHTML = inputVal.replace(/.$/, btnVal);
      }

      decimalAdded = false;
    }

    // Now only the decimal problem is left. We can solve it easily using a flag 'decimalAdded' which we'll set once the decimal is added and prevent more decimals to be added once it's set. It will be reset when an operator, eval or clear key is pressed.
    else if (btnVal === '.') {
      if (!decimalAdded) {
        input.innerHTML += btnVal;
        decimalAdded = true;
      }
    }

    // if any other key is pressed, just append it
    else {
      input.innerHTML += btnVal;
    }

    // prevent page jumps
    e.preventDefault();
  }
}
/* Basic reset */

* {
  margin: 0;
  padding: 0;
  box-sizing: border-box;
  /* Better text styling */
  font: bold 14px Arial, sans-serif;
}
/* Finally adding some IE9 fallbacks for gradients to finish things up */

/* A nice BG gradient */

html {
  height: 100%;
  background: white;
  background: radial-gradient(circle, #fff 20%, #ccc);
  background-size: cover;
}
/* Using box shadows to create 3D effects */

#calculator {
  width: 325px;
  height: auto;
  margin: 100px auto;
  padding: 20px 20px 9px;
  background: #9dd2ea;
  background: linear-gradient(#9dd2ea, #8bceec);
  border-radius: 3px;
  box-shadow: 0px 4px #009de4, 0px 10px 15px rgba(0, 0, 0, 0.2);
}
/* Top portion */

.top span.clear {
  float: left;
}
/* Inset shadow on the screen to create indent */

.top .screen {
  height: 40px;
  width: 212px;
  float: right;
  padding: 0 10px;
  background: rgba(0, 0, 0, 0.2);
  border-radius: 3px;
  box-shadow: inset 0px 4px rgba(0, 0, 0, 0.2);
  /* Typography */
  font-size: 17px;
  line-height: 40px;
  color: white;
  text-shadow: 1px 1px 2px rgba(0, 0, 0, 0.2);
  text-align: right;
  letter-spacing: 1px;
}
/* Clear floats */

.keys,
.top {
  overflow: hidden;
}
/* Applying same to the keys */

.keys span,
.top span.clear {
  float: left;
  position: relative;
  top: 0;
  cursor: pointer;
  width: 66px;
  height: 36px;
  background: white;
  border-radius: 3px;
  box-shadow: 0px 4px rgba(0, 0, 0, 0.2);
  margin: 0 7px 11px 0;
  color: #888;
  line-height: 36px;
  text-align: center;
  /* prevent selection of text inside keys */
  user-select: none;
  /* Smoothing out hover and active states using css3 transitions */
  transition: all 0.2s ease;
}
/* Remove right margins from operator keys */

/* style different type of keys (operators/evaluate/clear) differently */

.keys span.operator {
  background: #FFF0F5;
  margin-right: 0;
}
.keys span.eval {
  background: #f1ff92;
  box-shadow: 0px 4px #9da853;
  color: #888e5f;
}
.top span.clear {
  background: #ff9fa8;
  box-shadow: 0px 4px #ff7c87;
  color: white;
}
/* Some hover effects */

.keys span:hover {
  background: #9c89f6;
  box-shadow: 0px 4px #6b54d3;
  color: white;
}
.keys span.eval:hover {
  background: #abb850;
  box-shadow: 0px 4px #717a33;
  color: #ffffff;
}
.top span.clear:hover {
  background: #f68991;
  box-shadow: 0px 4px #d3545d;
  color: white;
}
/* Simulating "pressed" effect on active state of the keys by removing the box-shadow and moving the keys down a bit */

.keys span:active {
  box-shadow: 0px 0px #6b54d3;
  top: 4px;
}
.keys span.eval:active {
  box-shadow: 0px 0px #717a33;
  top: 4px;
}
.top span.clear:active {
  top: 4px;
  box-shadow: 0px 0px #d3545d;
}

    C

    7
    8
    9
    +
    4
    5
    6
    -
    1
    2
    3
    ÷
    0
    .
    =
    x


Finding min and max values of an iterable on the fly This is a follow up for my question about optimizing solution for DNA Health HackerRank problem.

Short re-cap:

  You are given 2 arrays (genes and health), one of
  which have a 'gene' name, and the other - 'gene' weight (aka
  health). You then given a bunch of strings, each containing values m and n, which denote the start and end of the slice to be applied
  to the genes and health arrays, and the 'gene'-string, for which
  we need to determine healthiness. Then we need to return health-values
  for the most and the least healthy strings.

At the advice from AJNeufeld, I optimized my code into the following. Unfotunately, it still fails to execute on large testcases, such as this one.

So, the question now is: what else can I do to make the solution less expensive?

if __name__ == '__main__':
    n = int(input())

    genes = input().rstrip().split()
    genes_regex = [re.compile(f"(?={gene})") for gene in genes]
    health = list(map(int, input().rstrip().split()))

    s = int(input())
    min_weight = math.inf
    max_weight = -math.inf

    for s_itr in range(s):
        m,n,gn = input().split()
        weight = 0
        for i in range(int(m),int(n)+1):
            if genes[i] in gn:
                matches = len(re.findall(genes_regex[i], gn))
                weight += health[i]*matches

        if weight < min_weight:
            min_weight = weight
        if weight > max_weight:
            max_weight = weight

    print(min_weight, max_weight)

There are three obvious suggestions here.

Measure!

First, and most important: develop a timing framework to measure your changes! You can't know if a change is beneficial if all you have is "the old version doesn't pass" and "the new version doesn't pass".

Build a standard set of test cases, and a timing framework, and subject any change to measurement in the timing framework. It's better if the timing is better, otherwise it's not.

Cache your results

The examples shown at the hackerrank site specifically include one where the same gene string is repeated twice. So it seems likely that caching your results might provide an obvious performance win.

Stop using the regex engine

This is a "maybe." You're using the regex engine to get the findall behavior, which is sensible, since it gives you access to C code that does what you want. But using that engine comes at a price -- regex operations are traditionally slower than string operations. So see if you can write your code to do the same job without using regex calls.

I'm honestly not sure if this will benefit you or not, since the regexes you are using are so simple. But if you pre-compute the minimum offset for each pattern, to allow for overlaps (like "a" -> +1, "aa" -> +1, "ab" -> +2) you should be able to scan using str.find or str.index and get what you want without any re calls.

Bonus: generator

Your original question asked about using generators. Because the underlying operation is so expensive, I'd suggest writing a single minmax function that yields both values at the same time (like divmod does). You can feed that function with a generator that yields up the scores:

queries = [input() for _ in range(s)]
low, high = minmax(gene_scores(int(l[0]), int(l[1]), l[2] for l in queries))

(This has nothing to do with performance. But you wanted to use them, so here's a way!)


Digit Summing Function I'm working through the common divisibility tests, and implementing them in Racket.  I'm working on divisible by three, for which I thought it'd be nice to keep summing until I got to a single digit (which I can then check for divisibility by 3).

My code is here, but it feels like there are rather a lot of steps, and I wonder if there's a way to make it feel a bit less imperative.

(define (digits n)
  (cond
    [(zero? n) '()]
    [else (cons (remainder n 10) (digits (quotient n 10)))]))

(define (digisum n)
  (apply + (digits n)))

(define (repeated-digit-sum x)
  (let ([ds (digisum x)])
  (if (< ds 10) ds
    (repeated-digit-sum ds))))

By the way, the fastest way to see if a number is divisible by three is to use (zero? (remainder n 3)). If you want to do the digit summing for fun, fine, but it will make your code slower.

So, one way to sum the digits is to extract digits directly during the summing process, rather than building a list of digits first.

(define (digisum n)
  (let loop ((sum 0) (n n))
    (define-values (q r) (quotient/remainder n 10))
    (if (zero? n)
        sum
        (loop (+ sum r) q))))

Alternatively, you can use a for comprehension for this:

(define (in-digits n (base 10))
  (make-do-sequence
   (thunk (values (curryr remainder base)
                  (curryr quotient base)
                  n
                  positive?
                  #f #f))))

(define (digisum n)
  (for/sum ((digit (in-digits n)))
    digit))


Implementing realloc in C I have an assignment to implement realloc in C, This's my code please review it.

void    *my_realloc(void *ptr, size_t len)
{
    void    *real;

    real = malloc(len);
    memset(real, 0, len);
    if (real)
        memcpy(real, ptr, len);
    free(ptr);
    return (real);
}

The code leaves a bad first impression because of how it is formatted. In particular, the extra whitespace between the return type and the function name, and that between the variable's type and its name, looks odd.
Unless you are writing code that needs to be compatible with pre-C99 compilers, you should break the habit of declaring variables at the top of the block and initializing them later. Instead, postpone the declaration of variables until the point when they can be initialized (to the extent possible). This helps to minimize bugs caused by uninitialized variables.
I recommend always using curly braces to create block scope after if statements, even if you currently only have one statement there. This minimizes the potential for serious bugs when you later go back and add some additional logic.
The return value of malloc must be checked immediately after the call to malloc! If malloc fails, it will return a null pointer, and any attempt to use a null pointer results in undefined behavior. Therefore, you need to move the call to memset to after you check the result of malloc, otherwise you risk undefined behavior.
In fact, you do not need to call memset at all. If you check the C language specification for information on the realloc function, you will see that it does not initialize the additional memory in the case where the new size is larger than the old size.
You are breaking the realloc specification in several other ways, too. If your goal is to simulate/reinvent realloc, then you absolutely must ensure that your implementation behaves in exactly the same way as realloc.

In fact, realloc is really a complete memory management subsystem wrapped in a single function—it does everything that malloc and free do—and more—making its semantics very complicated and confusing. Here's a summary of the relevant requirements for realloc from the standard:

If the requested block size is smaller than the original size of the block, realloc either frees the unwanted memory at the end of the block and returns the input pointer unchanged, or allocates a new block of the appropriate size, frees the original block, and returns a pointer to this newly-allocated block.
If the requested block size is larger than the original size of the block, realloc may allocate an expanded block at a new address and copy the contents of the original block into the new location. In this case, a pointer to the expanded block is returned, and the extended portion of the block is left uninitialized. Or, if possible, it may expand the original block in-place and return the input pointer unchanged.
If realloc cannot satisfy a request to expand a block, it returns a null pointer and does not free the original block. (realloc will always succeed when you request to shrink a block.)
If the input pointer is null, then realloc behaves exactly as if you had called malloc(size), returning a pointer to the newly-allocated block of the requested size, or a null pointer if the request could not be satisfied.
If the requested size is 0 and the input pointer is non-null, then realloc behaves exactly as if you had called free(ptr), and always returns a null pointer.
If the input pointer is null and the requested size is 0, then the result is undefined!

Whew! To be correct, your code needs to implement all of these conditions, precisely as documented.

Conversely, if you are not simulating realloc (and that's perfectly fine—maybe you're reinventing the wheel because you want different behavior in your program), then you need to use a different name so that other programmers are not fooled into thinking that it is identical to realloc and won't expect it to behave in the same way.
In fact, as you can see from the list of requirements above, it is actually impossible to reinvent realloc as required by the specification, since you do not have insider knowledge about how the standard library implements memory management. You need to be able to determine the original size of the memory block pointed to by ptr, and there is no portable way to do so.

Why do you need this information? Two reasons. First, you need to know whether the caller is requesting to shrink or expand the memory block so that you can follow the correct semantics. Second, you need to know how many bytes to copy when you call memcpy—right now, you are copying past the end of the original buffer, which is undefined behavior!

The first problem could be solved by altering the semantics of our function so that it only grows the size of a block, renaming it something like GrowBlock or ExpandMemory to make it clear that it is not identical to realloc. Unfortunately, there would be no reliable way to assert this requirement that len is greater than or equal to the current size of the memory block pointed to by ptr within the body of the function, so documentation alone would have to suffice, which is a very weak guarantee. Even more unfortunately, this won't solve the second problem—we still have no way to correctly call memcpy!

Therefore, our only real option is to modify the function's signature to accept the original size of ptr as a parameter (in addition to the desired new size that you already accept as a parameter).

All of that considered, here's how I'd write it:

// This function is similar to realloc() and implements the same semantics,
// except that the caller must explicitly pass the original size of the
// memory block pointed to by 'ptr', as well as the desired size.
void *my_realloc(void *ptr, size_t originalLength, size_t newLength)
{
   // Note that because we cannot rely on implementation details of the standard library,
   // we can never grow a block in place like realloc() can. However, it is semantically
   // equivalent to allocate a new block of the appropriate size, copy the original data
   // into it, and return a pointer to that new block. In fact, realloc() is allowed to
   // do this, as well. So we lose a possible performance optimization (that is rarely
   // possible in practice anyway), but correctness is still ensured, and the caller
   // never need be the wiser.
   //
   // Likewise, we cannot actually shrink a block of memory in-place, so we either
   // have to return the block unchanged (which is legal, because a block of memory
   // is always allowed to be *larger* than necessary), or allocate a new smaller
   // block, copy the portion of the original data that will fit, and return a
   // pointer to this new shrunken block. The latter would actually be slower,
   // so we'll avoid doing this extra work when possible in the current implementation.
   // (You might need to change this if memory pressure gets out of control.)

   if (newLength == 0)
   {
      free(ptr);
      return NULL;
   }
   else if (!ptr)
   {
      return malloc(newLength);
   }
   else if (newLength <= originalLength)
   {
      return ptr;
   }
   else
   {
      assert((ptr) && (newLength > originalLength));
      void *ptrNew = malloc(newLength);
      if (ptrNew)
      {
          memcpy(ptrNew, ptr, originalLength);
          free(ptr);
      }
      return ptrNew;
    }
}

Indeed, this looks quite complicated—and it it isn't even as complicated as a real implementation of realloc! It's a good lesson about why realloc is actually bad design. You should not write massive multi-purpose functions, and you certainly shouldn't implement an entire subsystem in a single function! Instead, break the important behaviors out into separate functions. In this case, you might have four separate functions that allocate a block (AllocateMemory), free a block (FreeMemory), expand a block (ExpandMemory), and shrink a block (ShrinkMemory). This division of labor makes the implementation(s) much easier to write, easier to reason about, easier to include error-checking, and therefore easier to maintain and less likely to contain bugs. I realize your assignment was to write realloc, but there is a broader (perhaps inadvertent) lesson here that you should not miss.


Deaf grandma doesn't allow me to leave until I say bye three times So, I'm working through Chris Pine's "learn to program" and I have been banging my head against the wall regarding the "deaf grandma" lesson.

I think I got it.

I'm looking for advice regarding indentations, and/or structuring of the code to make it easier to read, for myself and others.

Here's the premise:

  Write a Deaf Grandma program.
  Whatever you say to grandma (whatever you type in), she should respond with HUH?! SPEAK UP, SONNY!, unless you shout it (type in all capitals). If you shout, she can hear you (or at least she thinks so) and yells back, NO, NOT SINCE 1938! To make your program really believable, have grandma shout a different year each time; maybe any year at random between 1930 and 1950. (This part is optional, and would be much easier if you read the section on Ruby's random number generator at the end of the methods chapter.) You can't stop talking to grandma until you shout BYE.

  Hint: Don't forget about chomp! 'BYE'with an Enter is not the same as 'BYE' without one!

  Hint 2: Try to think about what parts of your program should happen over and over again. All of those should be in your while loop.

  Extend your Deaf Grandma program: What if grandma doesn't want you to leave? When you shout BYE, she could pretend not to hear you. Change your previous program so that you have to shout BYE three times in a row. Make sure to test your program: if you shout BYE three times, but not in a row, you should still be talking to grandma.

Here's my solution:

puts "say hi to grandma"

bye = 0

while bye < 3 do
  input = gets.chomp

  if input == "BYE"
  bye = bye + 1
  puts "HEH?!"
  else
    if input == input.upcase
    bye = 0
    puts "NO, NOT SINCE " + rand(1930..1950).to_s + "!"
    else
    puts "HUH?! SPEAK UP SONNY!"
    bye = 0
    end
  end
end

puts "EH?! OH, BYE DEAR!"

There are a few small changes that I would make:

Indent the bodies of if/else blocks.
Moving the bye = 0 line to the top of the else block reduces code duplication, and it'll make your code more clear, because it will read like: 'if the input is "BYE", increase the 'bye' counter, else, reset it to 0'.
Ruby has a += operator: bye += 1 is shorthand for bye = bye + 1.
The name bye isn't very accurate, something like bye_shouts_in_a_row is probably better. Picking accurate, meaningful names is often difficult, but it can make code (much) easier to understand, and that makes working with it easier as well.

Applying these gives the following result:

puts "say hi to grandma"

bye_shouts_in_a_row = 0

while bye_shouts_in_a_row < 3 do
  input = gets.chomp

  if input == "BYE"
    bye_shouts_in_a_row += 1
    puts "HEH?!"
  else
    bye_shouts_in_a_row = 0
    if input == input.upcase
      puts "NO, NOT SINCE " + rand(1930..1950).to_s + "!"
    else
      puts "HUH?! SPEAK UP SONNY!"
    end
  end
end

puts "EH?! OH, BYE DEAR!"


JQuery Promise Interface for a (very) simple JSON Parser I have written a parser function that returns a JQuery Promise.  You can see from the code that this is the top level parser and it delegates out to two other more specific parsers.

At the minute it feels a little scattered about with the promise being rejected and resolved all over the place.  perhaps it would be better with a try and catch and an error handler that rejected the promsie?  how would others approach this?

/**
 * Inspects JSON and delegates to either the sourceParser or the dataParser.
 *
 * @constructor
 * @param {Object} json
 *
 * @returns JQuery Promsise
 *      done:       -
 *      fail:       error message
 *      progress:   Progress message
 */
function Parser(json)
{
    var dfd = $.Deferred();

    dfd.notify("Parsing");

    if (json && typeof json === "object")
    {
        if (json.hasOwnProperty("GetDataSources"))
        {
            var dataSource = json.GetDataSources;

            if (this.isSuccessfulResponse(dataSource.Response))
            {
                // Notify the caller of any progress
                dfd.notify("Parsing Source");

                // Create a new Source Parser
                var sourceParser =  new SourceParser(dataSource);
                sourceParser.done(dfd.resolve);
                sourceParser.fail(dfd.reject);
            }
            else
            {
                dfd.reject("Parsing Source Failed");
            }
        }
        else if (json.hasOwnProperty("GetData"))
        {
            var data = json.GetData;

            if (this.isSuccessfulResponse(data.Response))
            {
                // Notify the caller of any progress
                dfd.notify("Parsing Data");

                // Create a new Data Parser
                var dataParser = new DataParser(data);
                dataParser.done(dfd.resolve);
                dataParser.fail(dfd.reject);
            }
            else
            {
                // Pass back an error message?
                dfd.reject("Parsing Data Failed");
            }
        }
    }
    else
    {
        dfd.reject("There was a problem reading the JSON")
    }

    return dfd.promise();
}

Chop up functions longer than 12 lines of code into smaller functions.
Since you're using this, then you can attach the additional functions to the prototype on Parser.

Here's what I came up with.

function Parser(json) {
    var dfd = $.Deferred();
    dfd.notify("Parsing");
    if (json && typeof json === "object") {
        if (json.hasOwnProperty("GetDataSources")) {
            this._handleData( dfd, json.GetDataSources, SourceParser, "Source" );
        } else if (json.hasOwnProperty("GetData")) {
            this._handleData( dfd, json.GetData, DataParser, "Data" );
        }
    } else {
        dfd.reject("There was a problem reading the JSON");
    }
    return dfd.promise();
}
Parser.prototype._handleData = function(dfd, data, OtherParser, type){
    if (data && this.isSuccessfulResponse(data.Response)) {
        dfd.notify("Parsing " + type );
        var otherParser = new OtherParser(data);
        otherParser.done(dfd.resolve);
        otherParser.fail(dfd.reject);
    } else {
        dfd.reject("Parsing "+ type +" Failed");
    }
};


WPF filling a DataGrid I'm creating a WPF application with a DataGrid. I'd like some advice on the code I've written:

<DataGrid  Margin="0,10,10,10" LoadingRow="DataGrid_LoadingRow"    Name="user_view" EnableRowVirtualization="False" Grid.Column="2"    CanUserReorderColumns="True" CanUserResizeColumns="True"  CanUserResizeRows="True" CanUserSortColumns="True"
    AlternatingRowBackground="Gainsboro" CanUserAddRows="False" CanUserDeleteRows="False" IsReadOnly="True" AutoGenerateColumns="False" IsSynchronizedWithCurrentItem="True">
    <DataGrid.Columns>
        <DataGridTextColumn  Header="Name" Width="*" Binding="{Binding name}" ></DataGridTextColumn>
        <DataGridTextColumn Header="Surname" Width="*" Binding="{Binding surname}"></DataGridTextColumn>
        <DataGridTextColumn Header="Address" Width="*" Binding="{Binding address}"></DataGridTextColumn>

 // and so on
 // and so on
 // and so on

...are all the cells that I bound my SQL View to.

Now, to fill the DataGrid:

public void LoadView()
{
    try
    {
        SQLiteCommand command = new SQLiteCommand(connection);
        command.CommandText = MyApk.Properties.Resources.user_datagrid;

        DataSet DST = new DataSet();
        DataTable DT = new DataTable();
        SQLiteDataAdapter SDA = new SQLiteDataAdapter(command);
        SDA.Fill(DT);
        this.user_view.ItemsSource = DT.AsDataView();
    }
    catch (Exception ex)
    {
        MessageBox.Show(ex.Message);
    }
}

And the query to create the view is basically:

CREATE VIEW IF NOT EXISTS user_details AS SELECT name,surname,address //more stuff here

The specific points I'd like addressed are:

Is this the right way to fill the DataGrid?
Is it hard coded?
How can I make it even better?

public void LoadView()
{
    try
    {
        SQLiteCommand command = new SQLiteCommand(connection);
        command.CommandText = MyApk.Properties.Resources.user_datagrid;

        DataSet DST = new DataSet();
        DataTable DT = new DataTable();
        SQLiteDataAdapter SDA = new SQLiteDataAdapter(command);
        SDA.Fill(DT);
        this.user_view.ItemsSource = DT.AsDataView();
    }
    catch (Exception ex)
    {
        MessageBox.Show(ex.Message);
    }
}

DataSet DST = new DataSet(); is never used and should be deleted.
both SQLiteCommand and SQLiteDataAdapter are implementing IDisposable so you should enclose them in a using statement to automatically dispose them after their usage. In addition the constructor of the SQLiteCommand is overloaded to take a commandtext too.
variables should be named using camelCase casing and shouldn't be named using abbreviations so SDA could simply be dataAdapter and DT will be dataTable.

Implementing the mentioned points will lead to

public void LoadView()
{
    try
    {

        using (SQLiteCommand command = new SQLiteCommand(MyApk.Properties.Resources.user_datagrid, connection))
        using (SQLiteDataAdapter dataAdapter = new SQLiteDataAdapter(command))
        {
            DataTable dataTable = new DataTable();
            dataAdapter.Fill(dataTable);
            this.user_view.ItemsSource = dataTable.AsDataView();
        }
    }
    catch (Exception ex)
    {
        MessageBox.Show(ex.Message);
    }
}

But now this is tightly coupled to the user_view control and the query MyApk.Properties.Resources.user_datagrid which is suboptimal.

If we pass an ItemsControl object and a string to this method it would be more flexible like so

public void LoadView(ItemsControl control, string commandText)
{
    try
    {

        using (SQLiteCommand command = new SQLiteCommand(commandText, connection))
        using (SQLiteDataAdapter dataAdapter = new SQLiteDataAdapter(command))
        {
            DataTable dataTable = new DataTable();
            dataAdapter.Fill(dataTable);
            control.ItemsSource = dataTable.AsDataView();
        }
    }
    catch (Exception ex)
    {
        MessageBox.Show(ex.Message);
    }
}

and can be called like

LoadView(this.user_view, MyApk.Properties.Resources.user_datagrid);


Singleton implementation of Cache Dependency Manager I am trying to implement a singleton CacheDependencyManager, by combining  this CacheDependencyManager from Steve Greatrex with a singleton pattern from Jon Skeet (example 6).

I'd really appreciate any feedback on it's correctness and / or suggestions for improvement.

ExplicitCacheDependency

public class ExplicitCacheDependency : CacheDependency
{
    private string _uniqueId;

    public ExplicitCacheDependency(string uniqueId)
        : base(new string[0]) //no file system dependencies
    {
        _uniqueId = uniqueId;
    }

    public override string GetUniqueID()
    {
        return _uniqueId;
    }

    // CacheDependency has a protected NotifyDependencyChanged method that will notify the caching framework that the cached item is no longer valid.
    // In most implementations this would be invoked in some callback, but for our purposes we can just add a new Invalidate method and invoke it directly
    public void Invalidate()
    {
        base.NotifyDependencyChanged(this, EventArgs.Empty);
    }
}

CacheDependencyManager

public sealed class CacheDependencyManager
{
    private static readonly Lazy<CacheDependencyManager> lazy =
        new Lazy<CacheDependencyManager>(() => new CacheDependencyManager());

    public static CacheDependencyManager Instance { get { return lazy.Value; } }

    private CacheDependencyManager()
    {
    }

    private Dictionary<string, ExplicitCacheDependency> _dependencies
        = new Dictionary<string, ExplicitCacheDependency>();

    public CacheDependency GetCacheDependency(string key)
    {
        if (!_dependencies.ContainsKey(key))
            _dependencies.Add(key, new ExplicitCacheDependency(key));

        return _dependencies[key];
    }

    public void InvalidateDependency(string key)
    {
        if (_dependencies.ContainsKey(key))
        {
            var dependency = _dependencies[key];
            dependency.Invalidate();
            dependency.Dispose();
            _dependencies.Remove(key);
        }
    }
}

As CacheDependencyManager is a singleton it should be thread safe.

ConcurrentDictionary is a nice fit for this.

Using ConcurrentDictionarythe code would look like this:

private ConcurrentDictionary<string, ExplicitCacheDependency> _dependencies = new ConcurrentDictionary<string, ExplicitCacheDependency>();

public CacheDependency GetCacheDependency(string key)
{
    return _dependencies.GetOrAdd(key, k => new ExplicitCacheDependency(k));
}

public void InvalidateDependency(string key)
{
    ExplicitCacheDependency dependency;
    if (_dependencies.TryRemove(key, out dependency))
    {
        dependency.Invalidate();
        dependency.Dispose();
    }
}