Controller actions permisson specs I'm just starting with rails and testing, and not sure what's good way to dry code like this.

require 'rails_helper'

describe EventsController, type: :controller do
  let(:admin) { create(:user_admin) }
  let(:mod) { create(:user_moderator) }
  let(:user) { create(:user) }

  let(:link) { create(:link) }
  let(:event) { create(:event) }

  describe 'GET #new' do
    it 'allows admin user to perform action' do
      sign_in admin

      get :new, link: link.id

      expect(flash[:alert]).to_not match(/^You are not authorized to perform this action./)
    end

    it 'allows mod user to perform action' do
      sign_in mod

      get :new, link: link.id

      expect(flash[:alert]).to_not match(/^You are not authorized to perform this action./)
    end

    it 'does not allow regular user to perform action' do
      sign_in user

      get :new, link: link.id

      expect(flash[:alert]).to match(/^You are not authorized to perform this action./)
    end

    it 'does not allow guest to perform action' do
      get :new, link: link.id

      expect(flash[:alert]).to match(/^You are not authorized to perform this action./)
    end
  end

  describe 'POST #create' do
    it 'allows admin user to perform action' do
      sign_in admin

      post :create, event: attributes_for_event(event, link)

      expect(flash[:alert]).to_not match(/^You are not authorized to perform this action./)
    end

    it 'allows mod user to perform action' do
      sign_in mod

      post :create, event: attributes_for_event(event, link)

      expect(flash[:alert]).to_not match(/^You are not authorized to perform this action./)
    end

    it 'does not allow regular user to perform action' do
      sign_in user

      post :create, event: attributes_for_event(event, link)

      expect(flash[:alert]).to match(/^You are not authorized to perform this action./)
    end

    it 'does not allow guest to perform action' do
      post :create, event: attributes_for_event(event, link)

      expect(flash[:alert]).to match(/^You are not authorized to perform this action./)
    end
  end
end

You could DRY the specs in the following way:

perform the authentication of the user in a before block
when testing a controller the subject could be the request itself
use context blocks for better structure
use functions for making the assertions when it comes to flash messages

So, keeping these in mind, you could write your specs like this:

require 'rails_helper'

describe EventsController, type: :controller do
  let(:link) { create(:link) }
  let(:event) { create(:event) }
  let(:admin) { create(:user_admin) }
  let(:moderator) { create(:user_moderator) }
  let(:user) { create(:user) }

  AUTHORIZATION_ERROR = /^You are not authorized to perform this action/

  describe 'GET #new' do
    subject { get :new, link: link.id  }

    before(:each) do
      sign_in(authorized_resource) if authorized_resource.present?
    end

    context 'for user with admin role' do
      let(:authorized_resource) { admin }
      it 'does not display an alert' do
        subject
        expect(flash[:alert]).to_not match(AUTHORIZATION_ERROR)
      end
    end

    context 'for user with moderator role' do
      let(:authorized_resource) { moderator }
      it 'does not display an alert' do
        subject
        expect(flash[:alert]).to_not match(AUTHORIZATION_ERROR)
      end
    end

    context 'for user with no role' do
      let(:authorized_resource) { user }
      it 'displays an alert' do
        subject
        expect(flash[:alert]).to match(AUTHORIZATION_ERROR)
      end
    end

    context 'for guest user' do
      it 'displays an alert' do
        subject
        expect(flash[:alert]).to match(AUTHORIZATION_ERROR)
      end
    end
  end
end

It might make sense to check for response codes as well (e.g. 401 Unauthorized, 403 Forbidden, etc) since a flash message on its own does say much about what's going on with the controller.


Is this a sensible form iteration pattern for a web form with an unknown number of fields? I'm designing a small intranet-based time-tracking web app that accepts an unknown number of data "rows" which each consist of 7 form fields. Rows can by dynamically added by the browser.

Can I do better?

Given this (partial) example POST data:

$_POST['project'] =>
 Array
 (
     [0] => PROJECT_CODE_1
     [1] => PROJECT_CODE_1
 )

$_POST['task']
 Array
 (
     [0] => 21
     [1] => 4
 )

$_POST['date']
 Array
 (
     [0] => 2012-07-31
     [1] => 2012-07-31
 )

And this iterator:

<?php
$insert_values = array();

for ($i = 0; $i < count($_POST['project']); $i++)
{
    $insert_values[] = array(
        'entry_id' => null,
        'user_id' => $this->session->userdata('user_id'),
        'project_id' => $_POST['project'][$i],
        'task_id' => $_POST['task'][$i],
        'date' => $_POST['date'][$i]
    );
}

$this->db->insert_batch('entries', $insert_values);
?>

In general, is this iteration pattern safe and sensible? POST['project'] is a drop-down, is validated and will always be filled.

is this iteration pattern safe?

Safe for what? Safe for a database? We can't tell you that! There's no code for your database entry function. What you have here is perfectly safe, but that's excluding any type of insert into a database. To safely protect yourself from SQL injection and other types of security exploits, use prepared queries, validate (sanitize only if needed) incoming data (which includes the username and $_POST['project']!), and encode any output.

Without your database function, it's difficult to critique your code. There's not too much that be be done wrong in the snippet provided.


Calculates best angle on path and approximates cardinal direction This program uses the C language. What's used are two points, and the usage of the inverse tan function finds the angle of the line between the two points. Afterward, it finds the cardinal direction on the plane. -y is south, +y is north. It seems to work quite well. However, I want to find what is unconventional of my program. I want to learn to write good code and this is a good step towards it.

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

#define PI 3.14159265

int cardinal(double degrees)
{
    // 0 is North
    // 1 is North-east
    // 2 is East
    // 3 is South-east
    // 4 is South
    // 5 is South-west
    // 6 is West
    // 7 is North-west
    int cardinal;
    if (degrees >= -22.5 && degrees <= 22.5)
        cardinal = 0;
    if (degrees > 22.5 && degrees < 67.5)
        cardinal = 1;
    if (degrees >= 67.5 && degrees <= 112.5)
        cardinal = 2;
    if (degrees > 112.5 && degrees < 157.5)
        cardinal = 3;
    if (degrees >= -157.5 && degrees <= 157.5)
        cardinal = 4;
    if (degrees > -157.5 && degrees < -112.5)
        cardinal = 5;
    if (degrees >= -112.5 && degrees <= -67.5)
        cardinal = 6;
    if (degrees > -67.5 && degrees < -22.5)
        cardinal = 7;

    return cardinal;
}

char* direction(int cardinal)
{
    if (cardinal == 0)
        return "north";
    if (cardinal == 1)
        return "northeast";
    if (cardinal == 2)
        return "east";
    if (cardinal == 3)
        return "southeast";
    if (cardinal == 4)
        return "south";
    if (cardinal == 5)
        return "southwest";
    if (cardinal == 6)
        return "west";
    if (cardinal == 7)
        return "northwest";
}

int main()
{
    int xpl = 494, ypl = 105;
    int xcom = 152, ycom = -168;

    double degrees;
    double radians;

    int num;

    double CONS = 180.0 / PI;

    int xo = xcom - xpl;
    int yo = ycom - ypl;

    radians = atan2(yo, xo);
    degrees = radians * CONS;

    num = cardinal(degrees);

    printf("The angle to travel in is %f (degrees) %f (radians)\n", degrees, radians);
    printf("Numerical direction: %d", num);
    printf("\nDirection: %s", direction(num));

    return 0;
}

Two suggestions:

in the cardinal function, return the number rather than setting a variable, so you shortcut all the other if statements. You could further optimize that by doing a binary rather than a linear search.
your direction function could be replaced by an array: int directions[8] = {"north", "northeast", ... };. Then the lookup is simply directions[cardinal]


Permutation 1...N. Answer queries "How many numbers between [X,Y] position are in the interval [L,K]"? So I am solving this task:
We have an permutation of the numbers 1..n.
I need to answer m queries:
-How many numbers between X-th and Y-th position are in the interval [L,K] (<=k and >= L)?

So my solution is simple: I create a segment tree and in every node i keep an map -> which numbers are in this interval and the position (when they are sorted).

For example ( 98, 14 , 22, 45, 33) the map for this interval is (14,1)(22,2),(33,3),(45,5)..
Then when i recieve a query (X,Y,l,k) I find the interval in the tree,then I use lower bound (to the map in the node (so i get Log(n) time complexity)) to find the closest element to l and the closest element to k and substract their positions. The solution works, but i exceed the time for some test. Any tips how can i make it faster ? Maybe faster way to create the maps in the nodes ? Or faster search in the tree?

#include <iostream>
#include <map>
using namespace std;

#define IntervalsIntersect(left,right,X, Y)  (left <= Y && right >= X)

struct node
{
    map<int, int> m;
};

int getNextIndRight(int ind, int n) // By going right how does the interval changes
    {
    int diff = n - ind;
    if (diff % 2 == 0)
        return n - diff / 2;
    return n - diff / 2 - 1;

}
int getNextIndLeft(int ind, int n)
{
    int diff = n - ind;
    if (diff % 2 == 0)
        return ind + 1 + diff / 2;
    return (ind + diff / 2) + 1;
}
void build(int* a, node* t, int v, int tl, int tr, int left, int right)
{
    if (tl == tr)
    {
        t[v].m.insert(pair<int, int>(a[tl], 1));
    }
    else
    {
        int tm = (tl + tr) / 2;
        build(a, t, v * 2, tl, tm, left, getNextIndRight(left, right));
        build(a, t, v * 2 + 1, tm + 1, tr, getNextIndLeft(left, right), right);
        t[v].m = t[v * 2].m;
        t[v].m.insert(t[v * 2 + 1].m.begin(), t[v * 2 + 1].m.end());
        int i = 1;
        for (auto &p : t[v].m)
        {
            p.second = i++;
        }

    }
}

int countInInterval(map<int, int>& m, int k, int l)
{

    //for (auto &p : m)
    //    cout<<p.first<<" "<<p.second<<endl;
    auto it = m.lower_bound(k);
    auto it2 = m.lower_bound(l);

    bool firstOK = it != m.end();
    bool secondOK = it2 != m.end();
    if (!firstOK && !secondOK)
        return 0;
    else if (firstOK && !secondOK)
        return m.size() - it->second + 1;

    if (it == it2)
    {
        if (k <= it->first&&it->first <= l)
            return 1;
        return 0;
    }

    int res= (it2->second - it->second);
    if (k <= it2->first && it2->first <= l)
        res++;
    return res;

}
bool hasIntersection(int left, int right, int X, int Y)
{
    return left <= Y && right >= X;
}
int f(node* t, int* a, int left, int right, int n, int v, int X, int Y, int k, int l)
{
    if (left >= X && right <= Y)
        return countInInterval(t[v].m, k, l);
    int leftFirst = left;
    int rightFirst = getNextIndRight(left, right);
    int res = 0;
    if (IntervalsIntersect(leftFirst, rightFirst, X, Y))
        res += f(t, a, left, getNextIndRight(left, right), n, v * 2, X, Y, k, l);

    int leftsecond = getNextIndLeft(left, right);
    int rightsecond = right;
    if (IntervalsIntersect(leftsecond, rightsecond, X, Y))
        res += f(t, a, getNextIndLeft(left, right), right, n, v * 2 + 1, X, Y, k, l);;
    return res;
}

int main()
{

    int n, m;
    cin >> n >> m;
    int* a = new int[n];
    for (int i = 0; i < n; i++)
        cin >> a[i];

    node* t = new node[4 * n + 1];

    build(a, t, 1, 0, n - 1, 1, n);

    for (int i = 0; i < m; i++)
    {
        int x, y, k, l;
        cin >> x >> y >> k >> l;
        cout << f(t, a, 1, n, n, 1, x, y, k, l) << endl;

    }

}

I believe the problem lies in the initialization time. If I am not wrong it is like O(n^2 log n) which might be a problem for tests with large n and small sample size.

To fix it you'll have to redo the whole algo if I am not mistaken.

There is a method that doesn't have the same long initialization problem while retaining the same O(log n) (at least on average, O(log^2 n) at worst) time per iteration - simply build a two dimensional segment tree. Since we work with a permutation it will be rather sparse.


Retrieve next card(s) base on the provided parameter How can I remove the duplicated params[:next_cards]?

def index
  if params[:next_cards] # check nil
    render json: current_user.next_cards(params[:next_cards].to_i),
           status: :ok
  else
    render json: current_user.cards, status: :ok
  end
end

Inside the user.rb module,

  def next_cards(num_of_cards=nil)
    if num_of_cards
      cards.next_cards.limit(num_of_cards)
    else
      cards.next_cards
    end
  end

Inside the card.rb module,

  def self.next_cards
    joins(:meta_sm2).merge(MetaSm2.next_cards).order('RANDOM()')
  end

Inside the meta_sm2.rb model,

  scope :next_cards, -> { where('next_repetition <= :current_time', current_time: Time.now) }

Ok, in response to the conversation in comments, Here's my suggested first step:

  def index
    render json: cards, status: :ok
  end

  private

  def cards
    if next_cards
      current_user.next_cards(next_cards.to_i)
    else
      current_user.cards
    end
  end

  def next_cards
    params[:next_cards]
  end

the larger issue here though is that you are essentially using params[:next_cards] as a switch for what should essentially be two controller actions.  the latter being the index action of the cards controller, and the former being something along the lines of user/next_cards/:id . Possibly a next cards controller.  The show action of which could look something like

routes.rb:

resource :next_cards, only: :show

app/controllers/next_cards_controller.rb:

def show
  render json: current_user.next_cards(params[:id])
end

controllers don't have to have a 1 to 1 mapping with models. This way takes advantage of rails routing to validate that params[:id] will always be present. thereby eliminating the need for a nil check.

Obviously this could take more time as your views will need to be updated to reflect the new route. If it's a greenfield app, go ahead and do it right now. If it's an old, bloated codebase, start small with the refactor and do it in chunks.


A recursive_count Function For Various Type Arbitrary Nested Iterable Implementation in C++ This is a follow-up question for A Summation Function For Arbitrary Nested Vector Implementation In C++ and A Summation Function For Various Type Arbitrary Nested Iterable Implementation in C++. Besides the summation result, I am trying to get the element count in arbitrary nested iterable things. For example, there are three elements in std::vector<int> test_vector = { 1, 2, 3 };, so the element count of test_vector is 3. The experimental implementation of recursive_count is as below.
size_t recursive_count()
{
    return 0;
}

template<class T> requires (!is_elements_iterable<T> && !is_iterable<T>)
size_t recursive_count(const T& input)
{
    return 1;
}

template<class T> requires (!is_elements_iterable<T> && is_iterable<T>)
size_t recursive_count(const T& input)
{
    return input.size();
}

template<class T> requires (is_elements_iterable<T> && is_iterable<T>)
size_t recursive_count(const T& input)
{
    size_t output{};
    for (auto &element : input)
    {
        output += recursive_count(element);
    }
    return output;
}

Some test cases of this recursive_count template function.
//  std::vector<int> case
std::vector<int> test_vector = {
    1, 2, 3
};
auto recursive_count_result1 = recursive_count(test_vector);
std::cout << recursive_count_result1 << std::endl;

//  std::vector<std::vector<int>> case
std::vector<decltype(test_vector)> test_vector2 = {
    test_vector, test_vector, test_vector
};
auto recursive_count_result2 = recursive_count(test_vector2);
std::cout << recursive_count_result2 << std::endl;

// std::deque<int> case
std::deque<int> test_deque;
test_deque.push_back(1);
test_deque.push_back(1);
test_deque.push_back(1);
auto recursive_count_result3 = recursive_count(test_deque);
std::cout << recursive_count_result3 << std::endl;

// std::deque<std::deque<int>> case
std::deque<decltype(test_deque)> test_deque2;
test_deque2.push_back(test_deque);
test_deque2.push_back(test_deque);
test_deque2.push_back(test_deque);
auto recursive_count_result4 = recursive_count(test_deque2);
std::cout << recursive_count_result4 << std::endl;

A Godbolt link is here.
All suggestions are welcome.
The summary information:

Which question it is a follow-up to?
A Summation Function For Arbitrary Nested Vector Implementation In C++ and
A Summation Function For Various Type Arbitrary Nested Iterable Implementation in C++

What changes has been made in the code since last question?
The previous questions are focused on summation operation, and I am trying to implement a recursive_count template function here.

Why a new review is being asked for?
In my opinion, I am not sure the name of this function is good or not. I know there is a function std::count in STL which purpose is to count the elements that are equal to given specific value. If there is any better naming suggestion, please let me know.

No need for an overload that takes no parameters
Nothing calls recursive_count() without parameters, so that overload is unnecessary.
Unnecessary requirements
You don't need to require both is_elements_iterable<T> and is_iterable<T>, the former will already cover the requirements of the latter, and if you negate them then the latter covers the former. So the overloads should be:
template<class T> requires !is_iterable<T>
size_t recursive_count(const T& input)
{
    return 1;
}

template<class T> requires (!is_elements_iterable<T> && is_iterable<T>)
size_t recursive_count(const T& input)
{
    return input.size();
}

template<class T> requires is_elements_iterable<T>
size_t recursive_count(const T& input)
{
    size_t output{};
    for (auto &element : input)
    {
        output += recursive_count(element);
    }
    return output;
}

Naming
Indeed you already noticed that STL has a std::count that does something different. However there is a function that does what you want for non-nested containers: std::size. So consider naming your function std::recursive_size.
Consider making more use of STL algorithms
You are implementing your own algorithms but you are not making use of STL's algorithms. This can reduce the amount of code you write. For example, the last overload can be rewritten as:
template<class T> requires is_elements_iterable<T>
size_t recursive_count(const T& input)
{
    return std::transform_reduce(std::begin(input), std::end(input), std::size_t{}, std::plus<std::size_t>(), [](auto &element){
        return recursive_count(element);
    });
}

Although I'm not sure this is more readable than your version.


Is it worth to use if-else statement as Java optional pattern? I came to a point while refactoring using if {} else {} code with Java optionals. While optimizing code it turned to a result to this:

Optional.of(myBoolean).filter(b -> b).ifPresent(b -> {/*my code if true*/});

I recognized filter(b -> b) is not the way i wanted to use optionals thus i turned it to

If.is(myBoolean).then(b -> {/*my code if true*/}).orElse(b -> {/*my code if false*/});

The code for the If class I wrote to handle this is:

public class If {

    private boolean isTrue;

    public If(final boolean value) {
        isTrue = value;
    }

    public If then(final Consumer s) {
        if (isTrue) {
            s.accept(null);
        }
        return this;
    }

    public void orElse(final Consumer s) {
        if (!isTrue) {
            s.accept(null);
        }
    }

    public static If is(final boolean value) {
        return new If(value);
    }
}

Is this a good (whatever good is or should be) approach using the if {} else {} statement in Java?

My first instinct is that your code is a pretty “cool” way to rewrite if else statements, but I don’t think it gains you too much. If you have massive amounts of if trees and want to make them more readable, I would consider using polymorphism to your advantage or, when appropriate, strategy pattern.

If you want some help refactoring a certain if tree please feel free to post the code and we can help.

There are often, IMO, two big reasons to make a new abstraction.

1) to decrease the mental load on the maintainers of this code or

2) to increase the ability to add new features in the future.

Both of these are, of course, related. My question is, and feel free to comment below, does your if abstraction decrease maintainer mental load through simplicity or brevity and/or does your if abstraction improve the ability to add new features in the future? If not then you need to consider why you are doing it in the first place.


Optimising String split into chunks of fixed size I am looking to optimise my solution of the 'SMS problem', i.e splitting a String into chunks of the same size without splitting words.

I did a recursion and am looking for ways to optimise that code, or a more efficient algorithm.
In my solution spaces are left at the beginning of each chunk, this is voluntary.

Here is the code:

var exampleTxt = `Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum.`;

function getIndexOfLastSpace(msg, nthChar) {
  var curMsg = msg.substring(0, nthChar);
  console.log(curMsg.lastIndexOf(" "));
  return curMsg.lastIndexOf(" ");
}

function getMessagesNumber (msg) {

  if (msg.length < 1) {
    return 0;
  } else if (msg.length > 1 & msg.length < 153) {
    return 1;
  } else {

    var messages = [];

    while (msg.length > 0) {
      var chunk;
      // Last message
      if (msg.length <= 153) {
        chunk = msg;
      } else {
        var lastSpaceIndex = getIndexOfLastSpace(msg, 153);
        chunk = msg.substring(0, lastSpaceIndex);
      }
      messages.push(chunk);
      msg = msg.replace(chunk, '');
    }
    return messages.length;
  }
}

console.log(getMessagesNumber(exampleTxt));

Also am I write in thinking that the complexity of this algorithm is O(n) ?

Firstly, great implementation. Getting the maximum amount of characters for the next chunk, finding the last space in that and then using it to get the correct chunk is fast and works well.
Other ideas that spring to mind would be to split the string into words first and rebuild with a loop, but I think this would be slower.

Yes this is O(n) - the solution's speed is proportional to the amount of input passed.

Your solution is a loop, not a recursion. A recursive solution would be a function which calls itself. If you want to see this style, let me know and I can post it.

Some improvements:

You don't need to wrap the exampleTxt variable in backticks. This creates a template literal, which is unnecessary here - you can just use normal quotes.
Running the function with a 1 character string causes it to run the while loop, instead of returning early.
You can simplify the levels of nesting in the if ... else by returning early. If one of the returns is triggered then nothing after it will be run.

if (msg.length < 1) {
    return 0;
}

if (msg.length >= 1 & msg.length < 153) {
    return 1;
}

// while loop

return messages.length;

In the else if you have & (Bitwise AND) instead of && (Logical AND).
Your while loop mutates the state of the msg variable. Mutation can lead to hard to find bugs - clone the string first (remainingMsg = msg) and use that in the loop instead.
It's not necessary to have the getIndexOfLastSpace function separate. You can use something like this:

var nextChunk = msg.substring(0, 153);
nextChunk = nextChunk.substring(0, nextChunk.lastIndexOf(' '));

To make the function more reusable you could return the message chunks directly instead of the length. The user can then call .length on the returned value if that's all they want.
You could also pass the chunk length into the function as a parameter, so that it can handle chunking to any length.
If using the point above, setting some standard values as consts is a good practice.

const SMS_MESSAGE_MAX_LENGTH = 153;
// ...
getMessagesNumber(msg, SMS_MESSAGE_MAX_LENGTH);

For a higher level abstraction you could use a curryable function to create new functions with specific chunk lengths. I've used ES5 format as that's how the question was asked.

var chunk = function(chunkLength) {
    return (function splitChunks (message) {
        // ...
    });
};

var chunkSmsMessage = chunk(153);
chunkSmsMessage('Lorem ipsum...');


Is my MySQL library wrapper neat? How can I do better? I'm moderately new to programming in C and I'm not sure about programming practices with code; suddenly I have had fears that my code is messy and disorganized. I can read it when I come back a month later yet I'm still nervous if I'm doing anything that's considered taboo.

This isn't all the code that I've done with this project, I think my other code is fine.

#include <stdio.h>
#include <stdlib.h>
#include <signal.h>
#include <unistd.h>
#include "functions.h"
#include "errh.h"
#include "mysqlerr.h"

void FIRST_TIME_HANDLER(MYSQL *conn)
{
    printw("[+] Doing initialization checks...\n");
    SEND_MYSQL_QUERY(conn, "create table IF NOT EXISTS user_2(username varchar(25) not null, passwd varchar(25) not null)");
    printw("[+] Handler has checked tables.\n");
    refresh();
}

void INThandler(int sig)
{
    signal(sig, SIG_IGN);
    printw("Closing server...\n");
    refresh();
    sleep(5);
    endwin();
    exit(0);
}

MYSQL_INFO *CONNECTION_INIT()
{
    MYSQL_INFO *info = malloc(sizeof(*info));
    info->IP = "IPADDRGOHERE"; info->user = "Hello";
    info->pass = "World"; info->DB = "DBGOHERE";
    return info;
}

int EXIT_SERVER(MYSQL *connection)
{
    mysql_close(connection);
    printw("MySQL session has now closed.\n");
    refresh();
    return KILL_SIGNAL;
}

MYSQL *MYSQL_CONNECT_DB(MYSQL_INFO *info)
{
    MYSQL *connection;
    const char *server = info->IP;
    const char *user = info->user;
    const char *password = info->pass;
    const char *database = info->DB;

    connection = mysql_init(NULL);

    if(!mysql_real_connect(connection, server, user, password, database, 0,
        NULL, 0))
    {
        //printw("[-] Error: %s\n", mysql_error(connection));
        MYSQL *f = KILL_SIGNAL;
        refresh();
        return f;
    }

    printw("[+] Connected to MySQL server.\n");
    FIRST_TIME_HANDLER(connection);
    return connection;
}

void SEND_MYSQL_QUERY(MYSQL *conn, const char *query)
{
    refresh();
    if(mysql_query(conn, query))
    {
        ERROR err;
        throw_error_init(err, 0, 1, NONFATAL, "MySQL QUERY ERROR",
            "ERROR IN THE PROCESS OF QUERY.", MySQL_query_error);
    }
    else
    {
        printw("Query sent.\n");
    }
}

int init_connection()
{
    MYSQL *conn;
    MYSQL_INFO *info = CONNECTION_INIT();

    if((conn = MYSQL_CONNECT_DB(info)) == (MYSQL *) 0)
    {
        ERROR err;
        throw_error_init(err, 1, 1, FATAL, "INIT CONNECTION ERROR",
            "Failed to connect to server.", MySQL_init_failure);
    }
    else {
        int key;
        nodelay(stdscr,TRUE);

        signal(SIGINT, INThandler);

        while(1)
        {
            if((key = getch()) == ERR)
            {
                if(mysql_ping(conn) == 1)
                {
                    printw("Error!\n");
                    refresh();
                    return 1;
                }
            }
            else
            {
                if(key == EOF) break;
            }
        }

        free(info);
        EXIT_SERVER(conn);
        return 0;
    }
    return 0;
}

In C the naming conventions typically are lower_case_with_underscore for function names and types (although for types some people also use PascalCase). UPPERCASE is only really used for macro definitions.
In CONNECTION_INIT you have a bunch of hard-coded settings. These should come from a config file or via command line arguments.
In init_connection you write:

if((conn = MYSQL_CONNECT_DB(info)) == (MYSQL *) 0)

There is no reason to cast in order to compare it to NULL, simply == NULL works and is more idiomatic.
There is no reason to do the assignment and compare in one statement. Splitting it up makes the if statement less crammed:

conn = MYSQL_CONNECT_DB(info);
if (conn == NULL)
{
    ...

I assume printw is some kind of way to print debug/info/tracing messages. If so then this is not a good name for the function. Better would log_info or similar (depending on the exact purpose).
There is this magic refresh method called everywhere but it doesn't seem to follow any particular pattern. Seems suspicious to me.


Is there a way to make this code simplier? Program description:

Given an array of integers nums and an integer target, return indices of the two numbers such that they add up to target.
You may assume that each input would have exactly one solution, and you may not use the same element twice.
You can return the answer in any order.

My solution:
/**
 * @param {number[]} nums
 * @param {number} target
 * @return {number[]}
 */
var twoSum = function(nums, target) {
  let N = nums.length;
  for(i=0;i<=N-1;i++) {
      for(j=i+1;j<=N;j++) {
          if(nums[i] + nums[j] == target) {
              return [i,j];
          };
      };
  };
};

Test input:
[2,7,11,15]
9

Test output:
[0,1]

Test summary:
Solution accepted.
Runtime: 84ms

Question: is there a way to make this code look more neat using some comprehensions, maybe also improving its runtime? Thank you in advance.

Performance The biggest issue with your current implementation is that it's O(n ^ 2). You have to iterate through an array of length N (N ^ 2) / 2 times in order to find matches. The algorithm can be improved by iterating only once, and by using a Set or an object when iterating to store which number the current number would have to be paired with in order to sum to the target.
For example, when iterating over 1, with a target of 5, you could put 4 into the Set, because 1 + 4 === 5. On further iterations, check to see if the element being iterated over exists in the Set. If it does, it's a match: return the pair. See bottom of answer for an implementation.
Prefer const over let Since N isn't being reassigned, declare it with const: https://softwareengineering.stackexchange.com/questions/278652/how-much-should-i-be-using-let-vs-const-in-es6
Don't create global variables Always declare variables before using them. Your current code with for(i=0;i<=N-1;i++) and for(j=i+1;j<=N;j++) { is implicitly creating global i and j variables. This is both inelegant and can lead to confusing bugs if the function gets called again before its first call completes. (Also consider adding spaces between operators for readability). You'd want to do:
for (let i = 0; i < N; i++) {

Note the use of i < N, not i <= N; since N is the length, nums[N] will be undefined, so you shouldn't iterate over it.
Use strict equality Don't use == - it has many strange rules with type coercion. Even if you're sure the types of the operands are the same, to make it clearer for readers at a glance, better to avoid it and use === or !== instead.

/**
 * @param {number[]} nums
 * @param {number} target
 * @return {number[]}
 */
const twoSum = function(nums, target) {
  // Key: Number which, if found, matches the number at the index value
  // eg: { 3 => 6 }: if 3 is found later, it'll be a match with the number at index 6
  const indexByPair = new Map();
  const { length } = nums;
  for (let i = 0; i < length; i++) {
    if (indexByPair.has(nums[i])) {
      return [indexByPair.get(nums[i]), i];
    }
    indexByPair.set(target - nums[i], i);
  }
};
console.log(twoSum([2,7,11,15], 9));


Finding image by date I have 12 games that I need to sort through that each have a date and img string.  I'm fairly new to jQuery and was wondering what is the best way to accomplish this. Is there a way that I can run this through a loop of some sort rather than having 12 else-if statements like I currently have (code condensed to 3 for readability)? This code works, I was just looking for the cleaner more concise way of writing it.

var now = new Date();
boise = { date: new Date ('May 15, 2012'), img: 'Boise' };
central = { date: new Date ('May 21, 2012'), img: 'Central' };
notreDame = { date: new Date ('May 27, 2012'), img: 'NotreDame' };
    ...
banner = $('#nextGameBanner');
imgDir = 'images/nextGame';

if (now < boise.date ) {
    banner.attr('src', imgDir+boise.img+'.jpg');
}
else if (now < central.date) {
    banner.attr('src', imgDir+central.img+'.jpg');
}
else if (now < notreDame.date) {
    banner.attr('src', imgDir+notreDame.img+'.jpg');
}
    ...

HTML

<img src="/website/images/nextGame.jpg" id="nextGameBanner" />

Try something like this:

if (now.getTime() < boise.date.getTime() ) {
    banner.attr('src', imgDir+boise.img+'.jpg');
}
.....

In total,

var now = new Date();
    objs = {
        // here I change your 12 variables to a single object, so I can loop over them
        boise : { date: new Date ('May 15, 2012'), img: 'Boise' },
        central : { date: new Date ('May 21, 2012'), img: 'Central' },
        notreDame : { date: new Date ('May 27, 2012'), img: 'NotreDame' }
    },
    banner = $('#nextGameBanner'),
    imgDir = 'images/nextGame';

// making loop with objs

$.each(objs, function(key, singleObj) {
   // compare time with now and stored time in object
   if (now.getTime() < singleObj.date.getTime() ) {
     // assigning src to image when the condition satisfied
     banner.attr('src', imgDir+ singleObj.img +'.jpg');
     break; // stop the loop when match found
   }
});

read about getTime()


jQuery plugin to create sticky headers I've written a plugin that takes a generic approach to creating sticky headers and would like to have the code reviewed.  Is this acceptable JavaScript?

Demo Page

It's my first plugin however and I'd like feedback about how to make it more user friendly and more readable before releasing it and putting it up on GitHub.

The header becomes stuck to the top of the page until it encounters the bottom of it's container, upon which it gets stuck to the bottom of the container and floats off the screen.

I had to create a placeholder to take the header's original place once it leaves the document flow and also apply fixed CSS settings to the header to keep it's appearance the same when it leaves the flow.

(function ($) {
$.fn.stickySectionHeaders = function (options) {

    var settings = $.extend({
        stickyClass: 'header',
        padding: 0
    }, options);

    return $(this).each(function () {

        var container = $(this);
        var header = $('.' + settings.stickyClass, container);

        var originalCss = {
            position: header.css('position'),
            top: header.css('top'),
            width: header.css('width')
        };

        var placeholder = undefined;
        var originalWidth = header.outerWidth();

        $(window).scroll(function () {
            var containerTop = container.offset().top;
            var headerOrigin = header.offset().top;
            var headerHeight = header.outerHeight();
            var containerHeight = container.outerHeight();
            var containerTop = container.offset().top;
            var containerSize = container.outerHeight();
            var pageOffset = $(window).scrollTop() + settings.padding;
            var containerBottom = containerHeight + containerTop;

            if (pageOffset < containerTop && placeholder != undefined) {
                if (placeholder != undefined) {
                    placeholder.remove();
                    placeholder = undefined;
                    header.css(originalCss);
                }
            }
            else if (pageOffset > containerTop && pageOffset < (containerBottom - headerHeight)) {
                if (placeholder == undefined) {
                    placeholder = $('<div/>')
                    .css('height', header.outerHeight() + 'px')
                    .css('width', header.width() + 'px');
                    header.before(placeholder);
                    header.css('position', 'fixed');
                    header.css('width', originalWidth + 'px');
                }
                header.css('top', settings.padding + 'px');
            }
            else if (pageOffset > (containerBottom - headerHeight)) {
                header.css('top', (containerBottom - headerHeight) - pageOffset + settings.padding + 'px');
            }
        });
    });
}
})(jQuery);

At the risk of nitpicking there's quite a fiew small/micro-optimizations that could be done:

group multiple var declarations using commas

var containerTop = container.offset().top, headerOrigin = header.offset().top;
chain method calls to the same object

header.before(placeholder).css('...')
group multiple calls to css() passing an object as the parameter:

header.css({ position: 'fixed', width: originalWidth + 'px'});


A graph representation of Adjacent Matrix in Python I'm trying to create a graph representation in Adj Matrix in Python. I'm not sure if this is the best pythonic way.

class Graph(object):
    def __init__(self, edge_list):
        self.edge_list = edge_list

    def add_edge(self, edge_list):
        self.edge_list.append(edge_list)

    def adj_mtx(self):
        v = 0
        counter = set()
        for src, dest in self.edge_list:
            counter.add(src)
            counter.add(dest)

        v = len(counter)

        mtx = [[0 for y in range(v)]for x in range(v)]
        for e in self.edge_list:
            src, dest = e
            src = src - 1
            dest = dest - 1
            mtx[src][dest] = 1

        return mtx

edge_list = [(1,2), (2,3), (1,3)]
graph = Graph(edge_list)
mtx = graph.adj_mtx()

assert mtx == [[0, 1, 1], [0, 0, 1], [0, 0, 0]]

graph.add_edge((3,3))
mtx = graph.adj_mtx()
assert graph.adj_mtx() == [[0, 1, 1], [0, 0, 1], [0, 0, 1]]

Use third party libraries if possible

Almost anytime you want to do something, you probably want to use someone else's code to do it.  In this case, whenever you're working with graphs in Python, you probably want to use NetworkX.

Then your code is as simple as this (requires scipy):

import networkx as nx

g = nx.Graph([(1, 2), (2, 3), (1, 3)])
print nx.adjacency_matrix(g)
g.add_edge(3, 3)
print nx.adjacency_matrix(g)

Friendlier interface

It seems unnecessarily cumbersome to have to explicitly initialize an empty Graph this way: g = Graph([]).  I think a better implementation would be something like

class Graph(object):

    def __init__(self, edge_list=None):
        self.edge_list = edge_list if edge_list is not None else []

Similarly, I think the parameters for add_edge could use work. For one, it isn't a list of edges that you're passing it - it is a single edge, encoded as a list/tuple/collection of some kind.  I also find it a little annoying that I have to create a tuple/list/whatever to actually add the edge to the graph - I'd rather just pass in the two end-points.  I'd prefer something like this:

def add_edge(self, first, second):
    self.edge_list.append((first, second))

Encoding your graph

At this point, however, I have to ask why you're representing your graph as a list of edges - to me, the most intuitive way to think of a graph (and how I've implemented one in the past) is to use a dictionary - to use your example, I'd probably encode it like this

{
    1: {2, 3},
    2: {1, 3},
    3: {1, 2}
}

Then, instead of searching through an entire list to find a given edge, you just have to perform a quick dictionary lookup for one of the endpoints, and then a theoretically smaller iteration over a (hopefully) shorter list.  I've also seen versions that use nested dictionaries very effectively.

Your adj_mtx function

Note - I didn't change anything about the encoding here, I just used your implementation.

You could clean this up a bit.  You don't need to initialize v where you do. It would also be easier if you kept the nodes in a set and added them whenever you added an edge.

In general, prefer xrange in Python 2, although that makes compatibility trickier - I generally use a library like six to handle things like that, although if you don't need everything you can write your own file (good name is usually compatibility.py) that has only the changes you need.

Instead of nested comprehensions, just do [0] * v.  Also, if you don't care about a variable (such as x or y) use _ to identify it.

You can split up iteration variables in a for loop - for src, dest in self.edge_list is cleaner than for e in self.edge_list: src, dest = e.

Overall you could use more descriptive names in this function.  I'd probably write it something like this:

def adj_mtx(self):
    count = len(self.nodes)

    matrix = [[0]*count for _ in range(count)]
    for src, dest in self.edge_list:
        src -= 1
        dest -= 1
        matrix[src][dest] = 1

    return matrix

Additionally, it seems like adj_mtx should just be called adjacency_matrix, and I would rather it be a property (potentially with caching) than a function.  Imagine something like this:

class Graph(object):
    _adjacency_matrix = None

    def __init__(self, edge_list=None):
        self.edge_list = edge_list if edge_list is not None else []
        self.nodes = set()
        self.cache_valid = False

    def add_edge(self, first, second):
        edge = first, second
        self.edge_list.append(edge)
        self.nodes.update(edge)
        self.cache_valid = False

    @property
    def adjacency_matrix(self):
        if not self.cache_valid:
            count = len(self.nodes)

            matrix = [[0]*count for _ in range(count)]
            for src, dest in self.edge_list:
                src -= 1
                dest -= 1
                matrix[src][dest] = 1

            self._adjacency_matrix = matrix
            self.cache_valid = True

        return self._adjacency_matrix

    def clear_cache(self):
        self.cache_valid = False
        self._adjacency_matrix = None

Note that if someone adds edges to the list manually, or does other weird things, then the caching won't work.


Lock-free FIFO queue implementation struct __fifo_node_t {
        void *next;
};
typedef struct __fifo_node_t fifo_node_t;

struct __fifo_list_t {
        void *next;
        void *tail;
};
typedef struct __fifo_list_t fifo_list_t;

static inline void __fifo_list_push(fifo_list_t *head, fifo_node_t *node)
{
        fifo_node_t *prev;

        node->next = head;
        prev = atomic_xchg64(&head->tail, node);
        prev->next = node;
}

static inline fifo_node_t *__fifo_list_pop(fifo_list_t *head)
{
        fifo_node_t *node;
        while ((node = ACCESS_ONCE(head->next)) != head) {
                fifo_node_t *next = ACCESS_ONCE(node->next);
                if (cmpxchg_eq(&head->next, node, next) == node) {
                        if (cmpxchg_eq(&head->tail, node, head) != node &&
                            next == head)
                                while (ACCESS_ONCE(head->next) == next)
                                        head->next = ACCESS_ONCE(node->next);
                        return node;
                }
        }
        return NULL;
}

static inline void __fifo_list_init(fifo_list_t *head)
{
        head->next = head->tail = head;
}

atomic_xchg64 - Atomic exchange 64bit with new value and return the old value

cmpxchg_eq - Atomic compare and exchange 64bit with new value and return the old value

ACCESS_ONCE - (*(volatile typeof(x) *)&(x))

Can you review it? Is this implementation thread-safe ?

Thank you.

The meaning of lock-free
Although your program is free of locks in the traditional sense, "lock free programming" typically describes a style of programming with certain guarantees.  One of these guarantees is that even if one or more threads are suspended indefinitely, other threads can continue to make progress.  This wikipedia article on Non-blocking algorithms describes "lock-free" and "wait-free" programming in more detail.
In your code, if one thread is permanently suspended between these two lines:

    prev = atomic_xchg64(&head->tail, node);
    prev->next = node;

then some other thread will spin forever in this loop:

                            while (ACCESS_ONCE(head->next) == next)
                                    head->next = ACCESS_ONCE(node->next);

and furthermore, no nodes past the node that the suspended thread "half-pushed" will be able to be popped.
ABA problem
Additionally, your code falls victim to the ABA problem.  Consider an initial situation with two nodes in your FIFO:
A -> B    (head->next = A, head->tail = B)

Suppose thread #1 calls __fifo_list_pop(), and gets to this line:

            if (cmpxchg_eq(&head->next, node, next) == node) {

Here, node will be A, and next will be  B, and the code is about to atomically change head->next from A to B.
Now suppose thread #1 is descheduled, and in the meantime, other threads pop both A and B off the FIFO, and then push A and C back on the FIFO.  Now, your FIFO looks like this:
A -> C    (head->next = A, head->tail = C)

If thread #1 now wakes up, it will perform the atomic exchange, causing head->next to point at B which isn't even alive any more:
A -> B    (but B was already popped and is now dead)


Mutually exclusive options for argparse I wrote a Python program that can take as argument either a string or a filename, with these options being mutually exclusive:

#!/usr/bin/env python

import argparse
import sys

def parse_command_line():
    parser = argparse.ArgumentParser()
    parser.add_argument('-t', '--text', dest = 'input_text', help = 'Text')
    parser.add_argument('-f', '--file', dest = 'input_file', help = 'File containing the text')
    return parser

if __name__ == '__main__':
    parser = parse_command_line()
    args = parser.parse_args()

    if args.input_text is not None and args.input_file is None:
        text = args.input_text
    elif args.input_file is not None and args.input_text is None:
        text = open(args.input_file, 'r').read()
    else:
        parser.print_help()
        sys.exit()

    # Program continues ...

The mutually exclusive check is not optimal and will become very ugly in case of three, four, ... options. How can I improve this code?

What you are looking for is add_mutually_exclusive_group. It allows you to define options that are mutually exclusive as well as specifying if selecting one of these options is a required action or not. And looking at your else: block, yours is; meaning if we call the program without arguments, it should fail with an error along the following:

usage: [-h] (-t TEXT | -f FILE)
program_name.py: error: one of the arguments -t/--text -f/--file is required

You are also handling files in your input and should avoid doing so manually. argparse provides a FileType object meant to be used as the type keyword of add_argument to handle files for you. It will fail with a proper error message if the file cannot be opened with the proper mode and will return the opened file object in the resulting parsed arguments.

Proposed improvements:

#!/usr/bin/env python

import argparse

def parse_command_line():
    parser = argparse.ArgumentParser()
    group = parser.add_mutually_exclusive_group(required=True)
    group.add_argument('-t', '--text', help='Text')
    group.add_argument('-f', '--file', type=argparse.FileType('r'), help='File containing the text')
    return parser

if __name__ == '__main__':
    args = parse_command_line().parse_args()

    if args.text is not None:
        text = args.text
    elif args.file is not None:
        with args.file as f:
            text = f.read()

    # Program continues ...

Also note the use of with to properly close the provided file.


Self-made linked list iterator I recently made a linked list for myself, the code of which was posted here, if anyone would be interested. Now, as the next task, I had to make an Iterator for my list, which is working properly.

public Iterator<T> iterator() {
    final MyLinkedList<T> list = this;
    return new Iterator<T>() {
        final Node<T> firstNode = list.firstNode;
        Node<T> currentNode = null;
        @Override
        public boolean hasNext() {
            if (list.isEmpty()) {
                return false;
            } else if (currentNode == null){
                return true;
            } else if (currentNode == list.lastNode){
                return false;
            }
            return true;
        }
        @Override
        public T next() {
            if (list.isEmpty()){
                throw new NoSuchElementException();
            } else if (currentNode == null){
                this.currentNode = firstNode;
                return currentNode.data;
            } else if (currentNode.nextNode == null) {
                throw new NoSuchElementException();
            }
            this.currentNode = currentNode.nextNode;
            return currentNode.data;
        }
    };
}

Here are the variables of the LinkedList class:

private Node<T> lastNode;
private Node<T> firstNode;
private int size;

This is a Node<U>:

private static class Node<U>{
    private Node<U> nextNode = null;
    private Node<U> previousNode = null;
    private int index;
    private U data;

    private Node(U data){
        this.data = data;
    }
}

And this is how I add a new Node:

public boolean add(T data) {
    if (data == null) {
        return false;
    }

    Node<T> currentNode = new Node<T>(data);

    if (this.isEmpty()){
        currentNode.previousNode = null;
        currentNode.index = 0;
        this.firstNode = currentNode;
    } else {
        currentNode.previousNode = lastNode;
        lastNode.nextNode = currentNode;
        currentNode.index = lastNode.index + 1;
    }
    this.lastNode = currentNode;
    this.size++;
    return true;
}

Could this be made significantly faster? Does the code comply with the Java coding conventions?

Does the code comply with the Java coding conventions?

I think it does comply quite well. A tiny thing is that it would be better to put a space between ){, for example in if (list.isEmpty()){. More importantly, it can be simplified.

There is no need for the inner list variable pointing to this.
The inner (non-static) class has direct access to the containing class' fields and methods: it can access firstNode directly from the containing list. So in this code:

final MyLinkedList<T> list = this;
return new Iterator<T>() {
    final Node<T> firstNode = list.firstNode;

You can drop both the list variable and the firstNode variable.

It's a minor thing, but currentNode can be private.

The hasNext method can be simplified by a lot:

@Override
public boolean hasNext() {
    if (list.isEmpty()) {
        return false;
    } else if (currentNode == null){
        return true;
    } else if (currentNode == list.lastNode){
        return false;
    }
    return true;
}

This is equivalent:

@Override
public boolean hasNext() {
    return !isEmpty() && currentNode != lastNode;
}

  Could this be made significantly faster?

Slightly. Notice that the isEmpty() check is pointless to perform for every iteration of a non-empty list. So as a minor optimization,
you could check for emptiness once, in the beginning,
and then omit all the isEmpty() checks from the rest of the code.

Suggested implementation

Putting it all together:

public Iterator<T> iterator() {
    if (isEmpty()) {
        return Collections.<T>emptyList().iterator();
    }
    return new Iterator<T>() {
        private Node<T> currentNode = null;

        @Override
        public boolean hasNext() {
            return currentNode != lastNode;
        }

        @Override
        public T next() {
            if (currentNode == null) {
                currentNode = firstNode;
                return currentNode.data;
            }
            if (currentNode.nextNode == null) {
                throw new NoSuchElementException();
            }
            currentNode = currentNode.nextNode;
            return currentNode.data;
        }
    };
}

Unit testing

I didn't just refactor aggressively. I wrote unit tests first to know that I'm not breaking anything. After that I could go ahead and refactor aggressively:

@Test
public void testEmpty() {
    MyLinkedList<Integer> list = new MyLinkedList<>();
    assertFalse(list.iterator().hasNext());
}

@Test(expected = NoSuchElementException.class)
public void throwIfNextOnEmpty() {
    MyLinkedList<Integer> list = new MyLinkedList<>();
    list.iterator().next();
}

@Test(expected = NoSuchElementException.class)
public void throwIfIterateBeyond() {
    MyLinkedList<Integer> list = new MyLinkedList<>();
    list.add(1);
    list.add(2);
    Iterator<Integer> iter = list.iterator();
    iter.next();
    iter.next();
    iter.next();
}

@Test
public void testStandardIteration() {
    MyLinkedList<Integer> list = new MyLinkedList<>();
    Integer[] items = { 12, 3, 4 };
    for (int i : items) {
        list.add(i);
    }
    Iterator<Integer> iter = list.iterator();
    for (Integer item : items) {
        assertTrue(iter.hasNext());
        assertEquals(item, iter.next());
    }
    assertFalse(iter.hasNext());
}

These may not be perfect, and might not cover all corner cases, but I hope they are enough to get you started.


Code Improvement: Copy Regex named capture groups to strongly typed object This code works to copy named capture groups of a Regex to the same named properties of a strongly typed object.  However, it's not pretty.  Someone mentioned I should use polymorphism to at least remove the five if statements.

What should I consider when improving this code?  Suggested reading?

// Copies named capture groups in a REGEX to the properties of an object
// if the names match
static public void CopyProperties(Regex regex, Match source, object target)
{
    if (target != null && source != null && source.Success)
    {
        var targetType = target.GetType();
        // only step through non numeric capture names
        foreach (var sourceProp in regex.GetGroupNames()
            .Where(p => !Regex.IsMatch(p, @"^\d+$") &&
            !String.IsNullOrWhiteSpace(p)))
        {
            // does the target property the same as source exist?
            var targetPropName = targetType.GetProperty(sourceProp);
            // if the source and target exist
            if (targetPropName != null &&
                !String.IsNullOrWhiteSpace(source.Groups[sourceProp].Value) )
            {
                var targetPropSetter = targetPropName.GetSetMethod();
                // created a variable for each possible type
                // because I couldn't inline the Convert.ToInt32, etc.
                string sourceValue = source.Groups[sourceProp].Value;
                int? intValue = !Regex.IsMatch(sourceValue,
                                  @"^\d{1,10}$")
                    ? (int?) null
                    : Convert.ToInt64(sourceValue) <= Int32.MaxValue
                        ? Convert.ToInt32(sourceValue)
                        : (int?) null;
                long? longValue = Regex.IsMatch(sourceValue,
                                  @"^\d{1,19}$")
                    ? Convert.ToInt64( sourceValue)
                    : (long?) null;
                bool? boolValue = Regex.IsMatch(sourceValue,
                                  @"^(?i)true$|^(?i)false$")
                    ? Convert.ToBoolean(sourceValue)
                    : (bool?) null;
                decimal? decimalValue = Regex.IsMatch(sourceValue,
                                  @"^\d+\.?\d*$")
                    ? Convert.ToDecimal(sourceValue)
                    : (decimal?) null;
                // multiple if because it didn't like a switch statement
                // I didn't think five micro functions would improve this
                if (targetPropName.PropertyType == typeof(string))
                    targetPropSetter.Invoke(target,
                        new[] { (object) sourceValue });
                if (targetPropName.PropertyType == typeof(Nullable<Int32>))
                    targetPropSetter.Invoke(target,
                        new[] { (object) intValue });
                if (targetPropName.PropertyType == typeof(Nullable<Int64>))
                    targetPropSetter.Invoke(target,
                        new[] { (object) longValue });
                if (targetPropName.PropertyType == typeof(Nullable<Boolean>))
                    targetPropSetter.Invoke(target,
                        new[] { (object) boolValue });
                if (targetPropName.PropertyType == typeof(Nullable<Decimal>))
                    targetPropSetter.Invoke(target,
                        new[] { (object) decimalValue });
            }
        }
    }
}

By seeing the structure of your code structure i assume that all those if staements can easily be changed by a 'switch case' - which obviuosly is a code smell. I think you need to refactor your code. For further details please visit this link


Check if the shop is open or closed The working hours are saved in Database like

{"1" : "11:00 - 14:30", "2" : "17:30 - 23:00"}

Now i wrote a function like, which test is the shop is opened or close at the moment. Can someone please suggest if there is a better way to do this? And how i can check if the timing are like

{"1" : "11:00 - 14:30", "2" : "17:30 - 02:00"}

Where 02:00 o clock is next day's time.
here is my funciton.

$working_hours = json_decode('{"1" : "11:00 - 14:30", "2" : "14:39 - 23:00"}');
$is_open = false;
$time = time();

foreach ($working_hours as $key=>$value)
{
   $working_hour = explode(" - ", $value);
   $from = strtotime($working_hour[0]);
   $to = strtotime($working_hour[1]);

   if ($time >= $from && $time <= $to)
   {
       $is_open = true;
       break;
   }
}

echo "\n***********************";
echo "\n". date("d.m.Y H:i", $time);

if($is_open)
   echo "\nOpend";
else
   echo "\nClosed";

echo "\n***********************";

Easiest solution (to the time problem) is to change your logic (just a tiny bit).

if ($time >= $from && $time <= $to)

To:

if (($to > $from && ($time >= $from && $time <= $to)) ||
    ($to < $from && ($time >= $from || $time <= $to)))

You just change whether to restrict the time to be between the two, or to restrict it to be greater than one or less than the other in the case that the $to time is less than $from.

As far as cleaning up your code: this is about as idiomatic as php will get. Though you should definitely extract this to a function as appropriate.


Loading images in the background for a table view I have fetchData method which fills my imagesArray:

func fetchData(){

    let imageQuery = PFUser.query()
    imageQuery?.whereKey("username", containedIn: namesArray)

    imageQuery!.findObjectsInBackgroundWithBlock { (objects: [PFObject]?, error: NSError?) -> Void in

        if error == nil {

            for object in objects! {
                print("\(object["username"] as! String)")

                self.imagesArray.append(object["image"] as! PFFile)
                if self.imagesArray.count == self.namesArray.count {
                    self.tableView.reloadData()
                    print("Names Order: \(self.namesArray)")
                }
            }
        } else {
            print("error: \(error?.localizedDescription)")
        }
    }
}

And I have tableView:cellForRowAtIndexPath method which uses this array to fill the UIImageView instance:

override func tableView(tableView: UITableView, cellForRowAtIndexPath indexPath: NSIndexPath) -> UITableViewCell {

    let cell = tableView.dequeueReusableCellWithIdentifier("Cell") as! ChatsCell
    cell.nameLabel.text = namesArray[indexPath.row]

    if imagesArray.count == namesArray.count && self.imagesLoaded == false{
        imagesArray[indexPath.row].getDataInBackgroundWithBlock { (imageData: NSData?, error: NSError?) -> Void in
            if error == nil {
                let image = UIImage(data: imageData!)
                cell.imageView?.image = image
                self.tableView.reloadData()
                self.imagesLoaded = true
            }
        }
    }

    return cell
}

Actually I use if condition for waiting my array imagesArray to be filled by query of another method and I use variable imagesLoaded to prevent infinite call of reloadData method. How can I write this method without if statement?

Is there any reason that imagesArray and namesArray have to be separated? If not, I probably will design a struct and join above two arrays to one since name and image can be fetched at the same time in func fetchData().

For example, let's assume we have a struct like:

struct Person {
    var username: String
    var image: PFFile
}

Then we have a global mutable array:

var items = [Person]()

Inside the for-loop in func fetchData(), we can create a new person and append it each time once we have a name and an image.

let person = Person(username: "myName", image: "myImage")
self.items.append(person)

Therefore, in the func tableView(tableView: UITableView, cellForRowAtIndexPath indexPath: NSIndexPath) -> UITableViewCell {}, we can get a person and associated username and image directly according to indexPath.row

let person = self.items[indexPath.row]
let username = person.username
let image = person.image

The benefits like this is that,

Cell count is restricted by self.items.count; therefore, no
worries to take care of indexPath.row beyond an array.
Also username and image are always paired. It also helps to avoid
concerning the order as imagesArray and namesArray are separated. No
doubt, the condition imagesArray.count == namesArray.count can
be removed as well.
As to boolean self.imagesLoaded which is able to be removed. Owing to before remote data has been fetched, self.items.count is 0 and none cell will be rendered at that moment.


Speeding up Project Euler 43 - sub-string divisibility The number, 1406357289, is a 0 to 9 pandigital number because it is
  made up of each of the digits 0 to 9 in some order, but it also has a
  rather interesting sub-string divisibility property.

  Let d1 be the 1st digit, d2 be the 2nd digit, and so on. In this way,
  we note the following:

  d2d3d4=406 is divisible by 2
  d3d4d5=063 is divisible by 3
  d4d5d6=635 is divisible by 5
  d5d6d7=357 is divisible by 7
  d6d7d8=572 is divisible by 11
  d7d8d9=728 is divisible by 13
  d8d9d10=289 is divisible by 17

  Find the sum of all 0 to 9 pandigital numbers with this property.

Project Euler 43

from itertools import permutations
from primes import primes_upto
from collections import Counter
from timeit import default_timer as timer

start = timer()
def follows_property(n):
    divisors = primes_upto(17)
    for k in range(7):
        if int(n[k:(k+3)]) % divisors[k] != 0:
            return False
    return True

ans = 0
digits = Counter(range(10))

start = timer()

for combo in permutations(range(10), 9):
    num = ''.join([str(x) for x in list(combo)])
    if follows_property(num):
        missing = int(list((digits - Counter(sorted([int(k) for k in str(num)]))).elements())[0])
        num = int(num)
        ans += int("%d%d" % (missing, num))

elapsed_time = (timer() - start) * 1000 # s --> ms

print "Found %d in %r ms." % (ans, elapsed_time)

You can make a few quick improvements without altering the algorithm significantly:

Remove one of the redundant calls to timer().
Store the list of primes instead of calculating it for every call to follows_property.
Convert the digits to strings in the list passed to permutations so you can simplify the calculation of num.
Run through all permutations instead of 9-tuples and remove the Counter and missing parts.

These are minor changes, but they clean up the code a bit. I also renamed ans to sum to clarify what it holds. They also cut the running time by more than half.

from itertools import permutations
from primes import primes_upto
from collections import Counter
from timeit import default_timer as timer

divisors = primes_upto(17)

def follows_property(n):
    for k in range(7):
        if int(n[k+1:(k+4)]) % divisors[k] != 0:
            return False
    return True

sum = 0
start = timer()

for combo in permutations([str(x) for x in range(10)]):
    num = ''.join(combo)
    if follows_property(num):
        sum += int(num)

elapsed_time = (timer() - start) * 1000 # s --> ms

print("Found %d in %r ms." % (sum, elapsed_time))


Refactoring to avoid the use of dynamic Based on the answer to my question on StackOverflow, I have ended up with the following code:

public class ColumnDataBuilder<T>
{
    public abstract class MyListViewColumnData
    {
        public string Name { get; protected set; }
        public int Width { get; protected set; }
        public ColumnType Type { get; protected set; }
        public delegate TOUT FormatData<out TOUT>(T dataIn);

        protected abstract dynamic GetData(T dataRow);

        public string GetDataString(T dataRow)
        {
            dynamic data = GetData(dataRow);
            switch (Type)
            {
                case ColumnType.String:
                case ColumnType.Integer:
                case ColumnType.Decimal:
                    return data.ToString();
                case ColumnType.Date:
                    return data.ToShortDateString();
                case ColumnType.Currency:
                    return data.ToString("c");
                    break;
                case ColumnType.Boolean:
                    var b = (bool)data;
                    if (b) return "Y";
                    else return "N";
                default:
                    throw new ArgumentOutOfRangeException();
            }
        }

    }

    public class MyListViewColumnData<TOUT> : MyListViewColumnData
    {

        public MyListViewColumnData(string name, int width, ColumnType type, FormatData<TOUT> dataFormater)
        {
            DataFormatter = x => dataFormater(x); // Per https://stackoverflow.com/a/1906850/298754
            Type = type;
            Width = width;
            Name = name;
        }
        public Func<T, TOUT> DataFormatter { get; protected set; }
        protected override dynamic GetData(T dataRow)
        {
            return DataFormatter(dataRow);
        }
    }
 }

This is called from a factory method (in ColumnDataBuilder) as

public MyListViewColumnData Create<TOUT>(string name, int width, ColumnType type, MyListViewColumnData.FormatData<TOUT> dataFormater)
{
    return new MyListViewColumnData<TOUT>(name, width, type, dataFormater);
}
public MyListViewColumnData Create(string name, int width, MyListViewColumnData.FormatData<DateTime> dataFormater)
{
    return new MyListViewColumnData<DateTime>(name, width, ColumnType.Date, dataFormater);
}
...

That, in turn, is called from my code as:

builder.Create("Date", 40, x => x.createdDate);

and

private ListViewItem CreateListViewItem<TDATA>(IEnumerable<ColumnDataBuilder<TDATA>.MyListViewColumnData> columns, TDATA rowData)
{
    var item = new ListViewItem();
    foreach (var col in columns)
    {
        item.SubItems.Add(col.GetDataString(rowData));
    }
    item.SubItems.RemoveAt(0); // We generate an extra SubItem for some reason.
    return item;
}

How can I refactor this so that I'm not using dynamic, but still preserve the syntax as it currently exists in the code?

I don't think you need MyListViewColumnData class there, I would replace it with interface and move the GetDataString implementation to MyListViewColumnData<TOut>. And you don't need dynamic here, just use object instead (yes, it will use boxing for most cases except strings, but it's more efficient than dynamics).

public class ColumnDataBuilder<T>
{
    public interface IMyListViewColumnData
    {
        string Name { get; }
        int Width { get; }
        ColumnType Type { get; }
        string GetDataString(T dataRow);
    }

    public delegate TOut FormatData<out TOut>(T dataIn);

    public class MyListViewColumnData<TOut> : IMyListViewColumnData
    {
        public string Name { get; private set; }
        public int Width { get; private set; }
        public ColumnType Type { get; private set; }
        private readonly FormatData<TOut> _dataFormatter;

        public MyListViewColumnData(string name, int width, ColumnType type, FormatData<TOut> dataFormater)
        {
            _dataFormatter = dataFormater;
            Type = type;
            Width = width;
            Name = name;
        }
        public string GetDataString(T dataRow)
        {
            object data = _dataFormatter(dataRow);

            switch (Type)
            {
                case ColumnType.String:
                case ColumnType.Integer:
                case ColumnType.Decimal:
                    return data.ToString();
                case ColumnType.Date:
                    return ((DateTime)data).ToShortDateString();
                case ColumnType.Currency:
                    return ((decimal)data).ToString("c");
                case ColumnType.Boolean:
                    return (bool)data ? "Y" : "N";
                default:
                    throw new ArgumentOutOfRangeException();
            }
        }
    }

    public IMyListViewColumnData Create<TOut>(string name, int width, ColumnType type, FormatData<TOut> dataFormater)
    {
        return new MyListViewColumnData<TOut>(name, width, type, dataFormater);
    }

    public IMyListViewColumnData Create(string name, int width, FormatData<DateTime> dataFormater)
    {
        return new MyListViewColumnData<DateTime>(name, width, ColumnType.Date, dataFormater);
    }
}

public enum ColumnType
{
    String,
    Integer,
    Decimal,
    Date,
    Currency,
    Boolean
}

Update
In comments it was asked if you can extract interface for ColumnDataBuilder. Of course you can :), and the easiest way would be to use "Extract interface" refactoring from ReSharper :). If you still don't use it you'll have to do that manually (move the IMyListViewColumnData and FormatData<TOut> declarations out of ColumnDataBuilder<T> first):

public interface IColumnDataBuilder<Tin>
{
    IMyListViewColumnData Create<TOut>(string name, int width, ColumnType type, FormatData<TOut> dataFormater);
    IMyListViewColumnData Create(string name, int width, FormatData<DateTime> dataFormater);
}


Python simple parser using tags First ever code review post, so I hope I'm doing it right. My understanding of this site is that it is a place to post stuff like this for feedback and criticism. If that isn't the case, I apologise for my misunderstanding.

I'm teaching myself Python 3 and I'm pretty near the start. I couldn't find a simple description of a file parser, so I spent a frankly embarrassing amount of time trying to craft one, and this is the result:

def parse(file, startTag, endTag):
    bears=[]
    bearFile = open(file, 'r')
    for bearLine in bearFile:
        if startTag in bearLine:
            bear = bearFile.readline()
            while not (endTag in bear):
                bears.append(bear.strip())
                bear = bearFile.readline()
    return bears

I tested it using:

print(parse("bears.txt", "==start==", "==end=="))

...on a file which looked like this:

This file contains a list of bears.

Bears are below, between the start and end tags.

==start==
Grizzly
Polar
Koala
Panda
Spectacled
Sun
==end==

These are some more tags, listing some false bears:

==start==
Purple
Hairless
Aquatic
Flying
==end==

...and it works! The output is:

['Grizzly', 'Polar', 'Koala', 'Panda', 'Spectacled', 'Sun', 'Purple', 'Hairless', 'Aquatic', 'Flying']

With some obvious flaws, including:

1 - I can't work out how to ignore empty lines between the tags without using another if. I wondered if it might be better to do this later, and strip out all the '' entries, perhaps as an argument option.

2 - It depends on new lines because of the readline method, so using something else like a comma as a delimiter is currently beyond me.

3 - If there is no end tag, it loops forever. try and except are quite new to me right now.

I hope I'm not just wasting everyone's time by being here...

Python has a style guide, PEP8. I recommend that you follow it, as it makes your code easier to read.
In this case it's mostly just using snake_case, rather than camelCase.
You should use with to automatically close the file. This happens in any circumstance.
Rather than using if startTag in bearFile, I'd invert the if and continue.
Rather than using while you can use for still. This, IMO, is easier to understand too.
You could use a generator function so you don't have to write as much.

Along with a couple of name edits, here's how I'd change your code:

def parse(path, start, end):
    with open(path) as file:
        for line in file:
            if start not in line:
                continue

            for line in file:
                if end in line:
                    break
                yield line.strip()

print(list(parse("bears.txt", "==start==", "==end==")))

To address your concerns:

Do this out of the function. If you ever do want these empty values, then you'll be duplicating code. You also only need to use:

[item for item in parse(...) if item]

You could implement a function, split_delim, that splits on a delimiter. If you make the function simple, then all you'd need to do is add one line to your function:

def parse(path, start, end):
    with open(path) as file:
        file = split_delim(file, '\n')
        for line in file:
            ...

My changes should remove this error, due to how Python's for loops work. However I havn't tested this.


A program that reads 8763x2 values from a CSV file and initialises an array and few other variables Relatively new to Python. Have some experience in C++.

I have written a small program that reads a CSV file (SAMPLE) and initialises two arrays and few values. There are 2 values in each line, and there are 8763 such lines. The first value of each line is put into one array and the second value is put into the second array. The last three values of each array (total 6) are then given to another variable.

I want further calculations as accurate as possible hence I've tried the decimal.Decimal approach for Float Point Arithmetic, but for some reason, it's not that accurate. I think I'm using it wrong. It is still in an acceptable tolerance.

Also, would like to know if there is a better/efficient way to initialise the array and/or variables.

Here is code:

    alpha = np.zeros(8763, dtype='float32')  # Edited in later for clarity
    gamma = np.zeros(8763, dtype='float32')  # Edited in later for clarity
    rf = 100
    counter = 0
    with open('CSVData.csv', 'r') as csv_file_in:
        csv_reader = csv.reader(csv_file_in)
        for line in csv_reader:
            alpha[counter] = (float(decimal.Decimal(line[0])))  # Alpha Angle Initialized
            gamma[counter] = (float(decimal.Decimal(line[1])))  # Gamma Angle Initialized
            if counter == 8762:
                break
            counter = counter + 1
    csv_file_in.close()

    # Initializing last 6 parameters
    C_NSX = rf * alpha[8760]
    C_NSY = rf * gamma[8760]
    C_EWX = rf * alpha[8761]
    C_EWY = rf * gamma[8761]
    NSC = int(alpha[8762])
    EWC = int(gamma[8762])

Here the value of C_NSX should be 2790 but is 2789.9999185. (Debug Image)

EDIT: ADDED INFO ON THE PURPOSE OF THIS CODE

Following is my problem statement:

There is a matrix of octagons, for example, 4 octagons in a row and 3 such rows. So 4 columns and 3 rows of octagons. But they are not arranged in a perfect rectangle form.

Case1:

The first Octagon, O(0,0) has coordinates(0,0), while the one at its right and the one at the bottom are at a bit offset. So O(0,1) has coordinates (15,50) and O(1,0) has coordinates (30,15). This information is enough to define the whole array of octagons. Now next requirement is to find how much is the net visible area of these octagons. In the first example, as the coordinates are far away, there would be no overlapping and total visible area is simply...

Total_Visible_Area = Area_of_Single_Octagon * No_Of_Rows * No_Of_Columns

But when the coordinates are a bit complex, for example, Case 2:

Here, the distance between the adjacent octagons is less than their width and hence they are overlapping. Coordinates also are negative (which does not matter much actually, just something to note). Now to determine the visible area for this I wrote the following function.

The piece of code that I have written above is the first step to calculate all these. Each case is defined using Alpha, Gamma, C_NSX, C_NSY, C_EWX, C_EWY, NSC and EWC. Among these Alpha and Gamma have 8760 different values (mostly zeroes) and all others (capitals) are constants. I have to find the overlapping area in each of the 8760 cases. The code for finding those I had discussed here earlier, and thought won't be necessary to bring out again. After that last question, I have moved to python from CPP and started learning OpenCV.

You don't really mention what are the values used for. You'll always have a tradeoff between speed and memory (in this case memory amounts to how many digits you have).

Some issues with your code:

1) You're casting your decimal.Decimal(line[0]) as float, that should negate the advantage of using Decimal. And this might be a reason why you see the unexpected inconsistency. You have two options (in my opinion), you drop the use of numpy in favor of array from the standard library or you drop the pretense of using Decimal and use a pure numpy solution. But this will depend on what exactly you're doing.

2) Looping over the CSV Reader is a way to read out the data, but numpy implements better ways to load a csv. See the last example in the documentation here.

Thus, in the end the question back is, what for do you want the numbers to be a accurate as possible? Keep in mind that the values in the file are already "wrong" as they most likely come already from a float type, so in my opinion, you don't gain anything by using Decimal in this case. A different scenario would be if the values are integers and then in your program you're doing some divisions or multiplications, in that case, you have from the beginning control of the floating-point values and then using Decimal might be what you want to do.

Keep in mind, that highly sophisticated scientific research and their calculations use 64-bit floats and they do just fine.


Wrapper template functions for memset() I have these three helper functions that I've used on some projects as a slightly better alternative to the C memset function. The objective is to simplify and clarify code, as well as add asserts where possible:

// Zero fills a POD type, such as a structure or union.
template<class T>
void ZeroStruct(T & s)
{
    std::memset(&s, 0, sizeof(T));
}

// Zero fills a statically allocated array of POD or built-in types. Array length inferred by the compiler.
template<class T, size_t N>
void ZeroArray(T (&arr)[N])
{
    std::memset(arr, 0, sizeof(T) * N);
}

// Zero fills an array of POD or built-in types, with array length provided by the caller.
template<class T>
void ZeroArray(T * arr, size_t arrayLength)
{
    assert(arr != nullptr);
    assert(arrayLength != 0);
    std::memset(arr, 0, sizeof(T) * arrayLength);
}

Usage example:

int arr[128];
ZeroArray(arr);

struct S {
    int   x;
    float y;
};
S s;
ZeroStruct(s);

I'd like to further improve those functions with C++11/14 features. I think it would be a good idea to static_assert with std::is_pod to ensure they are never called on a class instance. Do you agree?

Also, should I make them noexept?

Any other recommendations?

Don't do that it is so easy to have incorrect usage.

All your cases can be done using normal syntax in C++.

// int arr[128];
// ZeroArray(arr);

int arr[128] = {0}; // Zero whole array.
// Or better yet use a vector.
std:vector<int>  arr(128); // zero init the vector of 128 elements.
// Or alternatively an array
std::array<int,128> arr(0);

// S s;
// ZeroStruct(s);
S s = S();          // Calls the default constructor for zero initialization.

See:
C++ implicit copy constructor for a class that contains other objects
Proper way to initialize C++ structs
Does a c++ struct have a default constructor?


Finding a delimiting character in a text file I am writing a program that is capable of reading a matrix supplied from a text file. One intended feature of this program is that it should be capable of detecting various delimiters in the text file.

So, for example, these inputs should be interpreted equally:

1,2,3,4
5,6,7,8
9,10,11,12

1:2:3:4
5:6:7:8
9:10:11:12

My code for detecting the delimiting character is as follows:

#include <stdio.h>
#include <limits.h>

static int isNumeric(unsigned char c)
{
    switch(c) {
    case '.':
    case '0' ... '9':
        return 1;
    default:
        return 0;
    }
}

unsigned char findDelimeter(FILE * f)
{
    rewind(f);
    int c;
    unsigned char delimeter = '\0';
    long long int frequencies[UCHAR_MAX] = { 0 };
    long long int maxFrequency = 0;
    while ((c = fgetc(f)) != EOF) {
        frequencies[c]++;
    }
    for (c = 0; c < UCHAR_MAX; c++) {
        if (frequencies[c] > maxFrequency && !(isNumeric(c) || c == '\n')) {
            maxFrequency = frequencies[c];
            delimeter = c;
        }
    }
    return delimeter;
}

I have a few concerns about this code:

Are the large data types, like long long int necessary in the findDelimeter function? Would it be better to use the types defined in stdint.h, like uint64_t? My intention was to support very large files.
How can I deal with the situation where an arbitrary number of spaces follow a delimiting character?

Regarding the last point, I would also like my code to be able to deal with text files that may use a space or a non-space delimiter followed by an arbitrary number of spaces. For example, the following inputs should be interpreted equally:

1 2 3 4
5 6 7 8
9 10 11 12

1,   2,  3,     4
5,6,  7,      8
9,    10, 11,12

Currently, it detects the delimiter as the most frequent, non-numeric character. So in the last example, it would detect the space character as the delimiter since it is the most common non-numeric character, and it would mistakenly include the comma as part of the number when interpreting it. One solution that I thought of was modifying the findDelimeter function to return a string (er, character array), but I anticipate that would not be able to deal with an arbitrary number of spaces.

Also, as usual, I very much appreciate any stylistic comments or potential bugs.

The first way to deal with this issue is to examine what is the actual problem that you're attempting to solve.  Understand the problem fully before you solve it or you will end up with a mess.  There are two parts to why you might need this function...

1) You want to process the data.

To perform processing you are looking to extract blocks of numbers.  Knowing the delimiter is not relevant to your processing of the data.  You're looking for 1 or more numbers which are contiguous and anything that is not a number can be ignored until you hit another number block.

That's your matrix processing taken care of without having to ever address any of your concerns or having a findDelimiter at all.

2) You want to reformat the file

If you want to output a formatted file with the same delimiter then your overall idea to identify a delimiter by building a frequency table and excluding certain values is on the right track.

To handle the issue of spaces, you can just set the [space] count to a maximum of 1 and if no other delimiters are present then then [space] will be the most frequent character.

There are some potential issues since you're dealing with arbitrary input.  You have no guarantee the delimiter will remain constant throughout the file; it wouldn't be unreasonable to see two matrix files concatenated for example.

Equally you can avoid dealing with the very large datatypes if you limit your scanning to the first few kilobytes of a file.  This would also provide a usable result without the overhead of processing the whole file; though you could argue that it's premature optimisation I would consider it justified in this case as you are expecting very large files.

Other potential issues...

This code assumes ASCII so far as I can tell.  There could be some "undefined behaviour" should someone give this code a file with Unicode content.

You spelled delimiter as delimeter - while this shouldn't be a big issue as most IDE's autocomplete, it's worth keeping an eye on as if another coder worked on your code and spelled it properly there could be confusion.  Many IDE's offer spellcheck and you should make sure yours is set up.

isNumeric will be an existing function.  Though you could eliminate the need for it entirely by setting the frequency counts for characters you want to ignore (like 0...9 and control characters) to a frequency of 0 before you process the frequency table.  This would also help remove or manage some of the 'magic numbers' in your code and make the intent clearer.

It's good practice to delay defining variables until you need them.  This can be useful should you come to refactor the code as compiler errors will point out when you attempt to modify a variable before it has been initialised.

I'd like to see c and f have meaningful names and for there to be a few more blank lines to illustrate the program flow blocks - the steps of the process become more obvious with the blank lines (as they are here... build a frequency table, remove the unwanted values, de-emphasise space, find the most common term and return it).  When the higher levels are clearly visible it makes refactoring easier.

unsigned char findDelimiter(FILE * fileToScan)
{
    rewind(fileToScan);

    int character;
    long long int frequencies[UCHAR_MAX] = { 0 };
    while ((character = fgetc(fileToScan)) != EOF) {
        frequencies[character]++;
    }

    const char IGNORECHARS[] = "0123456789\r\n";
    // todo: set the IGNORECHARS frequencies to 0 here
    // todo: set [space] to a maximum of 1 here

    long long int maxFrequency = 0;
    unsigned char delimiter = '\0';
    for (character = 0; character < UCHAR_MAX; character++) {
        if (frequencies[character] > maxFrequency) {
            maxFrequency = frequencies[character];
            delimiter = character;
        }
    }

    return delimiter;
}

With the code formatted this way it's easier to see how you could split the frequency analysis from the file reading should you want to do that later.

delimiter should be initialised with a reasonable value like ',' or be checked to throw an exception if a delimiter has not been found.


Replacing DataContract serialization with Binary one in Azure Service Fabric Azure Service Fabric uses DataContract serialization, which is a total disaster – it violates LSP (wiki) - see all those KnownTypeAttribute things, making framework development basically impossible.

I made the following class be a parent of all ValueObject to convince Service Fabric actually consume Binary serialization when it thinks about Data Contract one:

[DataContract]
[Serializable]
[JsonObject(MemberSerialization.OptOut)]
public abstract class SerializableObject
{
    [DataMember]
    [JsonIgnore]
    byte[] State { get; set; }
    static object Recursive { get; } = new object();

    [OnSerializing()]
    void OnSerializing(StreamingContext context)
    {
        if (context.Context == Recursive)
            return;

        var formatter = new BinaryFormatter(null, new StreamingContext(StreamingContextStates.Other, Recursive));
        using (var stream = new MemoryStream())
        {
            formatter.Serialize(stream, this);
            State = stream.ToArray();
        }
    }

    [OnDeserialized()]
    void OnDeserialized(StreamingContext ctxt)
    {
        if (State == null)
            return;

        var formatter = new BinaryFormatter();
        using (var stream = new MemoryStream(State))
            Assign(formatter.Deserialize(stream));
    }

    void Assign(object obj)
    {
        foreach (var property in GetType()
            .GetProperties(BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic)
            .Where(p => p.CanRead && p.CanWrite))
            property.SetValue(this, property.GetValue(obj));
    }
}

So, actual objects could be something like this:

[DataContract]
[Serializable]
public class PacketRecieved : SerializableObject
{
    public PacketRecieved(DeviceId deviceId, InboundPacket packet)
    {
        DeviceId = deviceId;
        Packet = packet;
    }

    public DeviceId DeviceId { get; private set; }
    public InboundPacket Packet { get; private set; }
}

You should probably remove empty braces inside attributes. [OnSerializing()] => [OnSerializing].
This loop is very hard to read due to how you use offsets.

foreach (var property in GetType()
        .GetProperties(BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic)
        .Where(p => p.CanRead && p.CanWrite))
        property.SetValue(this, property.GetValue(obj));

Mainly because it is impossible to tell at first glace where foreach statement ends and loop body begins. Use offsets to separate them:

foreach (var property in GetType()
                        .GetProperties(BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic)
                        .Where(p => p.CanRead && p.CanWrite))
    property.SetValue(this, property.GetValue(obj));

Or just add { } braces (that's what I would do).
State is probably not the best name for a property, its too generic. Maybe SerializedState or BinaryData would do a better job at explaining its purpose.
It would be nice to have some control over which properties are deserialized. You know, similar to how you can use XmlIgnore attribute to skip a property during xml (de-)serialization.
Unless what you do should be completely obvious to anyone working with Azure, you should add some basic documentation. I am not familiar with Azure, and to me it is unclear what problem you are trying to solve, why do you solve it using inheritance as opposed to using separate non-abstract binary serializer or wtf Recursive property is used for. You might want to explain some of those things in your code using xml-documentation.


Shorten and optimize php custom code for wordpress page template and use shortcodes I'm pretty new to php coding, but i know wordpress for a long time. Also experimenting with bootstrap. I created a new Page Template and modified it a bit. What I'm looking for is to optimize this php code:

    <?php
        global $post;
        $post = get_post(56);
        setup_postdata($post);
        get_template_part( 'template-parts/content', 'page', get_post_format() );
    ?>

    <?php
        global $post;
        $post = get_post(1);
        setup_postdata($post);
        get_template_part( 'template-parts/content', 'page', get_post_format() );
    ?>

    <?php
        global $post;
        $post = get_post(3);
        setup_postdata($post);
        get_template_part( 'template-parts/content', 'page', get_post_format() );
    ?>

    <?php
        global $post;
        $post = get_post(2);
        setup_postdata($post);
        get_template_part( 'template-parts/content', 'page', get_post_format() );
    ?>

As you can see, it's calling for 4 specific post IDs, since i want them to appear on my homepage in a bootsrap grid. I guess this could be done better by using an array?
Any suggestion is very much appreciated! Stay healthy!

Here is an improvement, but I would recommend a few additional changes as well.
<?php global $post; ?>

    <?php
    $post = get_post(56);
    setup_postdata($post);
    get_template_part('template-parts/content', 'page');
    ?>

    <?php foreach ([1, 3, 2] as $post_id) : ?>

                <?php
                $post = get_post($post_id);
                setup_postdata($post);
                get_template_part('template-parts/content', 'page');
                ?>

    <?php endforeach; ?>

<?php
// Reset back to the original post
$post = get_queried_object();
setup_postdata($post);
?>

Since you're new to PHP here are some notes on PHP specifically

you only need to get the global variable once. Once a global variable is pulled in, it exists for the entire scope (whatever that scope happens to be).
if your column code is the same it should be in a foreach loop

Some WordPress stuff

get_template_part should only accept two parameters $slug and $name, maybe you meant get_template_part( 'template-parts/content', 'page' . get_post_format() ) possibly?
changing the global $post will typically cause a lot of problems down the line, so I would not do that unless you're certain it needs to be done. I would recommend something like a WordPress shortcode or PHP function that handles your template for non-current post data. Only use get_template_part for calling templates for the currently queried post.
if you do change the global $post, you'll (typically) need to reset it back to the original value because the rest of the (WordPress) code is dependent on it.

Please let me know if you're interested in using a WordPress shortcode for your columns.
Hope that helps!
Edit: As a side note, it looks like your html is malformed and a little bit different from the init question. Can you please check that as well. Also, please let me know if you're interested in using WordPress shortcodes. We can certainly optimize more, as we can write a little extra something today to will provide us a lot of value in the future. If WordPress is something you're never going to come back to again, then these micro-optimizes are not worth while. If this is something you plan to stick we, then it would be a good idea to take things a bit further. Thanks!


Approach to download files through POST request I have a requirement to download multiple files using POST request. So I am using form submit to get this done.

I have followed two approaches to accomplish this.

Create a Form. Fill input details with payload data. Then set the target of the Form to _self then manually submit the Form. But I see few problems with this approach. Like, When payload is incorrect or Server has download API problems the current page will be redirected to 404 page.

var method = "POST";
var form = document.createElement("form");
form.setAttribute("method", method);
form.setAttribute("action", "https://some-url");
appendFormData(form, options.data.items);
document.body.appendChild(form);
form.submit();
document.body.removeChild(form);

2) Second approach is similar to the first one, except that I am creating an iframe, appending it to DOM and providing iframe as the target for Form. This works but the problem is, I am not sure when to remove the iFrame from the DOM.

    var method = "POST";
    var iframe = document.createElement("iframe");
    iframe.setAttribute("name", "hidden-frame");
    iframe.style.display = "none";
    document.body.appendChild(iframe);
    var iDocument = iframe.contentWindow.document;
    var form = iDocument.createElement("form");
    form.setAttribute("method", method);
    form.setAttribute("action", "https://some-url");
    form.setAttribute("target", "hidden-frame");
    appendFormData(form, options.data.items);
    iDocument.body.appendChild(form);
    form.submit();

Can you please suggest ways to improve the above solution or suggest if there are any better approach to accomplish the same.

Have you tried something like:

iframe.addEventListener('loadend', function() {
  document.body.removeChild(iframe);
});

The iframe has many other events (load, error, abort, progress) which might also be useful.