Checking whether a store is open or not by querying over its business hours I currently have a method that checks if the store is open. It's working nicely, however on the main page I have quite a few stores. Whom again have different locations. Each of these locations again have opening hours.

Now I'm not particularly good with SQL and DB querying and Skylight is complaining and telling me this can most likely be optimized. However I have no clue how.

This is taken from my Store model:

  has_many :business_hours, through: :locations
  ...

  def open?
    if current_business_hour
      true
    else
      false
    end
  end

  def current_business_hour(current_time: Time.current)
      business_hours.where('week_day = ? AND open_at <= ? AND close_at >= ?',
                           current_time.wday, current_time.to_s(:time), current_time.to_s(:time)).first
  end

I'm using Postgres. Now the code should be quite straight forward. I'm looking for business hours that match the current day, and that have a business hour that fits within the current time frame.

This generates:

SELECT  "business_hours".* FROM "business_hours" INNER JOIN "locations" ON "business_hours"."location_id" = "locations"."id" WHERE "locations"."store_id" = ? AND (week_day = ? AND open_at <= ? AND close_at >= ?) ORDER BY week_day LIMIT ?

I'm being notified that:

Skylight detected that this SQL query was repeated multiple times in a single request, which had a significant impact on its response time.

The stores are being rendered like this on a page:

<%= render @stores %>

the _store.html.erb looks like this:

<% if store.open? %>
  <li style="background-image: url(<%= store.header.url %>);">
    <%= link_to store_path(store) do %>

<%= store.name %>, <%= store.city.name %>

Open

    <% end %>

<% else %>
  <li style="background-image: url(<%= store.header.url %>);" class="store-closed">
    <%= link_to store_path(store) do %>

<%= store.name %>, <%= store.city.name %>

    <% end %>

Closed

<% end %>

Now my goal is to try and improve this.

Here's your problem:

<%= render @stores %>

<% if store.open? %>

You're rendering multiple stores via partials. When each partial is fully rendered out it looks like this:

<% if store.open? %>

<% if store.open? %>

<% if store.open? %>

<% if store.open? %>

Each call to

<% if store.open? %>

will result in a db query, which is where your n+1 problem is coming from.

You have 2 options:

1. Eager loading

#pseudocode
#controller
@stores = Store.includes(:business_hours).where(some_condition:...)

#partial
<% if store.is_open?(store.business_hours) %>

#model
def is_open?(business_hours)
   open = business_hours.detect {|h| (h.wday == Time.now.wday) && ((h.open_time..h.close_time).cover?(Time.now)) }
   open.present? ? true : false
end

May get very memory intensive if you have thousands of stores and many business_hours for each store.

2. Denormalize your database and cache weekdays and business hours in your table

#pseudocode
#migration
table "stores"
   t.integer wday
   t.string open_time
   t.string close_time

#model
def is_open?
  (self.wday == Time.now.wday) && ((self.open_time.to_time..self.close_time.to_time).cover?(Time.now))
end

The approach you take will depend on how many records you need to deal with at any one time.


Write to the event log without needing to check whether or not it exists by use of a `-Force` parameter Below is some powershell to write to the event log; if the -Force parameter is specified this saves you the effort of writing additional code to first check whether the log exists.

I had to make use of Remove-Variable to allow some parameters to flow through; I'd not used that feature before / been aware of that nuance, so not sure whether this is the best approach?

cls
$myLog = 'Application'
$mySource = 'My PS Script'
$myEventId = 1
[System.Diagnostics.EventLogEntryType]$myEntryType = [System.Diagnostics.EventLogEntryType]::Error
$myMessage = 'This is a test message'
$myMessage2 = 'This is a test message (2)'
$myMessage3 = 'This is a test message (3)'

function Write-EventLog2
{
    [cmdletbinding()]
    param(
        [Parameter(Position=0,Mandatory=$true,ValueFromPipeline=$true,ValueFromPipelineByPropertyName=$true)]
        [String]$LogName
        ,
        [Parameter(Position=1,Mandatory=$true,ValueFromPipeline=$true,ValueFromPipelineByPropertyName=$true)]
        [String]$Source
        ,
        [Parameter(Position=3,Mandatory=$true,ValueFromPipeline=$true,ValueFromPipelineByPropertyName=$true)]
        [Int32]$EventId
        ,
        [Parameter(Position=4,Mandatory=$false,ValueFromPipeline=$true,ValueFromPipelineByPropertyName=$true)]
        [System.Diagnostics.EventLogEntryType]$EntryType
        ,
        [Parameter(Position=5,Mandatory=$false,ValueFromPipeline=$true,ValueFromPipelineByPropertyName=$true)]
        [String]$Message
        ,
        [Parameter(Position=6,Mandatory=$false,ValueFromPipeline=$false,ValueFromPipelineByPropertyName=$true)]
        [Int16]$Category
        ,
        [Parameter(Position=7,Mandatory=$false,ValueFromPipeline=$false,ValueFromPipelineByPropertyName=$true)]
        [String]$ComputerName
        ,
        [Parameter(Position=8,Mandatory=$false,ValueFromPipeline=$false,ValueFromPipelineByPropertyName=$true)]
        [Byte[]]$RawData
        ,
        [Parameter(Position=9,Mandatory=$false,ValueFromPipeline=$false,ValueFromPipelineByPropertyName=$true)]
        [switch]$Force
    )

    begin
    {
        if($Force.IsPresent)
        {
            if (! ([System.Diagnostics.EventLog]::Exists($LogName) -and [System.Diagnostics.EventLog]::SourceExists($Source) ))
            {
                New-EventLog -LogName $LogName -Source $Source
            }
        }
    }
    process
    {
        if(!$EntryType) {Remove-Variable EntryType};
        if(!$Message) {Remove-Variable Message};
        if(!$Category) {Remove-Variable Category};
        if(!$ComputerName) {Remove-Variable ComputerName};
        if(!$RawData) {Remove-Variable RawData};
        Write-EventLog -LogName $LogName -Source $Source -EventId $EventId -EntryType $EntryType -Message $Message #-Category $Category -ComputerName $ComputerName -RawData $RawData
    }
    #end{} #no ending actions required
}

Write-EventLog2 $myLog $mySource $myEventId $myEntryType $myMessage -Force #ensure the log's created if doesn't exist
Write-EventLog2 $myLog $mySource $myEventId $myEntryType $myMessage2 -Force #just to demo that we don't get an error if source already exists
Write-EventLog2 $myLog $mySource $myEventId $myEntryType $myMessage3 #this is how we'd generall call once we were certain the command had been called at least once with -Force
Show-EventLog #check that the above code produced the expected logs

There is one fundamental flaw in this script, you need to run the whole script as admin to avoid errors of EventSource.Exists unless you ignore or hide them. But if you do that, then you can as well just use EventLog.WriteEntry that will create the source for you automatically.

There are few problems with your code. Let's try to go through them in the order they appear:
1) Specifying ValueFromPipeline for more than one parameter is not a  good thing to do. Value from pipeline is meant to easily bind input from the pipeline that can't satisfy the properties defined by ValueFromPipelineByPropertyName. Usually you use it to bind the most significant and most volatile piece of information. For Item related cmdlets it is Path. In your case it should be the message.

2) Specifying Force as ValueFromPipelineByPropertyName might have unexpected results. In my point of view you should only bind data this way, not behavior. So binding Message, Source, ComputerName and LogName is OK. Binding Force is not.

3) Specifying position for every parameter is not necessary, the order is inferred by the order of the parameters by default. Specifying position is needed only if you don't need some of the parameters to be positional or you want to define them in order different from their actual usage.

4) Specifying Mandatory=$false is not necessary, it is the default.

5) Switch converts to Boolean implicitly, there is no need to explicitly check for IsPresent.

6) I prefer to use -not instead of !, it keeps you with just one style of operators, plus it's easier to spot.

7) Use parameter splatting to forward complicated calls cmdlets or other functions. Splatting is a technique where you populate a hashtable with keys names as parameter names, and values that are the actual values for that parameter. See example in the code below.

8) Optionally in PowerShell 3 and later you don't need to provide $true to set options of parameters.

9) Optionally set default value to the message type.

 #this is how we'd generall call once we were certain the command had been called at least once with -Force

Why not check in the begin even if the -Force is not specified then?

function Write-EventLog2
{
    [cmdletbinding()]
    param(
        [Parameter(Mandatory,ValueFromPipelineByPropertyName)][String]$LogName,
        [Parameter(Mandatory,ValueFromPipelineByPropertyName)][String]$Source,
        [Parameter(Mandatory,ValueFromPipelineByPropertyName)][Int32]$EventId,
        [Parameter(ValueFromPipelineByPropertyName)][Diagnostics.EventLogEntryType]$EntryType,
        [Parameter(ValueFromPipeline,ValueFromPipelineByPropertyName)][String]$Message,
        [Parameter(ValueFromPipelineByPropertyName)][Int16]$Category,
        [Parameter(ValueFromPipelineByPropertyName)][String]$ComputerName,
        [Parameter(ValueFromPipelineByPropertyName)][Byte[]]$RawData,
        [Switch]$Force
    )

    begin
    {
        if($Force)
        {
            if (! ([System.Diagnostics.EventLog]::Exists($LogName) -and [System.Diagnostics.EventLog]::SourceExists($Source) ))
            {
                New-EventLog -LogName $LogName -Source $Source
            }
        }
    }
    process
    {
        $params = @{
            #put mandatory params here directly
            LogName = $LogName
            Source = $Source
            EventId = $EventId
        }
        if($EntryType) { $params.Add('EntryType', $EntryType) }
        if($Message) { $params.Add('Message', $Message) }
        if($Category) {$params.Add('Category', $Category) }
        if($ComputerName) { $params.Add('ComputerName', $ComputerName) }
        if($null -ne $RawData) { $params.Add('RawData', $RawData) }

        #since your parameters are named the same way as Write-EventLog parameters, and you use cmdlet binding
        #you might get away with just specifying @PSBoundParameters
        Write-EventLog @params
    }
}

Another option would be to implement a proxy function for the cmdlet, more info here


Find values that add up to a number Input

  The input for this problem is a String of sorted numbers and a special number. The syntax of the input would be 1,2,3,4;5

  Output

  The program will output all of the possible sums that equate to the special number. The output for the example would be 1,4;2,3

I solved the problem by using some simple subtraction. The program will begin at the greatest value and work its way to the lowest value. For each value the program will subtract it from the pre-set value and search the array for that value. I want to know what I could do better for this.

import java.util.*;
public class NumPairs {
    static String line = "1,2,3,4;5";
    public static void main(String[] args) {
        String fina = "";
        if(!line.equals("")){
        String a = line.substring(line.indexOf(";")+1,line.length());
        String[] valu = line.substring(0,line.indexOf(";")).split(",");
        for(int h = valu.length-1; h >0; h--){
            int need = Integer.parseInt(a) - Integer.parseInt(valu[h]);;
            if(Arrays.asList(valu).indexOf(need + "") != -1 && fina.indexOf(need +"") == -1 && need != Integer.parseInt(valu[h])){
                if(need > Integer.parseInt(valu[h]))
                fina += valu[h] +","+need +";";
                else
                fina += need +","+ valu[h] + ";";
            }
        }
        }
        if(fina.equals(""))
        System.out.println("NULL");
        else
        System.out.println(fina.substring(0,fina.length()-1));
    }
}

Lack of separation of concerns

You have a single main function that:

Reads a String and parses it to an internal format
Applies logic to it (finds numbers that sum to the target)
Pretty prints the result

You should have at least 3 more functions:

public class NumPairs {

    // Pseudocode, to return two objects at once read
    // http://stackoverflow.com/questions/457629/how-to-return-multiple-objects-from-a-java-method
    public static (int[], int) parseInput(String input) {

    }

    public static int[][] sumsTo(int[] list, int result) {

    }

    public static String prettify(int[][] result) {

    }

    public static void main(String[] args) {
        // Simply use the other functions
    }

}

You may still need to subdivide sumsTo surther.


Automated test system I am creating an automated test system (scope bloat - the original intent was a one-off bash automation of a particular test case that has turned into creating a system that will do many other things). I've done a few things similar to this, but somehow never got to using classes; everything was down at the functions level in modules I wrote for past projects.

I'm looking for a simple check/confirmation that I'm approaching this right. It's a communications module intended to connect via telnet to a remote Linux machine, then pull various I2C reads from a device that the Linux machine is connected to.

I want to make sure that I am creating this correctly correctly, and declaring objects correctly so the functions/methods in the class object will perform their individual tasks correctly. I think I'm on the right track but having people a little more experienced take a look couldn't hurt. In particular, I want to make sure I'm setting the class up for instantiation correctly, and declaring self/__init__ in the right way.

Note: the code here worked reasonably well when it was separate functions not gathered under a unifying class structure, but I was mucking about with global declarations which made me nervous, and it took more tweaking than I was happy with.

class Communications: #logs in to the remote as root.
    def __init__(self):
        self.term = telnetlib.open('10.100.100.103')
        print ('logging in now')
        term.read_until(b"ogin: ")
        term.write('root\n')
        time.sleep(3)
        term.read_very_eager()

    #device power-up and power-down
    def node_startup(node):
        if node == 1:
            node_command = 'node_on 1\n'
        elif node == 2:
            node_command = 'node_on 2\n'
        term.write(node_command)
        print (b'powering node', node, 'on\n')

    def node_shutdown(node):
        if node == 1:
            node_command = 'node_off 1\n'
        elif node == 2:
            node_command = 'node_off 2\n'
        term.write(node_command)
        print (b'shutting node', node, ' down')

    #device sample retrieval
    def get_voltage_i2c(bus, device, datapoint, test, node):
        i2c_sample =  term.write('i2cget -y -f %d %d 0x 8b w >> /tmp/%s_%d.txt\n' %(bus, device, test, node))

    def get_current_i2c(bus, device, datapoint, test, node):
        i2c_sample =  term.write('i2cget -y -f %d %d %d %s %d w >> /tmp/%s_%d.txt\n' %(bus, device, datapoint, test, node))

    def get_power_i2c(bus, device, datapoint, test, node):
        i2c_sample =  term.write('i2cget -y -f %d %d %d %s %d w >> /tmp/%s_%d.txt\n' %(bus, device, datapoint, test, node))

    #output-scraper to slurp results from the output files the i2cgets write to. (devnote: incomplete at time of posting)
    def get_sample_results():
        time.sleep(30)
        term.read_very_eager()
        term.write(b'cat ')

First thing I noticed is you don't seem to be checking if your commands are executed. There's no Exception handling to be found.

Second, avoid the use of magic numbers.

telnetlib.open('10.100.100.103') should be telnetlib.open(TARGET_IP)

time.sleep(3) should be time.sleep(SLEEP_INIT)

Etcetera. Ofcourse, those variables should be declared on the top of your file. Although Python does not support constant by default, there's a workaround if you're interested.


C strlcat implementation I have to reproduce some basic functions in C and one of them is strlcat.

After reading the man and testing the strlcat from <bsd/string.h> library, I wrote the following code which seems to be a working copy of the function:

#include <string.h>

size_t  ft_strlcat(char *dst, const char *src, size_t size)
{
    size_t  lsize;
    size_t  dsize;

    lsize = 0;
    dsize = strlen(dst);
    while (*dst && size > 0 && size--)
    {
        dst++;
        lsize++;
    }
    while (*src && size > 1 && size--)
        *dst++ = *src++;
    if (size == 1)
        *dst = '\0';
    return (dsize + lsize);
}

Is this implementation missing something?

Using freebsd strlcat as a guide.

"length" with C strings typical does not include the null character.  "size" usually does include the null character.  Suggest name change

// dsize = strlen(dst);
dlen = strlen(dst);

Code walks the array and repeatedly adjust 3 variables.  Could be done adjusting only 2.

while (*dst && size > 0 && size--) {
    dst++;
    lsize++;
}
// vs.
size_t remaining_size = size;
while (*dst && remaining_size > 0) {
  dst++;
  remaining_size--;
}
size_t lsize = size - remaining_size;

The below could use memcpy() or strcpy().  These functions are typically optimized for speedy execution, especially long strings.  The key performance assessment should be based on long strings and with sufficient space.  Little need to optimize for undersized values of size.

while (*src && size > 1 && size--)
  *dst++ = *src++;
// vs.
memcpy(dst, src, ...);

Code does not handle overlapping strings.  Should use restrict to indicate that.

size_t ft_strlcat(char * restrict dst, const char * restrict src, size_t size);

Pedantic code would test for overflow in dsize + lsize, but let us skip that corner case for now.
Initialize when declaring, if possible.

// size_t  lsize;
// lsize = 0;
size_t  lsize = 0;

Some lightly tested code follows.

size_t ft2_strlcat(char *restrict dst, const char *restrict src, size_t size) {
  size_t s_length = strlen(src);
  size_t remaining_size = size;

  while (*dst && remaining_size > 0) {
    dst++;
    remaining_size--;
  }
  // Length of destination is the lesser of `size` and
  // offset of the null character, if any.
  size_t d_length = size - remaining_size;

  if (d_length < remaining_size) {
    // We know there is at least room to write the \0

    size_t copy_length = remaining_size - 1;
    if (s_length < copy_length) {
      copy_length = s_length;
    }
    memcpy(dst, src, copy_length);
    dst[copy_length] = '\0';
  }
  return d_length + s_length;
}


Cluster date time values I came up with this TSQL for SQL Server 2008:

DECLARE @TimeWindowInSeconds INT

SET @TimeWindowInSeconds = 10

IF OBJECT_ID('tempdb..#Temp') IS NOT NULL
    DROP TABLE #Temp
CREATE TABLE #Temp
    (
      Class INT,
      DT DateTime
    )

INSERT INTO #Temp (Class, DT)
SELECT 1, '2014-11-05 10:55:00'
    UNION ALL
SELECT 1, '2014-11-05 10:55:10'
    UNION ALL
SELECT 1, '2014-11-05 11:55:10'
    UNION ALL
SELECT 2, '2014-11-05 10:55:11'
    UNION ALL
SELECT 2, '2014-11-05 13:56:10'

;WITH CTE1 AS
(
    SELECT
        Class,
        DT,
        ROW_NUMBER() OVER (ORDER BY Class, DT ) AS RowNumber
    FROM #Temp
)
,CTE2 AS
(
    -- A is the successor
    SELECT
        A.Class,
        A.RowNumber,
        B.RowNumber AS RowNumber1,
        A.DT,
        B.DT AS DT2,
        DATEDIFF(second, B.DT, A.DT) AS DifferenceInSeconds,
        CASE WHEN B.DT IS NULL THEN 1 END z
    FROM CTE1 AS A
    LEFT OUTER JOIN CTE1 AS B ON A.RowNumber = B.RowNumber + 1 AND A.Class = B.Class
    AND DATEDIFF(second, B.DT, A.DT) <= @TimeWindowInSeconds

)
,CTE3 AS
(
    SELECT
        CTE2.Class,
        CTE2.DT,
        (
            SELECT
                SUM(z)
            FROM      CTE2 c
            WHERE     c.DT <= CTE2.DT AND c.Class = cte2.Class
                ) AS SubGroup
    FROM CTE2
)
SELECT
    *
FROM CTE3
    WHERE CAST(CTE3.Class AS NVARCHAR(100)) + '-' + CAST(CTE3.SubGroup AS NVARCHAR(100)) IN
    (
        SELECT CAST(CTE3.Class AS NVARCHAR(100)) + '-' + CAST(CTE3.SubGroup AS NVARCHAR(100)) FROM CTE3 GROUP BY CTE3.Class, CTE3.SubGroup
HAVING COUNT(*) > 1
    )

This clusters datetime values inside groups (class) if they fall inside the same time window and if the cluster contains more than 1 entry. Please review the code.

I agree with the other feedback given here so far; I have two three suggestions of my own, two minor and one more significant.

Minor point (1)

Initial assignation to a variable can be done in one line, so instead of:

DECLARE @TimeWindowInSeconds INT

SET @TimeWindowInSeconds = 10

you could just write:

DECLARE @TimeWindowInSeconds INT = 10

Minor point (2)

I am not a fan of selecting columns in intermediate rowsets that are never used subsequently. They add clutter and unnecessarily make the reader spend time understanding or visualising them.

CTE1 selects just three columns, all of which are necessary for the join in CTE2: this is good.

However, CTE2 then selects seven columns, of which only A.Class, A.DT and the expression aliased as z are required by CTE3 or presented in the final output.

I suspect that you put these columns in whilst writing CTE2, because the others are all used in the join within that CTE. This is fine for work-in-progress, but these columns should be removed from the finished query. They don't need to be in the SELECT list just because they're used in the JOIN.

The bigger one

Your code uses both common table expressions (CTEs) and subqueries. For me, this is quite a painful inconsistency. CTEs were effectively an evolution of subqueries in the ANSI SQL standard; mixing both in one query feels to me like designing an aeroplane that has both propellers and a jet engine.

Aside: please don't read too far into that analogy: this answer is not about their relative performance, nor am I claiming that one is made redundant by the other. People still use prop planes, after all.

Unless performance is particularly important, and you know for certain that subqueries perform measurably better on your server, with your data, I recommend sticking with CTEs alone:

The syntax is easier to follow
They mimic step-by-step problem solving, walking your reader through the solution steadily
Well-chosen CTE names are tantamount to good comments, explaining your rationale
They can make code shorter because you can re-use them

In your particular case, I think the SubGroup subquery will need to become a separate CTE between CTE2 and CTE3 (you'd then join to that CTE in the current CTE3, using c.DT <= CTE2.DT AND c.Class = cte2.Class as the joining conditions).

Having done that, if I've understood your intentions correctly, the correlated subquery right at the end (WHERE...IN ( subquery )) could be removed entirely, and replaced with a partitioned COUNT() function in the current CTE3 (because Subgroup will be available as a regular column in CTE3 now).

In other words, in order to work out which combinations of Class and Subgroup occur more than once, you'd add a column to your refactored CTE3 something like:

COUNT(*) OVER ( PARTITION BY Class, Subgroup ) as CountBy_Cls_Sgp

and then your final query outside the CTEs would simply restrict where  CountBy_Cls_Sgp > 1.

This also removes altogether the need to CAST and concatenate Class and Subgroup, since you're not including that concatenation in your result set.

Even if you didn't want to change the WHERE...IN ( subquery ) part, you could simplify things by adding an extra, final CTE in which you do the casting and concatenation of Class and Subgroup just once, give it an alias, and then do

WHERE alias IN ( SELECT alias FROM cte3 GROUP BY ... )


Get chat message data from table, grouped by date Giving the following table:

id sender_id receiver_id msg  date_added
    1    2          5       hii     2018-10-24 16:42:41
    2    5          2       hello     2018-10-25 16:42:41

I want to extract data as:

[2018-10-24]=>[{
                sender_id=>2,
                receiver_id=>5,
                msg=>hii}],
[2018-10-25]=>[{
               sender_id=>5,
                receiver_id=>2,
                msg=>hello
              }]

I'm getting it by first querying for the date and then grouping by dates. After, I loop through thoses dates to retrives date's data.

However, I think using loops with a lot of dates and users will slow the app quickly

The code I've used is:

 //the dates is used to get the dates from table using group by
$dates = $this->get_date_list($checkArr);
////then i loop through each date to get the corresponding data
foreach($dates as $key=>$value){
$msg = $this->get_msg_by_date($value,$checkArr);
$data[$value] = $msg;
}

//here is the code to get the date from the date column
public function get_date_list($checkArr){
$sql = "Select date(date_added) as new_date from tb_chats where (sender_id = $checkArr[sender_id] AND receiver_id = $checkArr[receiver_id]) OR (sender_id = $checkArr[receiver_id] AND receiver_id = $checkArr[sender_id]) GROUP BY new_date";
$query = $this->db->query($sql);
$data  = $query->result_array();
return array_column($data,'new_date');
}

//here is the code to get the data for the looped dates
public function get_msg_by_date($date,$checkArr){
$sql = "Select * from tb_chats where (sender_id = $checkArr[sender_id] AND receiver_id = $checkArr[receiver_id] AND date_added LIKE '$date%') OR (sender_id = $checkArr[receiver_id] AND receiver_id = $checkArr[sender_id] AND date_added LIKE '$date%')";
$query = $this->db->query($sql);
$data  = $query->result_array();
return $data;
}

Lots of little pieces of advice:

Tab the lines of code inside of your loops and functions (etc) for improved readability.
Only declare the $key variable in your foreach() loop if you intend to use it.
Avoid performing iterated trips to the database.  Furthermore, always endeavor to minimize total trips to the database unless you have a data volume issue to contend with.
Use meaningful variable names.  $checkArr does not speak clearly of the data that it holds.
Quote your associative array keys.
You aren't filtering out any dates between the two selected users, so you can avoid calling date-fetching queries entirely.
After writing your WHERE condition to isolate the messages between the pair of users, sort the rows by descending datetime.
Use all uppercase for MySQL reserved words for clarity / readability.
Before return $data, you can run a simple loop to group the message data by date.  Use the date value as the first level keys and just push the row data as subarrays.

Suggested single query: (well... really I recommend a prepared statement)

$query =
"SELECT DATE(date_added) AS `date`, sender_id, receiver_id, `msg
FROM tb_chats
WHERE
  (sender_id = " . (int)$checkArr['sender_id'] . " AND receiver_id = " . (int)$checkArr['receiver_id'] . ")
  OR
  (sender_id = " . (int)$checkArr['receiver_id'] . " AND receiver_id = " . (int)$checkArr['sender_id'] . ")
ORDER BY date_added DESC";

Grouping with php:

foreach ($data as $row) {
    $grouped[array_shift($row)][] = $row;
}
// this step can be performed in a number of different ways according to style preference / micro-optimization
return $grouped;

I don't know code igniter, so if the grouping can be done with an available function, I don't know it.


Run code every n invocations I came across a situation where I needed code to run every n invocations of a method.  Specifically, I was clearing out massive amounts of data and needed to hint to the VM that it should run garbage collection.  My solution was a little more condensed than this, but this class offers reuseability and ensures accurate counts.

public class ExecutionLimiter {
    private final AtomicInteger ai = new AtomicInteger(0);
    private final Runnable task;
    private final int runEvery;

    public ExecutionLimiter(Runnable task, int runEvery){
        this.task = task;
        this.runEvery = runEvery;
    }

    public boolean tryRun(){
        synchronized(ai){
            if(ai.incrementAndGet() != runEvery) return false;
            ai.set(0);
        }
        task.run();
        return true;
    }

    public static void main(String... args) throws InterruptedException{
        AtomicInteger example = new AtomicInteger(0);
        ExecutorService service = Executors.newWorkStealingPool(5);
        ExecutionLimiter limiter = new ExecutionLimiter(example::incrementAndGet, 10);

        int invocations = 100;

        for(int i=0;i<invocations;i++){
            service.submit(()->{
                try{
                    Thread.sleep(Math.round(Math.random() * 1000));
                }catch(InterruptedException ignore){}
                if(limiter.tryRun())
                    System.out.println(Thread.currentThread().getName() +
                            ": Successfully ran: value is " + example.get());
            });
        }

        service.shutdown();
        service.awaitTermination(invocations, TimeUnit.SECONDS);
    }
}

I can't help but feel that something like this exists already in Java, but I haven't been able to find it.  The closest I've come is CyclicBarrier, but I don't want to hang my threads while incrementing.

A few concerns:

Is AtomicInteger overkill for this class?  Would it be better to use an int and synchronize on a lock Object?
Should I be synchronizing AtomicInteger to perform the incrementAndGet-and-set, or is there a better way to go about this?

The main is just for demonstration.

synchronized(ai){
            if(ai.incrementAndGet() != runEvery) return false;
            ai.set(0);
        }

You should either use AtomicInteger or use synchronized.  Using both together is certainly overkill.

Consider

    private static int count = 0;
    private static final Object lock = new Object();

and later

        synchronized (lock) {
            if (++count != runEvery) {
                return false;
            }

            count = 0;
        }

Then you don't need the AtomicInteger at all.

I prefer a descriptive name like count to an abbreviated type name like ai.  This is especially so in this case, as my natural expansion of AI is Artificial Intelligence.

You use AtomicInteger when you don't want to use a synchronized block.  Perhaps

        int current = ai.incrementAndGet();
        if (current % runEvery != 0) {
            return false;
        }

        int next = 0;
        do {
            if (ai.compareAndSet(current, next) {
                task.run();
                return true;
            }
            current = ai.get();
            next = current - runEvery;
        } while (next >= 0);

Obviously this is more complicated than the synchronized version.  But this is how one uses an AtomicInteger.

The modulus is more reliable in case of high contention that keeps compareAndSet from returning true.

If we aren't worried that the count will overflow the bounds of integer, we can get rid of the reset.  Without the reset, this makes more sense:

        if (ai.incrementAndGet() % runEvery != 0) {
            return false;
        }

That's how an AtomicInteger is supposed to work.  Even if two threads call this code at the same time, only one will get past the check.  Because the update happens atomically.  The problem with the original code is that there are actually two updates and you need to synchronize them.

Of course, if the range for ai is not constrained, then it will overflow eventually.  The synchronized block is more reliable in that case.  This usage fits synchronized better than atomic types because there's no atomic increment, compare, and reset operation.

You could also use an explicit Lock here, but you don't seem to need it.  The blocking behavior of synchronized better matches what you are doing.  The lock would be better if it were OK to only sometimes update (when you can get the lock) and sometimes not (when you can't get the lock).  But that's not what you are doing here.

It would be possible to create a new class with the necessary method (countCompareReset), but it may be simpler just to use the lock variable with the synchronization block.


Throwing error if settings / arguments are undefined is this a good breakout function? var _ = require("underscore");

var checkSettingsUndefined = function(settings){
    if(_.isEmpty(settings)){
        throw new Error("settings are empty");
    }
}

var Class = function(settings){
    checkSettingsUndefined(settings);
    this.settings = settings;
}

Class.prototype.settings = function(){
    return this.settings;
}

module.exports = Class;

and here's my unit test

it("empty settings should throw error", function(){
    var Class = require("Class.js");
    expect(function(){
        return new Class();
    }).to.throwException(function (e) {
        expect(e.message).to.be("settings are empty");
    });
});

The abstraction is good but the tight coupling is bad.  The only real use case for checkSettingsUndefined is for your Class object.

Here is an example of a more generic method, it just takes an array of validation objects that will be run on their associated calling args.

/**
 * @param  {[Object]} validationArray Array of validation configs
 * that follow the api of
    {
        fn: function(value) {},
        message: ''
    }
 * @return Function
 */

function createArgValidatorFn(validationArray) {

    return function() {
        var argValue,
            validationFn,
            args = arguments;

        validationArray.forEach(function(obj, index) {
            validationFn = obj.fn;
            argValue = args[index];

            if (validationFn(argValue)) {
                throw new Error('argument[' + index + '] : '  + obj.message);
            }

        });
    }
}

You can use in Class by doing:

var classValidationFn = createArgValidatorFn([{
        fn: _.isEmpty,
        message: 'settings are empty'
    }
]);

var Class = function(settings){
    classValidationFn.apply(this, arguments);
    this.settings = settings;
};

This will allow to stuff like:

var fn = createArgValidatorFn([{
        fn: function(val) {
            return typeof val !== 'string'
        },
        message: 'Argument must be a string'
    }, {
        fn: function(val) {
            return !val;
        },
        message: 'Argument must be a truthy'
    }
])

fn('a');

  "argument[1] : Argument must be truthy"


Finding Possible Strings from Series of Numbers Given a string containing digits from 2-9 inclusive, return all possible letter combinations that the number could represent.

A mapping of digit to letters is done just like they are arranged on a telephone keypad.

Looking for ways of making the code more consise and easily readable. Want to learn the C# tricks to make solving or implementing this solution easier.

using System;
using System.Collections.Generic;

namespace ConsoleApp1
{
    class Program
    {
        static void Main(string[] args)
        {
            LetterCombinations("34");
        }
        public static void LetterCombinations(String number)
        {
            string[][] StringMapping = new string[number.Length][];
            int LengthIndex = 0;

            foreach (char c in number)
            {
                switch (c)
                {
                    case '2':
                        StringMapping[LengthIndex] = new string[] {"a", "b", "c"};
                        break;
                    case '3':
                        StringMapping[LengthIndex] = new string[] {"d", "e", "f"};
                        break;
                    case '4':
                        StringMapping[LengthIndex] = new string[] {"g", "h", "i"};
                        break;
                    case '5':
                        StringMapping[LengthIndex] = new string[] {"j", "k", "l"};
                        break;
                    case '6':
                        StringMapping[LengthIndex] = new string[] {"m", "n", "o"};
                        break;
                    case '7':
                        StringMapping[LengthIndex] = new string[] {"p", "q", "r", "s"};
                        break;
                    case '8':
                        StringMapping[LengthIndex] = new string[] {"t", "u", "v"};
                        break;
                    case '9':
                        StringMapping[LengthIndex] = new string[] {"w", "x", "y", "z"};
                        break;
                }
                LengthIndex++;
            }

            int[] SizeArray = new int[StringMapping.Length];        //Keeps track of the size of each inner String array
            int[] counterArray = new int[StringMapping.Length];     //Keeps track of the index of each inner String array, which will be used to make the next combination

            //Discover the size of each inner array and populate sizeArray.
            //Also calculate the total number of combinations possible using the inner String array sizes.
            int totalCombinationCount = 1;
            for (int i = 0; i < StringMapping.Length; i++)
            {
                SizeArray[i] = StringMapping[i].Length;
                totalCombinationCount *= StringMapping[i].Length;
            }

            //Store the combinations in a List of String objects
            List<String> combinationList = new List<String>(totalCombinationCount);
            String sb = "";

            for (int countdown = totalCombinationCount; countdown > 0; countdown--)
            {
                sb = "";
                for (int i = 0; i < StringMapping.Length; i++)
                {
                    sb += StringMapping[i][counterArray[i]];
                }

                combinationList.Add(sb.ToString());

                //Now we need to increment the counterArray so that the next
                //combination is taken on the next iteration of this loop.
                for (int incIndex = StringMapping.Length - 1; incIndex >= 0; incIndex--)
                {
                    if (counterArray[incIndex] + 1 < SizeArray[incIndex])
                    {
                        ++counterArray[incIndex];
                        break;
                    }
                    counterArray[incIndex] = 0;
                }
            }

            //Output the strings in the list
            foreach (String s in combinationList)
            {
                Console.WriteLine(s);
            }
        }
    }
}

A string implements IEnumerable<char> and has an indexer, so you can iterate it as an array.

You can therefore simplify this:

string[][] StringMapping = new string[number.Length][];
            int LengthIndex = 0;

            foreach (char c in number)
            {
                switch (c)
                {
                    case '2':
                        StringMapping[LengthIndex] = new string[] {"a", "b", "c"};
                 ...

without any other changes to:

string[] StringMapping = new string[number.Length];
            int LengthIndex = 0;

            foreach (char c in number)
            {
                switch (c)
                {
                    case '2':
                        StringMapping[LengthIndex] = "abc";

and your code will work fine.

You could though create a separate method for the mapping as:

private static string GetPattern(char number)
{
  switch (number)
  {
    case '2': return "abc";
    case '3': return "def";
    case '4': return "ghi";
    case '5': return "jkl";
    case '6': return "mno";
    case '7': return "pqrs";
    case '8': return "tuv";
    case '9': return "wxyz";
    default: throw new InvalidOperationException();
  }
}

So your StringMapping could be initialized as:

string[] stringMapping = number.Select(n => GetPattern(n)).ToArray();

Some would claim that a Dictionary<char, string> would be better instead of the switch statement. In a localized scenario for instance, where you would load the mapping dynamically that would be the solution.

Notice the naming: stringMapping (camelCase) for class fields and local variables instead of StringMapping (PascalCase) that is used for properties and method names.

I find SizeArray unnecessary and waste of memory:

  int[] SizeArray = new int[StringMapping.Length];

The only place you are using it is here:

      if (counterArray[incIndex] + 1 < SizeArray[incIndex])

which could be replaces with:

if (counterArray[incIndex] + 1 < stringMapping[incIndex].Length)

  for (int countdown = totalCombinationCount; countdown > 0; countdown--)

There is nothing wrong with iterating backward if the situation demands it, but at least I have to look an extra time, when I see a backward loop. Here there is no reason for the above backward iteration as you only use it to count combinations, the countdown index isn't used to anything in the loop, so it could run forward without any damage:

for (int countdown = 0; countdown < totalCombinationCount; countdown++)

Your algorithm works fine, but the overall impression is that it is more complex than it has to be, and due to the double nested loops it is not very efficient.

FYI: below are some other approaches (they all use the GetPattern() method shown above):

1) An iterative version that uses a list to cache all the combinations while creating them:

void LetterCombinationsIter(string numbers)
{
  IEnumerable<string> CreateCombinations()
  {
    List<string> combs = new List<string> { "" };

    for (int i = 0; i < numbers.Length; i++)
    {
      List<string> temp = new List<string>();
      string chars = GetPattern(numbers[i]);

      foreach (string comb in combs)
      {
        foreach (char ch in chars)
        {
          temp.Add(comb + ch);
        }
      }

      combs = temp;
    }

    return combs;
  }

  Console.WriteLine(string.Join(Environment.NewLine, CreateCombinations()));
}

2) A recursive version which is often the intuitive choice for this kind of problems:

void LetterCombinationsRec(string numbers)
{
  char[] buffer = new char[numbers.Length];

  IEnumerable<string> HandleIndex(int index)
  {
    if (index >= numbers.Length)
    {
      yield return new string(buffer);
    }
    else
    {
      foreach (char ch in GetPattern(numbers[index]))
      {
        buffer[index] = ch;
        foreach (string comb in HandleIndex(index + 1))
        {
          yield return comb;
        }
      }
    }
  }

  Console.WriteLine(string.Join(Environment.NewLine, HandleIndex(0)));
}

3) Finally a version using Linq that solves the problem in a few lines of code:

void LetterCombinationsLinq(string numbers)
{
  IEnumerable<string> CreateCombinations()
  {
    return numbers.Aggregate(new[] { "" }.AsEnumerable(), (combs, n) =>
    {
      string chars = GetPattern(n);
      return combs.SelectMany(pre => chars.Select(ch => pre + ch));
    });
  }

  Console.WriteLine(string.Join(Environment.NewLine, CreateCombinations()));
}


Combining a list of realtime responses with a list of static responses I have written this code that accepts a list of static times and a list of realtime-delayed times.  These are frequently being refreshed and there are times where a train, for example train 23, is scheduled for 2:00PM, so after 2:00PM it leaves the list.

But if it's delayed, I still want it to be a part of the final list (predictionArray), which is why I have the boolean flag set the way it is. So basically it compares every static response with every realtime response, and if true, it won't append to predictionArray (since all realtime responses are in there already).
Is there a way I could do this maybe without booleans?

Again, I need realtime responses in the predictionArray to begin with in case there are any realtime trains that have been added that aren't on the returned static schedule.

def coalesceMetraData(staticResponses, realtimeResponses):
    predictionArray = list(realtimeResponses)
    for response in staticResponses:
        responseExists = False
        for responseReal in realtimeResponses:
            if (response['key'] == responseReal['key']):
                responseExists = True
                break
        if (responseExists != True):
            predictionArray.append(response)

    predictionArray = sorted(predictionArray, key=lambda d:d['time'])

    return predictionArray[:5]

You are constructing a list out of realtimeResponses.

You then iterate over each item in staticResponses, and for each static response, you (slowly) look to see if that key is in your list of realtimeResponses.

Right there is your room for improvement: a list is not the professional data structure of choice for testing if an item is in a collection. For that, you want either a set or a dict.

In your case, you have extra data in your realtimeResponses, but you don't use it. So set seems to be the right choice: put your realtimeResponses key values into a set, and then you can just use code like:

    realKeys = {rtr['key'] for rtr in realtimeResponses}

    for response in staticResponses:
        if response['key'] in realKeys:
            predictionArray.append(response)

Of course, once you have a for and an if in a tight loop, you can replace it with a list comprehension:

predictionArray += [resp for resp in staticResponses if resp['key'] in realKeys]


Longest-match lookup via nested dicts I have the following datastructure to perform a longest-match lookup via nested dicts. For example. cs['a b c d'] with a and a b c being in the structure will return a b c and the remainder d.

I wonder if there's a better way to implement the _lookup method and possible the __setitem__ method. I'm also not completely sure if the way __getitem__ works (returning a tuple with the data in the structure and the remainder of the input) isn't a bit confusing when used with the [] operator.

class CommandStorage(object):
    """Stores multi-part commands.

    Performs fast lookups returning the command and any arguments which were
    not part of the command.
    """

    def __init__(self, commands={}):
        self._root = {}
        self._commands = {}
        for cmd, func in commands:
            self[cmd] = func

    def _lookup(self, line):
        parts = line.split(' ')
        found = None
        found_container = None
        container = self._root
        command_parts = 0
        for i, part in enumerate(parts):
            if part not in container:
                return found_container, found, parts[command_parts:]
            container = container[part]
            if None in container:
                found_container = container
                found = container[None]
                command_parts = i + 1
        return found_container, found, parts[command_parts:]

    def __setitem__(self, cmd, func):
        container = self._root
        parts = cmd.split(' ')
        for part in parts:
            container = container.setdefault(part, {})
        if None in container:
            raise ValueError('Command %s already exists' % cmd)
        container[None] = func
        self._commands[cmd] = func

    def __getitem__(self, line):
        return self._lookup(line)[1:]

    def __contains__(self, item):
        cmd, args = self[item]
        return cmd is not None and not args

    def __delitem__(self, cmd):
        if cmd not in self:
            raise KeyError(cmd)
        container = self._lookup(cmd)[0]
        del container[None]
        del self._commands[cmd]

    def __iter__(self):
        return iter(self._commands)

    def __len__(self):
        return len(self._commands)

    def __nonzero__(self):
        return bool(self._commands)

    def iterkeys(self):
        return self._commands.iterkeys()

    def iteritems(self):
        return self._commands.iteritems()

    def itervalues(self):
        return self._commands.itervalues()

    def __repr__(self):
        return '<CommandStorage(%r)>' % self._commands.keys()

The full code (with a doctest) is in this gist as the doctest is not really relevant for the question

class CommandStorage(object):
    """Stores multi-part commands.

    Performs fast lookups returning the command and any arguments which were
    not part of the command.
    """

    def __init__(self, commands={}):
        self._root = {}
        self._commands = {}

Do you really need to store all the data twice?

        for cmd, func in commands:

That should be commands.items(), guess you've never used the parameter

            self[cmd] = func

    def _lookup(self, line):
        parts = line.split(' ')
        found = None
        found_container = None
        container = self._root
        command_parts = 0
        for i, part in enumerate(parts):
            if part not in container:
                return found_container, found, parts[command_parts:]
            container = container[part]

The recommended way to structure something like this to catch the KeyError, so:

try:
    container = container[part]
except KeyError:
    return found_container, found, parts[command_parts:]
else:
    proceed

As it stands, your code looks up the container[part] twice.

            if None in container:

Using None to store the actual value is counter-intuitive. That's not what None usually means.

                found_container = container
                found = container[None]
                command_parts = i + 1

Having these three pieces of data feels ugly.

        return found_container, found, parts[command_parts:]

I think the problem is your data structure. Your collection of dicts of dicts of dicts is tricky to navigate.

    def __setitem__(self, cmd, func):
        container = self._root
        parts = cmd.split(' ')
        for part in parts:

I'd combine these last two lines

            container = container.setdefault(part, {})
        if None in container:
            raise ValueError('Command %s already exists' % cmd)
        container[None] = func
        self._commands[cmd] = func

    def __getitem__(self, line):
        return self._lookup(line)[1:]

I think this is a bad idea. This function doesn't act like [] typically acts in python. So just make this a regular method

    def __contains__(self, item):
    def __delitem__(self, cmd):
    def __iter__(self):
    def __len__(self):
    def __nonzero__(self):
    def iterkeys(self):
    def iteritems(self):
    def itervalues(self):

You've implemented all these functions to make it act like a dictionary. But its not a dictionary. Unless the object is designed to provide the features of a dictionary, don't attempt to provide the dictionary interface.

    def __repr__(self):
        return '<CommandStorage(%r)>' % self._commands.keys()

Here's how I'd approach it. Don't use nested dicts, just store everything in one dict. __setitem__ becomes trivial:

def __setitem__(self, cmd, func):
    parts = tuple(cmd.split(' '))
    if parts in self._commands:
        raise ValueError('Command %s already exists' % cmd)
    else:
        self._commands[parts] = func

Then, _lookup becomes:

def _lookup(self, line):
    parts = tuple(line.split(' '))
    # start with longest match, work backwards
    # first match found will be longest match
    for index in xrange(len(parts), -1, -1):
        try:
            function = self._commands[parts[:index]]
        except KeyError:
            pass # no match found, try next one
        else:
            break # match, stop search
    else:
        function = None # if all else fails, report None as function

    return function, list(parts[index:])


Counting items in categories I'm an experienced programmer that also has a little experience with functional programming, although mostly theoretical (as in hobby-level reading and very minor projects).

I recently decided to work through the new book "Beginning Haskell" to get up to speed with Haskell and try to crank out something worthy of a github repo. Early in the book you are asked to implement a Client ADT including some supportive data types for Person and Gender. I did it like this and I'm not liking it at all:

data Gender = Male | Female | Unknown | None
              deriving (Show, Eq)

data Person = Person { fName :: String, lName :: String, gender :: Gender }
              deriving (Show, Eq)

data Client = GovOrg { code :: String,     name :: String }
            | Individual { code :: String, person :: Person }
            | Company { code :: String,    name :: String, contact :: Person }
              deriving (Show, Eq)

After this you are asked to write a function that takes [Client] and returns the count for the different genders, e.g. "out of the 20 clients listed, we have 5 male, 5 female, 10 unknown". Let's call it countGenders. Below is the code I've ended up with, and I feel that it's a complete mess and lacks any feeling of clarity or elegance. I think my OOP-brain is forcing me into bad designs. I'll walk you through the lines below. Once again I am not at all happy with this.

First we have a result data type. Could be just an (Int, Int, Int), but I feel like the location in the tuple isnt a natural fit with what the data should represent (as opposed to Point Int Int):

data GenderCount = GenderCount { male :: Int, female :: Int, unknown :: Int }

Then we have the top level function that is supposed to be invoked. Maps genderCount across a [Client], which results in [(Int, Int, Int)], then unzips those and sums each list.

countGenders :: [Client] -> GenderCount
countGenders cs = GenderCount (sum m) (sum f) (sum u)
    where (m, f, u) = unzip3 $ map genderCount cs

Then we have genderCount, which maps a Client to a result tuple (Male, Female, Unknown):

genderCount :: Client -> (Int, Int, Int)
genderCount c = case cGender c of
                     Male    ->  (1, 0, 0)
                     Female  ->  (0, 1, 0)
                     Unknown ->  (0, 0, 1)
                     None    ->  (0, 0, 0)

Last we have cGender, which gets the gender from a client (using RecordWildCards extension):

cGender :: Client -> Gender
cGender GovOrg {}         = None
cGender Individual { .. } = gender person
cGender Company { .. }    = gender contact

As you can see this is all a mess. But I'm having trouble thinking of how to refine it. I feel like my data model is bloated, which bleeds over into too many functions to do something that should be really simple. I would appreciate any feedback on the data model, the functions or anything else.

Here is the entire code if you want to look it over in full context: http://pastebin.com/Vymcd6Bj

This isn't that much of a mess at all. Let's clean it up, then get a little fancy.

Right off the bat it seems a little funky to have a None Gender, as it appears you're not actually trying to account for agender individuals but instead providing for the failure to produce a Gender for a particular Client. Operations that might fail in Haskell usually signal so by returning a Maybe value, so let's drop None from the definition and see what needs changing.

data Gender = Male | Female | Unknown
              deriving (Show, Eq)

genderCount and cGender depend on None, let's start with the latter. c doesn't mean much as a prefix to me, so I'm going to call this function clientGender, but that's a stylistic preference. Changing this function to use Maybe is straightforward.

clientGender :: Client -> Maybe Gender
clientGender GovOrg {}       = Nothing
clientGender Individual {..} = Just (gender person)
clientGender Company {..}    = Just (gender contact)

Now let's turn to genderCount. The first thing to notice is that the function is a tiny wrapped up case statement. This is usually a code smell to me that hints at a missed opportunity to break functions into smaller, more independent pieces. In this case genderCount has nothing to do with Clients, so let's rip that part out!

genderCount :: Gender -> (Int, Int, Int) -- Counting Genders, not Clients!
genderCount Male    = (1, 0, 0)
genderCount Female  = (0, 1, 0)
genderCount Unknown = (0, 0, 1)

This is still unsatisfactory, right? There's that convention-typed tuple, and we've got a perfectly good GenderCount datatype lying around, so let's use it.

genderCount :: Gender -> GenderCount
genderCount Male    = GenderCount 1 0 0
genderCount Female  = GenderCount 0 1 0
genderCount Unknown = GenderCount 0 0 1

It's a small change, but users of GenderCount can rely on the field names rather than a tuple ordering.

And now countGenders, the piece that glues it all together. Our type is still correct, which is awesome! No changes there. The implementation though is doing a few different things we'll need to adjust. In an informational sense it's determining the genders of all of the clients, then accumulating a count of each Gender. What it looks like though is some weird tuple math to produce a GenderCount value from nowhere! We can rewrite it given our new implementations to be a little prettier, but first we're going to need a way to add two GenderCounts together.

addGenderCounts :: GenderCount -> GenderCount -> GenderCount
addGenderCounts (GenderCount m f u) (GenderCount n g v) = GenderCount (m + n) (f + g) (u + v)

A lot of repeated instances of GenderCount in there, but that's not so bad given we can use this as a combinator. Now we can put our countGenders function together.

countGenders :: [Client] -> GenderCount
countGenders = foldr (addGenderCounts . genderCount) (GenderCount 0 0 0) . mapMaybe clientGender

This works! I've imported mapMaybe from Data.Maybe here to account for our clientGender function sometimes returning Nothing (mapMaybe drops all the Nothings and returns a list of the Just values). We use a right fold to accumulate our GenderCount values, and a starting value of GenderCount 0 0 0 for our accumulator.

There are a few ways to go from here to clean things up further. You could get rid of the GenderCount 0 0 0 value by using foldr1, at the cost of adding another composition with map into the mix. If you have sharp eyes and a working knowledge of the Typeclassopedia you'll note a striking similarity between the way we use GenderCount with a right fold, and a Monoid. If you don't have a working knowledge of the Typeclassopedia, our motivation is that Monoids allow us to specify an identity element and an associative reduction operation, revealing some higher level abstractions (and functions) we can use to wire our code together. Let's make a Monoid.

instance Monoid GenderCount where
    mempty = GenderCount 0 0 0
    mappend = addGenderCounts

-- Laws:
-- mempty <> x = x
-- x <> mempty = x
-- x <> (y <> z) = (x <> y) <> z

I won't prove the Monoid laws, but you should be able to see that they are trivial given the properties of addition and our definition of GenderCount.

Let's try one more pass at countGenders now.

countGenders :: [Client] -> GenderCount
countGenders = foldMap genderCount . mapMaybe clientGender

Nice!


K&R 2-7 - Invert a bitfield from a number This is my solution to the Exercise 2-7 of K&R C book. The assignment is:

  Write a function invert(x,p,n) that returns x with the n bits that begin at position p inverted (i.e., 1 changed into 0 and vice versa), leaving the others unchanged.

Note: It's assumed that the rigthmost bit of a number has position 0.

Example: If x = 8 (00001000 in binary), p = 2 and n = 3, the result should be 20 (00010100), because bits 2, 3 and 4 are inverted.

Code:

#include <stdio.h>

unsigned invert(unsigned x, int p, int n);

int main(void)
{
    // just a test
    printf("%u\n", invert(8, 2, 3));
    return 0;
}

unsigned invert(unsigned x, int p, int n)
{
    /* The right part makes a mask with 1's under the
       desired bits to be inverted. Then, they're inverted
       by X0Ring it and x.
    */
    return x ^ (~(~0 << n) << p);
}

Just a few notes, since the code is so short:

Put your main after all of your other function declarations so you don't have to declare the function prototypes at the top.
You don't have to return 0 at the end of main(). The C standard  knows how frequently this is used, and lets us omit it.

  C99 & C11 §5.1.2.2(3)

  ...reaching the } that terminates the main() function returns a
  value of 0.

Move your comment to the top of the function and explicitly say what the parameters are and what the output will be.
Your function parameter names could be better:

unsigned invert(unsigned num, int position, int num_bits)

Those parameters to your function could be declared const
Does your function perform as is supposed to with negative numbers?  Zeros?  Other edge cases?  You should have more tests in your code to make sure you get the expected behavior, and if not handle those edge cases more appropriately.  Use assert from the standard library header <assert.h> to help with this.


Architecture Logging I have two database tables UsersDownload, Non-UsersDownload. There is also two model object and two service that take in the object and insert the data into the correct table. Is this the best architecture for this? or is there a better way to handle this? Would it be better to use an interface?

public class UserLog
{
    public string CustomerId { get; set; }
    public bool? IsMobile { get; set; }
    public int? DeviceId { get; set; }
}
public class NonUserLog
{
    public string DeviceId { get; set; }
    public bool? IsMobile { get; set; }
    public string AppType { get; set; }
}

insert using Store Procedure

 internal ResultStatus UsersDownload(UserLog userLog)
        {
            queryName = "usp_Insert_UserLog";
            SQLHelper sql = new SQLHelper(conn);
            ResultStatus rs = new ResultStatus();
            SqlParameter[] parms = new SqlParameter[] {
                            new SqlParameter("@CustomerID", SqlDbType.NVarChar, 50),
                            new SqlParameter("@IsMobile", SqlDbType.Bit),
                        new SqlParameter("@DeviceId", SqlDbType.Int)
            };

            parms[0].Value = guidelineLog.CustomerId;
            parms[1].Value = guidelineLog.IsMobile;
            parms[2].Value = guidelineLog.DeviceId;

            using (SqlDataReader dataReader = sql.ExecuteReaderStoreProcedure(queryName, parms))
            {
                rs.ResultCode = 1;
                rs.ResultMessage = "Success";
                dataReader.Close();
            }

            return rs;
        }
 internal ResultStatus NonUsersDownload(NonUserLog nonUserLog)
        {
            queryName = "usp_Insert_NonUserLog";
            SQLHelper sql = new SQLHelper(conn);
            ResultStatus rs = new ResultStatus();
            SqlParameter[] parms = new SqlParameter[] {
                            new SqlParameter("@DeviceId", SqlDbType.NVarChar, 50),
                            new SqlParameter("@IsMobile", SqlDbType.Bit),
                        new SqlParameter("@AppType", SqlDbType.NVarChar)
            };

            parms[0].Value = guidelineLog.DeviceId;
            parms[1].Value = guidelineLog.IsMobile;
            parms[2].Value = guidelineLog.AppType;

            using (SqlDataReader dataReader = sql.ExecuteReaderStoreProcedure(queryName, parms))
            {
                rs.ResultCode = 1;
                rs.ResultMessage = "Success";
                dataReader.Close();
            }

            return rs;
        }

service

public ResultStatus InsertUsersDownload(UserLog userLog)
        {
            return glDb.UsersDownload(userLog);
        }

        public ResultStatus InsertNonUsersDownload(NonUserLog nonUserLog)
        {
            return glDb.NonUsersDownload(nonUserLog);
        }

It'll add more work if you use interface since your models are different, only one property (Mobile) is the common one, and you'll add Reflection to the equation as well. Even if you used a generic method, you'll add extra work to it. So, in your case, your best approach would be to use overloads (which you've already done) and keep it simple, but you've over-done it. Rather than just duplicating the method and change the model, you'll need to set a private base method, which is going to be used as call back. The other overloads will be set to each process respectively, then it'll callback the base method.

// UserLog overload
internal ResultStatus DownloadsLog(UserLog log)
{
    var parameters = new SqlParameter[]
    {
        new SqlParameter { ParameterName = "@CustomerID", SqlDbType = SqlDbType.NVarChar, Size = 50, Value = log.CustomerId },
        new SqlParameter { ParameterName = "@IsMobile", SqlDbType = SqlDbType.Bit, Value = log.IsMobile },
        new SqlParameter { ParameterName = "@DeviceId", SqlDbType = SqlDbType.Int, Value = log.DeviceId }
    };

    return DownloadsLog("usp_Insert_UserLog", parameters);
}

// NonUserLog overload
internal ResultStatus DownloadsLog(NonUserLog log)
{
    var parameters = new SqlParameter[]
    {
        new SqlParameter { ParameterName = "@DeviceId", SqlDbType = SqlDbType.NVarChar, Size = 50, Value = log.DeviceId },
        new SqlParameter { ParameterName = "@IsMobile", SqlDbType = SqlDbType.Bit, Value = log.IsMobile },
        new SqlParameter { ParameterName = "@AppType", SqlDbType = SqlDbType.NVarChar, Value = log.AppType }
    };

    return DownloadsLog("usp_Insert_NonUserLog", parameters);
}

// base
private ResultStatus DownloadsLog(string procedureName, SqlParameter[] parameters)
{
    using (var dataReader = new SQLHelper(conn).ExecuteReaderStoreProcedure(procedureName, parameters))
    {
        return new ResultStatus
        {
            ResultCode = 1,
            ResultMessage = "Success"
        };
    }
}

as you can see, the UserLog and NonUserLog overloads are there just to set the parameters with the procedure name. Then, I just callback the base method.

Update

This is another approach where you define the parameters and the store procedure name in the model itself, kinda like gathering everything related into one box.

public interface ISqlDbLog
{
    string GetSqlProcedureName();

    SqlParameter[] GetSqlParameters();
}

public class UserLog : ISqlDbLog
{
    public string CustomerId { get; set; }

    public bool? IsMobile { get; set; }

    public int? DeviceId { get; set; }

    public string GetSqlProcedureName() => "usp_Insert_UserLog";

    public SqlParameter[] GetSqlParameters()
    {
        return new SqlParameter[]
        {
            new SqlParameter { ParameterName = "@CustomerID", SqlDbType = SqlDbType.NVarChar, Size = 50, Value = CustomerId },
            new SqlParameter { ParameterName = "@IsMobile", SqlDbType = SqlDbType.Bit, Value = IsMobile },
            new SqlParameter { ParameterName = "@DeviceId", SqlDbType = SqlDbType.Int, Value = DeviceId }
        };
    }
}

public class NonUserLog : ISqlDbLog
{
    public string DeviceId { get; set; }

    public bool? IsMobile { get; set; }

    public string AppType { get; set; }

    public string GetSqlProcedureName() => "usp_Insert_NonUserLog";

    public SqlParameter[] GetSqlParameters()
    {
        return new SqlParameter[]
        {
            new SqlParameter { ParameterName = "@DeviceId", SqlDbType = SqlDbType.NVarChar, Size = 50, Value = DeviceId },
            new SqlParameter { ParameterName = "@IsMobile", SqlDbType = SqlDbType.Bit, Value = IsMobile },
            new SqlParameter { ParameterName = "@AppType", SqlDbType = SqlDbType.NVarChar, Value = AppType }
        };
    }
}

class Program
{
    internal static ResultStatus DownloadsLog(ISqlDbLog log)
    {
        return DownloadsLog(log.GetSqlProcedureName(), log.GetSqlParameters());
    }

    static void Main(string[] args)
    {
        var log = new UserLog
        {
            CustomerId = "Test Customer",
            DeviceId = 5,
            IsMobile = true
        };

        var result = DownloadsLog(log);

    }
}

the advantage of this approach is that you have defined the data mapping within the model, which would be easier to navigate. The other thing, you're also defining the correct SqlDbType mapping. So, instead of using AddWithValue, you will pass the correct datatype, and it's already defined in the model. You can make a generic method where you use Reflection to auto-generate the SqlParameters, all you have to do is to change GetSqlParameters() to :

public SqlParameter[] GetSqlParameters()
{
    return this.GetType()
        .GetProperties()
        .Where(x => x.GetValue(this, null) != null)
        .Select(x => new SqlParameter($"@{x.Name}", x.GetValue(this, null)))
        .ToArray();
}

you can use it as an extension, if you have a wider usage (like applying it to other models as well). like this :

public static class SqlDbExtensions
{
    public static SqlParameter[] ToSqlParameterArray<T>(this T model) where T : ISqlDbLog
    {
        return model.GetType()
            .GetProperties()
            .Where(x => x.GetValue(model, null) != null)
            .Select(x => new SqlParameter($"@{x.Name}", x.GetValue(model, null)))
            .ToArray();

    }
}

This will be applied on all classes that implement ISqlDbLog. This is an insurance. So, with this, you will not need the GetSqlParameter() method in each class nor the ISqlDbLog interface. but you still need ProcedureName.

I hope this would add more helpful thoughts to your work.


Optimization of code for searching in db This is my code:

var test = (from x in myDb.myTable
            where (x.name == tmp || x.name == tmp2 || x.name == tmp3) && x.unit == u
            select x).FirstOrDefault();

if (test == null)
    test = (from x in myDb.myTable
            where (x.name == tmp || x.name == tmp2 || x.name == tmp3)
            select x).FirstOrDefault();

How to optimize it>?

var test = (from x in myDb.myTable
            where (x.name == tmp || x.name == tmp2 || x.name == tmp3)
            select x)
           .AsEnumerable()
           .OrderBy(x => x.unit == u ? 0 : 1)
           .FirstOrDefault();

I removed the x.unit == u condition. Instead I sort the items to make the ones where
this condition would be met to appear first. FirstOrDefault then makes the rest.

I split the EF part from the LINQ-to-objects part with AsEnumerable() as I am not sure if EF can translate the order by to SQL. If it can, you can try this

var test = (from x in myDb.myTable
            where (x.name == tmp || x.name == tmp2 || x.name == tmp3)
            orderby x.unit == u ? 0 : 1
            select x)
           .FirstOrDefault();


Iterate from "0" to "ZZZZZ" I was given an assignment, it was as follows:

Give a JS code snippet which writes to the console from:
"0" to "ZZZZZ" all the combination of the characters of a-z, 0-9 and A-Z.

Is my solution correct?
I am sure there are better ways to achieve the same, can anybody show some?
My code is:

function createCharSet(){
    var charset = "", c;

    for(c=48;c<58;c++){
        charset += String.fromCharCode(c);
    }
    for(c=97;c<123;c++){
        charset += String.fromCharCode(c);
    }
    for(c=65;c<91;c++){
        charset += String.fromCharCode(c);
    }

    return charset;

}

function getNext(curr, charSet){
    var
        length = curr.length,
        arr = curr.split(""),
        radix = charSet.length,
        i, c;

    for(i=length-1;i>-1;i--){
        c = charSet.indexOf(arr[i])+1;

        if(c==radix){
            arr[i]=charSet[0];
            if(i==0)
                return;
        }else{
            arr[i]=charSet[c];
            break;
        }
    }

    return arr.join("");
}

function iterate() {
    var
        charSet = createCharSet(),
        maxLen = 4,
        len,
        i,
        arr,
        curr;

    for(len = 1;len<maxLen+1;len++){
        arr = [];
        for(i=0;i<len;i++){
            arr.push(charSet[0]);
        }

        curr = arr.join("");
        while(curr) {
            console.log(curr);
            curr = getNext(curr, charSet);
        };
    }

}

iterate();

Minor observations:

I'd just hardcode the charset in this case. Doing so is faster to write (and a lot more explicit) than building it with a bunch of loops.
The modulo operator % is a good friend to have for this, as it'll let you "wrap around" array indices.

Overall, there's too much code here, I think. This is just counting in base 62.

It'd be awesome if Number.prototype.toString() accepted 62 as an argument, but, unfortunately, it only seems to accept bases 2-36. Similarly, it'd be great if we could somehow use the built-in base64 encoding function (btoa), but it's, well, base 64 (and it's got other issues). So we do have to roll our own.

Now, if this was base 10, we could obviously just do

var limit = Math.pow(10, 5); // i.e. 100000

for(var i = 0 ; i < limit ; i++) {
  console.log(i);
}

which will print 0-99999.

Using this same basic setup, we can print our base62 values. Our limit will be \$base^{numberOfDigits}\$ just like \$10^5\$ will end up printing five 9s. So for 0-ZZZZZ we'll need to loop from zero to Math.pow(62, 5).

And of course, we'll have to do the actual base62 conversion. The basic algorithm is:

Get the char corresponding to \$value\bmod base\$ (e.g. charset[538 % 62] => charset[42]) and add it to the output string.
Set \$value\$ to the quotient of \$\frac{value}{base}\$ (e.g. Math.floor(538 / 62) => 8).
If value is bigger than zero, go to step 1. Otherwise, return the output.

In code:

var output = "";
do {
  output = charset[value % base] + output;
  value = (value / base) | 0; // bitwise floor trick
} while(value > 0);
return output;

Putting it all together, I get this:

// define our base62 function (using an IIFE)
var toBase62 = (function () {
  var charset = "0123456789abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ",
      base = charset.length; // 62, natch

  return function (value) {
    // TODO: It'd be nice to guard against negative values here (see comments)
    var string = "";
    do {
      string = charset[value % base] + string;
      value = (value / base) | 0;
    } while(value > 0);
    return string;
  }
}());

// loop for a long, long time
for(var i = 0, l = Math.pow(62, 5) ; i < l ; i++) {
  console.log( toBase62(i) );
}

Note that it takes forever to print all the 916,132,832 values from 0 up to \$62^5\$. JS ain't that fast. I haven't explored optimization, so for testing, I'd advice using (much) fewer iterations.


Four jQuery date time pickers I'm using the Jquery date time picker link here

In the four methods below I am initializing the DT pickers but they are all pretty similar except for the ID in the onChangeDateTime.

Is there a way I can reduce the duplicate code?

$('.jqDatetimeDateOne').datetimepicker({
    defaultTime: '07:00',
    lang: 'en',
    className: 'setTimeClicker',
    step: 30,
    onChangeDateTime: function (current_time, $input) {
        // alert(ct.dateFormat('d/m/Y'))
        $('#emailGenDateOne').removeClass('empty').val(current_time.dateFormat('Y-m-d H:i'));
    },
    inline: true
});

$('.jqDatetimeDateTwo').datetimepicker({
    defaultTime: '07:00',
    lang: 'en',
    className: 'setTimeClicker',
    step: 30,
    onChangeDateTime: function (current_time, $input) {
        // alert(ct.dateFormat('d/m/Y'))
        $('#emailGenDateTwo').removeClass('empty').val(current_time.dateFormat('Y-m-d H:i'));
    },
    inline: true
});

$('.jqDatetimeSearchDateOne').datetimepicker({
    defaultTime: '07:00',
    lang: 'en',
    className: 'setTimeClicker',
    step: 30,
    onChangeDateTime: function (current_time, $input) {
        // alert(ct.dateFormat('d/m/Y'))
        $('#searchGenDateOne').removeClass('empty').val(current_time.dateFormat('Y-m-d H:i'));
    },
    inline: true
});

$('.jqDatetimeSearchDateTwo').datetimepicker({
    defaultTime: '07:00',
    lang: 'en',
    className: 'setTimeClicker',
    step: 30,
    onChangeDateTime: function (current_time, $input) {
        // alert(ct.dateFormat('d/m/Y'))
        $('#searchGenDateTwo').removeClass('empty').val(current_time.dateFormat('Y-m-d H:i'));
    },
    inline: true
});

So:

function configureDateTimePicker(selector1,selector2)
{
    $(selector1).datetimepicker(
    {
        defaultTime: '07:00',
        lang: 'en',
        className: 'setTimeClicker',
        step: 30,
        onChangeDateTime: function (current_time, $input)
        {
            // alert(ct.dateFormat('d/m/Y'))
            $(selector2).removeClass('empty').val(current_time.dateFormat('Y-m-d H:i'));
        },
        inline: true
    });
}

configureDateTimePicker('.jqDatetimeDateOne','#emailGenDateOne');
configureDateTimePicker('.jqDatetimeDateTwo','#emailGenDateTwo');
configureDateTimePicker('.jqDatetimeSearchDateOne','#searchGenDateOne');
configureDateTimePicker('.jqDatetimeSearchDateTwo','#searchGenDateTwo');

There could be further optimisations if you changed your classes/ids so that one could be derived from the other or from a common string (e.g. if they were all .jqDatetimeSomething and #genSomething). Or if there's a way to find the one of the targets through their relationship in the DOM (e.g. one is a descendant of the other matching a given selector).


Least suprising extension methods for DateTime to NATO date-time group I have the following code to get a NATO date-time group for a DateTime. I usually need Zulu-Time or from a specific time zone. Is it clear what those methods are providing? If want to parse a NATO dtg string would those be extension methods for string or DateTime?

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading.Tasks;
using System.Collections.ObjectModel;
using System.Globalization;

namespace scratchpad
{
/// <summary>
/// Create NATO date-time group from DateTime
/// </summary>
static class NatoDtg
{
    /// <summary>
    /// Get NATO date-time group for UTC/Zulu timezone
    /// </summary>
    /// <param name="dt">this</param>
    /// <returns>NATO date-time group</returns>
    public static string ToZuluDtg(this DateTime dt)
    {
        DateTime utc = dt.ToUniversalTime();
        CultureInfo ci = new CultureInfo("en-GB");

        StringBuilder builder = new StringBuilder();
        builder.Append(utc.ToString("ddHHmm", ci));             // day, hours, minutes
        builder.Append("Z");                                    // time zone
        builder.Append(utc.ToString("MMMyy", ci).ToLower());    // month, year
        return builder.ToString();
    }

    /// <summary>
    ///
    /// </summary>
    /// <param name="dt">this</param>
    /// <param name="tz">Time zone for the </param>
    /// <returns></returns>
    public static string ToDtg(this DateTime dt, TimeZoneInfo tz)
    {
        if (tz == null) throw new ArgumentNullException("TimeZoneInfo is null");
        CultureInfo ci = new CultureInfo("en-GB");
        StringBuilder builder = new StringBuilder();
        builder.Append(dt.ToString("ddHHmm", ci));

        TimeSpan ts = tz.GetUtcOffset(dt);

        char letter = '#';
        if(ts.Hours == 0)
        {
            letter = 'Z';
        }
        else if(ts.Hours >= 1 && ts.Hours <= 9) // A-I
        {
            letter = 'A';
            letter += (char)(ts.Hours-1);
        }
        else if(ts.Hours >= 10 && ts.Hours <= 12) // K-M TRICKY: skip J (Juliet)
        {
            letter = 'A';
            letter += (char)ts.Hours;
        }
        else if(ts.Hours >= -12 && ts.Hours <= -1) // N-Y
        {
            letter = 'Z';
            letter += (char)ts.Hours;
        }
        else
        {
            throw new InvalidOperationException("Unknown UTC offset for timezone");
        }

        builder.Append(letter);
        builder.Append(dt.ToString("MMMyy", ci).ToLower());
        return builder.ToString();
    }
}

class Program
{
    static void Main(string[] args)
    {

        DateTime now = DateTime.Now;

        Console.WriteLine("Zulu   DTG {0}", now.ToZuluDtg());
        Console.WriteLine("Berlin DTG {0}", now.ToDtg(TimeZoneInfo.GetSystemTimeZones().Where(i => i.DisplayName.Contains("Berlin")).FirstOrDefault()));
        Console.WriteLine("Local  DTG {0}", now.ToDtg(TimeZoneInfo.Local));
        Console.ReadLine();
    }
}
}

Firstly, I don't like the acronyms. With intellisense you can hit tab pretty early on to finish your names for you, so there's no sense adding an extra cognitive leap to work out what these functions do (even with your fairly comprehensive commenting). Call them ToDateTimeGroup and ToZuluDateTimeGroup or, even better, ToNatoDateTimeGroup and ToNatoZuluDateTimeGroup.

Speaking of commenting, your meta comments for the second method are unfinished. Additionally your commenting in-method is only commenting obvious code, which is unhelpful. Comments should explain why, not what or how, unless it's really hard to work it out (in which case, it probably needs refactoring). Additionally, comments on a new line above the code in question are generally more readable.

Your parameter names would also be better written out fully, e.g. dateTime instead of dt.

Your "Z" code is a magic string and should be a constant somewhere. It is arguable that your format strings are too.

I seem to say this a lot, but prefer to use var when the right hand side of a declaration makes the type obvious. Doing so means that should you need to change a variable's type, you only have to edit the declaration once.

I like to extend this rule to also include when the variable name makes the type obvious too, but many disagree, so your mileage may vary.

Resulting code looks like this:

/// <summary>
/// Extension methods to create NATO date-time groups from DateTime instances.
/// </summary>
static class NatoDtg
{

    private const char ZuluTimeZoneCode = `Z`;

    /// <summary>
    /// Get NATO date-time group for UTC/Zulu timezone
    /// </summary>
    /// <param name="dt">this</param>
    /// <returns>NATO date-time group</returns>
    public static string ToNatoZuluDateTimeGroup(this DateTime dateTime)
    {
        var utcTime = dateTime.ToUniversalTime();
        var culture = new CultureInfo("en-GB");

        var builder = new StringBuilder();
        builder.Append(utcTime.ToString("ddHHmm", culture));
        builder.Append(ZuluTimeZoneCode);
        builder.Append(utcTime.ToString("MMMyy", culture).ToLower());
        return builder.ToString();
    }

    /// <summary>
    /// Get NATO date-time group for a time zone
    /// </summary>
    /// <param name="dateTime">this</param>
    /// <param name="timeZone">Time zone for the date-time group </param>
    /// <returns></returns>
    public static string ToDtg(this DateTime dateTime, TimeZoneInfo timeZone)
    {
        if (timeZone == null) throw new ArgumentNullException("TimeZoneInfo is null");
        var culture = new CultureInfo("en-GB");
        var builder = new StringBuilder();
        builder.Append(dt.ToString("ddHHmm", culture));

        TimeSpan time = timeZone.GetUtcOffset(dateTime);

        char letter = '#';
        if(ts.Hours == 0)
        {
            letter = ZuluTimeZoneCode;
        }
        else if(ts.Hours >= 1 && ts.Hours <= 9)
        {
            //Letters A-I are based on hours
            letter = 'A';
            letter += (char)(ts.Hours-1);
        }
        else if(ts.Hours >= 10 && ts.Hours <= 12)
        {
            //Letters K-M are based on hours, offset because we skip J
            letter = 'A';
            letter += (char)ts.Hours;
        }
        else if(ts.Hours >= -12 && ts.Hours <= -1) // N-Y
        {
            //Letters N-Y are based on hours backwards.
            letter = 'Z';
            letter += (char)ts.Hours;
        }
        else
        {
            throw new InvalidOperationException("Unknown UTC offset for timezone");
        }

        builder.Append(letter);
        builder.Append(dt.ToString("MMMyy", culture).ToLower());
        return builder.ToString();
    }
}

class Program
{
    static void Main(string[] args)
    {

        var now = DateTime.Now;

        Console.WriteLine("Zulu   DTG {0}", now.ToZuluDtg());
        Console.WriteLine("Berlin DTG {0}", now.ToDtg(TimeZoneInfo.GetSystemTimeZones().Where(i => i.DisplayName.Contains("Berlin")).FirstOrDefault()));
        Console.WriteLine("Local  DTG {0}", now.ToDtg(TimeZoneInfo.Local));
        Console.ReadLine();
    }
}


Method to return statistics from a collection with O(1) time and space I have a DelayQueue, and the goal is to collect statistical information from the collection in O(1) time and space from getStatistics(). The stream for loop does not provide it. However I thought of keeping statistics updated on add of a transaction. But this will not guarantee that at the time when the getStatistics() called the queue will contain only unexpired item, and therefore I would need to iterate anyway. Or maybe the whole architecture here is not correct. I had also other thoughts, like having background task, with waiting time of the difference of the oldest transaction to drain the queue, and running it when that time expires, and update that watch to run it again when the other oldest transaction is approaching in time.

public class TransactionManager {

    private DelayQueue<Transaction> transactions;

    public TransactionManager() {
        transactions = new DelayQueue<>();
    }

    /**
     * Create a transaction. If the json string is not valid, return {@link HttpStatus} UNPROCESSABLE_ENTITY
     * If a transaction was created more than 60 seconds ago, return {@link HttpStatus} NO_CONTENT
     * Otherwise add this transaction to the queue
     *
     * @param json {@link String} json object containing amount (type of {@link Double}),
     *             and timestamp (type of {@link Long})
     */
    public Object createTransaction(String json) {
        JSONObject jsonObject = null;
        try {
            jsonObject = JsonValidator.validateTransactionJson(json);
        } catch (Exception ex) {
            return new ResponseEntity(HttpStatus.UNPROCESSABLE_ENTITY);
        }
        long delay = System.currentTimeMillis() - ((Long) jsonObject.get(TIMESTAMP));
        if (delay > ONEMINUTE) {
            return new ResponseEntity(HttpStatus.NO_CONTENT);
        }
        transactions.add(new Transaction((Double) jsonObject.get(AMOUNT), (Long) jsonObject.get(TIMESTAMP)));
        return new ResponseEntity(HttpStatus.OK);
    }

    /**
     * Return statistics of transaction for the last 60 seconds.
     *
     * @return {@link String} in a JSON format
     */
    public String getStatistics() {
        //drain to a list
        List<Transaction> tempForCleaning = new ArrayList<>();
        transactions.drainTo(tempForCleaning);
        //clear it
        tempForCleaning.clear();

        Statistic statistics = new Statistic();
        transactions.forEach(transaction -> {
            statistics.setCount(statistics.getCount() + 1);
            statistics.setSum(statistics.getSum() + transaction.getAmount());
            statistics.setMax(statistics.getMax() > transaction.getAmount() ? statistics.getMax() : transaction.getAmount());
            statistics.setMin(statistics.getMin() < transaction.getAmount() ? statistics.getMin() : transaction.getAmount());
        });
        if (statistics.getCount() > 0) {
            statistics.setAvg(statistics.getSum() / statistics.getCount());
        }
        return statistics.toJSON();
    }
}

Update:

Statistic.class with getters and setters.

public class Statistic {
private double sum = 0.0;
private double avg = 0.0;
private double max = 0.0;
private double min = 0.0;
private int count = 0;}

Just in case transaction as well:

public class Transaction implements Delayed {
    private final Double amount;
    private final Long timestamp;   //timestamp of a time when the item was created and send here

    public Transaction(double amount, long timestamp) {
        this.amount = amount;
        this.timestamp = timestamp;
    }

    @Override
    public long getDelay(TimeUnit unit) {
        return unit.convert(ONEMINUTE - (System.currentTimeMillis() - timestamp), MILLISECONDS);
    }

    @Override
    public int compareTo(Delayed delayed) {
        return delayed == this ?
                0 :
                Long.compare(getDelay(MILLISECONDS), delayed.getDelay(MILLISECONDS));
    }

    public double getAmount() {
        return amount;
    }
}

Initializing in the declaration

    private DelayQueue<Transaction> transactions;

    public TransactionManager() {
        transactions = new DelayQueue<>();
    }

You could replace this entire block with just

    private BlockingQueue<Transaction> transactions = new DelayQueue<>();

This has the identical effect.  No big advantage, just less code.

Using the interface rather than the implementation is good practice, as it makes the code more flexible.

Statistics

You have

        Statistic statistics = new Statistic();
        transactions.forEach(transaction -> {
            statistics.setCount(statistics.getCount() + 1);
            statistics.setSum(statistics.getSum() + transaction.getAmount());
            statistics.setMax(statistics.getMax() > transaction.getAmount() ? statistics.getMax() : transaction.getAmount());
            statistics.setMin(statistics.getMin() < transaction.getAmount() ? statistics.getMin() : transaction.getAmount());
        });
        if (statistics.getCount() > 0) {
            statistics.setAvg(statistics.getSum() / statistics.getCount());
        }

And

  Statistic.class with getters and setters.

public class Statistic {
private double sum = 0.0;
private double avg = 0.0;
private double max = 0.0;
private double min = 0.0;
private int count = 0;}

Consider getting rid of the setters.  Instead, move that block from the getStatistics method into an addDatum method in the Statistics class.

public class Statistics {

    private double sum = 0.0;
    private double maximum = 0.0;
    private double minimum = 0.0;
    private int count = 0;

    public void addDatum(double amount) {
        count++;
        sum += amount;

        if (amount > maximum) {
            maximum = amount;
        } else if (amount < minimum) {
            minimum = amount;
        }
    }

    public double calculateAverage() {
        return (count > 0) ? (sum / count) : 0.0;
    }

}

This way the Statistics class knows how to do calculations.  In the original, it was only a dumb container.  So you had to get the value, update it, and then set to the result.

I also fixed the indentation and spacing of the class.

I updated the name.  Average is a statistic.  This is a collection of statistics.

I moved the average calculation into a replacement for getAvg.  It calculates it on the fly.  You could memoize the result if you wanted, but you'd have to invalidate it with each addition.  Or you could recalculate the average each time you add something.

Updating as you go

You are considering making changes to update as you go.  This is still consistent with that.

You may also want to consider how you use this.  You say

     * Return statistics of transaction for the last 60 seconds.

But there are easier ways to do that than to use a DelayQueue.  Rather than use a sorted queue, use buckets.  E.g. with a HashMap<MinuteTime, Transactions>:

buckets.get(timeToTheMinute).add(transaction);

Then you can calculate the statistics in the add method.  And you don't have to worry about expirations to separate one period from another.


VC++ Fast Http Download I am a C# developer, dont know much about vc++ or c++, never used it, for some reasons i have decided to use a c++ dll in my app for downloading content from the web.

I dont want to use WebClient.

I want it to download html content by providing Url of the resource, the dll will return the string response.

this is the code i have till now, source

#include <iostream>
#include <string>

#include <stdlib.h>
#include <winsock.h>//dont forget to add wsock32.lib to linker dependencies

using namespace std;

#define BUFFERSIZE 1024
void die_with_error(char *errorMessage);
void die_with_wserror(char *errorMessage);

int main(int argc, char *argv[])
{
    string request;
    string response;
    int resp_leng;

    char buffer[BUFFERSIZE];
    struct sockaddr_in serveraddr;
    int sock;

    WSADATA wsaData;
    char *ipaddress = "208.109.181.178";
    int port = 80;

    request+="GET /test.html HTTP/1.0\r\n";
    request+="Host: www.zedwood.com\r\n";
    request+="\r\n";

    //init winsock
    if (WSAStartup(MAKEWORD(2, 0), &wsaData) != 0)
        die_with_wserror("WSAStartup() failed");

    //open socket
    if ((sock = socket(PF_INET, SOCK_STREAM, IPPROTO_TCP)) < 0)
        die_with_wserror("socket() failed");

    //connect
    memset(&serveraddr, 0, sizeof(serveraddr));
    serveraddr.sin_family      = AF_INET;
    serveraddr.sin_addr.s_addr = inet_addr(ipaddress);
    serveraddr.sin_port        = htons((unsigned short) port);
    if (connect(sock, (struct sockaddr *) &serveraddr, sizeof(serveraddr)) < 0)
        die_with_wserror("connect() failed");

    //send request
    if (send(sock, request.c_str(), request.length(), 0) != request.length())
        die_with_wserror("send() sent a different number of bytes than expected");

    //get response
    response = "";
    resp_leng= BUFFERSIZE;
    while (resp_leng == BUFFERSIZE)
    {
        resp_leng= recv(sock, (char*)&buffer, BUFFERSIZE, 0);
        if (resp_leng>0)
            response+= string(buffer).substr(0,resp_leng);
        //note: download lag is not handled in this code
    }

    //display response
    cout << response << endl;

    //disconnect
    closesocket(sock);

    //cleanup
    WSACleanup();
    return 0;
}

void die_with_error(char *errorMessage)
{
    cerr << errorMessage << endl;
    exit(1);
}

void die_with_wserror(char *errorMessage)
{
    cerr << errorMessage << ": " << WSAGetLastError() << endl;
    exit(1);
}

The instructions with the code says, code will end an http download 'early' if the download is anything slower than instant.

Please suggest better code, or modifications, which can be perfect for use in a Fast Crawler.

The above implementation has little to no chance of working. The HTTP protocol is quite complex, and none of that complexity is contemplated.

My advice is, don't waste time and energy in this. Use a high-level library.

Also, using low-level code will NOT improve the performance of fetching a document via HTTP -- even if your implementation was blazing-fast 100% polished assembly code, you'd still have to wait centuries (from the CPU point of view) for the data to arrive from the network.


Arrange three integers from least to greatest I'm a C++ beginner and I was told to make a program to arrange three integers from least to greatest. How did I do?

#include <iostream>

using namespace std;

int main()
{
    int x = 0;
    int y = 0;
    int z = 0;
    int first = 0;
    int second = 0;
    int third = 0;
    cout<<"Enter three integers to be arranged from least to greatest.\n";
    cin>>x>>y>>z;

    if(x <= y)
    {
        if(x <= z)
        {
            first = x;
            if(y <= z)
            {
                second = y;
                third = z;
            }
            else
            {
                third = y;
                second = z;
            }
        }
        else
        {
            second = x;
            if(y <= z)
            {
                first = y;
                third = z;
            }
            else
            {
                third = y;
                first = z;
            }
        }
    }
    else
    {
        if(x <= z)
        {
            second = x;
            if (y <= z)
            {
                first = y;
                third = z;
            }
        }
        else
        {
            third = x;
            if ( y <= z)
            {
                first = y;
                second = z;
            }
            else
            {
                second = y;
                first = z;
            }
        }
    }
    cout<<first<<second<<third;
}

Input/output

You need to output a space between the numbers, otherwise they will look like one long number.  It's also customary to finish the output with a newline.

You should verify that you actually received three inputs successfully.

Lint

using namespace std; is considered a bad habit.

Your code would be more readable if you put some spaces with your operators.  For example, cin>>x>>y>>z; would look better as std::cin >> x >> y >> z;

Only initialize what you need to initialize.  For example, int x, y, z; is enough — in fact it's safer, because the compiler can then warn you if you subsequently use one of those variables without assigning a value.

Strategy

You've basically manually written out all the possible comparisons that a sorting algorithm would do, the long way.  I suggest the following short solution (which doesn't generalize beyond three numbers).

#include <algorithm>
#include <iostream>

int main()
{
    int x, y, z;
    std::cout << "Enter three integers to be arranged from least to greatest.\n";
    if (!(std::cin >> x >> y >> z))
    {
        std::cerr << "Error in input.\n";
        return 1;
    }

    int first = std::min({x, y, z});
    int third = std::max({x, y, z});

    // Toss x, y, z into a bag, and take out the min and max.
    // What's left is the middle number.
    int second = x ^ y ^ z ^ first ^ third;

    std::cout << first << ' ' << second << ' ' << third << std::endl;
}

Note that std::min({x, y, z}) uses C++11.  For older compilers, you can use std::min(x, std::min(y, z)).