Code to find the sum of the product of all size-n combinations of an array The title is a bit of a mouthful, so here's a quick example:

You've got an array, maybe just the digits 1-3, and you want to get the sum of product of all size-2 combinations of the array. Those combinations are (1,2), (1,3), and (2,3), so the answer I'm looking for is this:

$$1\cdot2+1\cdot3+2\cdot3=11$$

Except now, the array is about 3-4 orders of magnitude in size, and the problem is kinda intractable beyond size-2 combinations.

Here's the code I have made to do the task:

import numpy as np
from itertools import combinations as comb

#A is the target array, in this case, a random array of 1000 elements.
#B is a iterator of length-2 tuples which should act as indices for reading A.

n = 2
A = np.random.rand(1000)
B = comb(range(1000),n)

print np.sum(np.array([np.prod(A[i,]) for i in B]))

The code is fairly condensed, but is kinda simple.  It first makes an iterator object containing all the unique combinations of the indices of A, and then uses those tuples in an (ab)use of numpy's array slicing notation to get reference those indices, find the product there, and then sum it all together.

The greatest weakness of this code, I would guess, is the list comprehension, since it doesn't make use of numpy functions.  As it stands, the code runs fast enough for size-2 combinations, but falls flat for anything higher.  I would like it to at least be tractable for size-3 combinations, but as it stands, this code doesn't make the cut.

Any suggestions or things to look into would be greatly appreciated.

It first makes an iterator object containing all the unique combinations of the indices of A

Brute force is the first step in "Make it work; then make it fast". Since it's the bottleneck, you need to take the second step: exploit the structure of the problem. Here there's plenty of algebraic structure to exploit (as long as numerical stability isn't a major issue).

In particular, think about divide and conquer. Rather than compute all \$\binom{S}{2} = \Theta(S^2)\$ pairs separately, take a running total and multiply each element by each of the previous elements simultaneously:

result, running_total = 0, 0
for a in A:
    result += running_total * a
    running_total += a

and you have \$O(S)\$ time. This can be extended for larger values of \$n\$ to work in \$O(Sn)\$ time and \$O(n)\$ space.


Random permutation with defined order of last occurrences This Python function takes two arguments:

chr_list which is a mutable sequence of characters c
chr_counts which maps to each character c the number of occurrences n

For each character c in chr_list, it inserts this character into the list before its last occurrence n-1 times, where n equals chr_counts[c]. The resulting list contains chr_counts[c] occurrences of each character c, and the order of last occurrences is preserved.

def insert_repetitions_before(chr_list, chr_counts):

  for c, num_occurences in chr_counts.items():
    num_repeats = num_occurences - 1
    orig_pos_last = chr_list.index(c)

    for curr_pos_last in range(orig_pos_last, orig_pos_last+num_repeats):
      random_pos_before_last = random.randint(0, curr_pos_last)
      chr_list.insert(random_pos_before_last, c)

Example output:

chr_list = ['c','o','d','e']
chr_counts = {'c': 3, 'o': 1, 'd': 2, 'e': 2}
>>> insert_repetitions_before(chr_list, chr_counts)
>>> chr_list
['c', 'd', 'c', 'c', 'e', 'o', 'd', 'e']

I want to make it more efficient. Currently its time complexity is O(n*m), n being the number of distinct characters and m the average number repetitions. Perhaps something like random.shuffle on a list containing all characters with repetitions, followed by some reordering?

(I'm not sure whether this is the right site to ask, since this question isn't quite specific to Python. Maybe I'll move it to CS.)

I want to make it more efficient. Currently its time complexity is O(n*m), n being the number of distinct characters and m the average number repetitions.

There are two important errors there. Firstly, the time complexity of the given code is worse than stated. Secondly, the stated complexity is optimal.

Taking those in reverse order: the stated complexity is optimal because the number of items which must be inserted into chr_list is \$\Theta(mn)\$, and no matter how you arrange things each insertion will take \$\Omega(1)\$ time.

However, the given code does insertions at random, and an insertion into a random position in a list takes \$\Theta(\textrm{len})\$ time, so the overall complexity of the given code should be \$\Theta(m^2 n^2)\$.

How to achieve optimal asymptotic performance? You need to arrange for the insertions to be done in order. I think that probably requires some additional datastructures. The simplest approach conceptually is to have an array (or a list in Python terms) to store the characters in their final positions and a datastructure of unassigned indexes which allows fast selection of the maximum element and a random element. In Pythonesque pseudocode:

tmpList = [None] * sum(chr_counts)
unassigned = new Datastructure(sum(chr_counts))
for c in reverse(chr_list):
  tmpList[unassigned.removeMax()] = c
  for i in 1 to chr_counts[c] - 1:
    tmpList[unassigned.removeRandom()] = c
copy tmpList to chr_list

The best implementation I've thought of so far for the datastructure of unassigned indexes combines a linked list for fast implementation of max with an array for fast implementation of random. I'm not familiar with Python's OO syntax, so this is again pseudocode:

class LinkedListNode:
  LinkedListNode prev
  LinkedListNode next
  int arrayIdx
  int value

class Datastructure:
  LinkedListNode[] array
  LinkedListNode sentinel = new LinkedListNode

  init(len):
    array = []
    for i in range(len):
      array[i] = new LinkedListNode { arrayIdx = i, value = i }
    for i in range(len-1):
      array[i].next = array[i+1]
      array[i+1].prev = array[i-1]
    sentinel.next = array[0]
    sentinel.prev = array[len-1]
    sentinel.next.prev = sentinel
    sentinel.prev.next = sentinel

  removeMax():
    return removeNode(sentinel.prev)

  removeRandom(rnd):
    randIdx = rnd.randint(0, len(array))
    return removeNode(array[randIdx])

  removeNode(node):
    # Remove from linked list
    node.next.prev = node.prev
    node.prev.next = node.next
    # Remove from array
    array[node.arrayIdx] = array[-1]
    array[node.arrayIdx].arrayIdx = node.arrayIdx
    del array[-1]

    return node.value

Note: I don't really think Datastructure is a good name, but it's not an easy class to name so I'm using it as a placeholder instead of Foo.


RxJS Observable with initial value Is it possible to get rid of the BehaviourSubject to have an Observable with an initial value? Another thing I don't like is that I'm using getValue() and then modify this value and push it with next back to the Observable. Maybe there is a more elegant way to get the same thing?

import { Injectable } from '@angular/core';

import { Observable } from 'rxjs/Observable';
import { BehaviorSubject } from 'rxjs/BehaviorSubject';

@Injectable()
export class UserProvider {

  users$: Observable<{ id: string; name: string }[]>;

  private usersSource: BehaviorSubject<{ id: string; name: string }[]>;

  constructor() {
   this.usersSource = new BehaviorSubject<any>([
     { id: '111', name: 'Jack' },
     { id: '222', name: 'John' },
     { id: '333', name: 'Foo' },
   ]);
   this.users$ = this.usersSource.asObservable();
  }

  addUser(user: { id: string; name: string }) {
    const currentUsers = this.usersSource.getValue();
    currentUsers.push(user);
    this.usersSource.next(currentUsers);
  }

  removeUser(userId: string) {
    const currentUsers = this.usersSource.getValue();
    const index = currentUsers.findIndex(user => user.id === userId);
    if (index !== -1) {
      if (this.selectedUserIdSource.getValue() === currentUsers[index].id) {
        this.selectedUserIdSource.next(null);
      }
      currentUsers.splice(index, 1);
      this.usersSource.next(currentUsers);
    }
  }
}

This is a really good question -- starred it! I am not very experienced with RxJs, but I believe your code around BehaviorSubject and Observable is as concise as possible.

BehaviourSubject itself is "subscribable" (just like an Observable) but it's a really bad practice to expose it directly to the consumer, i.e. your separation of users$ and usersSource is a right thing to do. Exposing Observable is okay because it's designed to be read-only.

usersSource.getValue() is inevitable in your code, unless you decide to keep the usersSource state as a local field, but IMO it does not do any good in this scenario. Having state to be stored as a part of the BehaviorSubject is handy.

There are minor style things that could be improved in the code, but that's unrelated to RxJs, so I'll not comment on that.

Update 1

Just a few months ago I was not aware of .startsWith() which seems to be an answer to a part of your question:

const neverButStartsWithZero = Rx.Observable
                                .never()
                                .startWith(0);
// Emits 0
const subscription1 = neverButStartsWithZero
                        .subscribe(value => console.log('subscription1:', value));

const threeEventsWithExtraZero = Rx.Observable
                                .of(1, 2, 3)
                                .startWith(0);
// Emits 0, 1, 2, 3
const subscription2 = threeEventsWithExtraZero
                        .subscribe(value => console.log('subscription2:', value));


Sorting and summation of a spreadsheet I am working on a sorted data file, so there does not need to be any sorting logic.  I find unique values, feed them into an array, then use the worksheetfunction.sum to get totals for a field.

Is this as efficient as I could be, or are there some things I'm not seeing and should?

The "Items" sheet is where a number of accounts are stored (in the thousands).  The user cuts and pastes the data from another spreadsheet  It is sorted by account, and each line all the columns are identical except for the totals, and the first coloumn which contains the account number.

I collect an array of account numbers, then cycle through the array to find the first instance, and continue down until I find the last instance.  Populate a second array with the contents of all cells in that row (up to the last column with a header, then get a sum of the totals, and overwrite that element of the array.

When completed, I drop the entire array made from the items sheet into the summary sheet.

Option Explicit

Sub makeSummary()

Dim inws As Worksheet
Dim outws As Worksheet

Dim fndRange As Range
Dim zell As Range

Dim firstRow As Integer
Dim lastRow As Integer
Dim colctr As Integer
Dim totalCol As Integer
Dim LastCol As Integer

Dim ctr As Long
Dim arrBound As Long

Dim distVals() As String
Dim newRows() As String

' Initialize variables
Set inws = Sheets("Items")
Set outws = Sheets("Summary_Sheet")
outws.Cells.Clear
Set fndRange = Range(GetLast(3, inws.Cells))
LastCol = fndRange.Column
lastRow = fndRange.Row
Set fndRange = Nothing

'populate the header columns in the output worksheet.
For ctr = 1 To LastCol
    outws.Cells(1, ctr) = inws.Cells(1, ctr).Value
Next ctr

' redim array, and populate with unique SFC values
ReDim distVals(1)
distVals(1) = inws.Cells(2, 1)
For ctr = 2 To lastRow
    If inws.Cells(ctr, 1).Value <> distVals(UBound(distVals)) Then
        ReDim Preserve distVals(UBound(distVals) + 1)
        distVals(UBound(distVals)) = inws.Cells(ctr, 1).Value
    End If
Next ctr

'Get upper bound of search array and use it to set max row value of newrows array
arrBound = UBound(distVals)
ReDim newRows(1 To arrBound, 1 To LastCol)

'build array,
For ctr = 1 To arrBound
    Set fndRange = inws.Cells.Find(distVals(ctr), lookat:=xlPart, LookIn:=xlFormulas)
    firstRow = fndRange.Row
    lastRow = fndRange.Row
    Do Until inws.Cells(lastRow + 1, 1) <> distVals(ctr)
        lastRow = lastRow + 1
    Loop
'fill row
    For colctr = 1 To LastCol
        newRows(ctr, colctr) = inws.Cells(firstRow, colctr)
    Next colctr
'Get total of totals in SFC
    newRows(ctr, 5) = WorksheetFunction.Sum(Range(inws.Cells(firstRow, 5), inws.Cells(lastRow, 5)))
Next ctr

'clean up of destination sheet
With outws
    .Columns("E").NumberFormat = "_($* #,##0.00_);_($* (#,##0.00);_($* ""-""??_);_(@_)"
    .Range(.Cells(2, 1).Address, .Cells(arrBound + 1, LastCol).Address) = newRows
    'excel doesn't recognize the numbers as numbers unless you multiply by 1 and drop the value back down.
    For Each zell In .Range(.Cells(2, 5).Address, .Cells(arrBound + 1, 5).Address)
        zell.Value = zell.Value * 1
    Next zell
    .Calculate
End With

End Sub

Function GetLast(choice As Long, rng As Range)
    ' 1 = GetLast row
    ' 2 = GetLast column
    ' 3 = GetLast cell

Dim ReturnRng As Range

Set ReturnRng = rng.Find(What:="*", After:=rng.Cells(1), lookat:=xlPart, LookIn:=xlFormulas, _
                SearchOrder:=xlByRows, SearchDirection:=xlPrevious, MatchCase:=False)

If Not ReturnRng Is Nothing Then
    With ReturnRng
        Select Case choice
            Case 1
                GetLast = .Row
            Case 2
                GetLast = .Column
            Case 3
                GetLast = .Address
            Case Else
        End Select
    End With
End If
End Function

Quick Things
First, it's good practice to indent all of your code that way Labels will stick out as obvious.
Second, Worksheets have a CodeName property - View Properties window (F4) and the (Name) field (the one at the top) can be used as the worksheet name. This way you can avoid Dim inws as Worksheet, set inws = Sheets"Items" and instead just use Items.
Comments - "code tell you how, comments tell you why". The code should speak for itself, if it needs a comment, it might need to be made more clear. If not, the comment should describe why you're doing something rather than how you're doing it. Here are a few reasons to avoid comments all together.
Integers - integers are obsolete. According to msdn VBA silently converts all integers to long.

Naming
Your variable names leave a lot to be desired. It's pretty easy to use descriptive names and characters are mostly free, so things like
ctr = columnCounter or rowCounter or even just counter.
Also, Standard VBA naming conventions have camelCase for local variables and PascalCase for other variables and names. So inws should be inWS or inWorksheet or sourceWorksheet.
Right now things like zell and distVals don't really tell me anything at all about what I should be expecting them to do.

Function
Function GetLast - it's your prerogative, but I think it's unneeded. If you did want you keep it, it would be better as
Private Function GetLast(ByVal choice as Long, ByVal rng as Range) as Long

Right now you're passing ByRef, which isn't best practice and you don't have a return type defined. You can return a row number, column number or address (string?). That can lead to errors down the line if you change what you're looking for and expect the wrong type back. Personally,
Dim lastRow As Long
lastRow = Sheet.Cells(Rows.Count, 1).End(xlUp).Row
Dim lastColumn As Long
lastColumn = Sheet.Cells(1, Columns.Count).End(xlToLeft).Column

Would work fine for me. And you could eliminate all of
Set fndRange = Range(GetLast(3, inws.Cells))
LastCol = fndRange.Column
lastRow = fndRange.Row
Set fndRange = Nothing

Arrays
Dim distVals() As String
Dim newRows() As String
ReDim distVals(1)
    distVals(1) = inws.Cells(2, 1)
    For ctr = 2 To lastRow
        If inws.Cells(ctr, 1).Value <> distVals(UBound(distVals)) Then
            ReDim Preserve distVals(UBound(distVals) + 1)
            distVals(UBound(distVals)) = inws.Cells(ctr, 1).Value
        End If
    Next ctr

    'Get upper bound of search array and use it to set max row value of newrows array
    arrBound = UBound(distVals)
    ReDim newRows(1 To arrBound, 1 To LastCol)

I think what's happening here is that you initialize the array, then populate it one cell at a time, assuming that your data is in some sort of order because you're checking only the last element for duplicates, right? That is a lot of redimming and isn't needed. A dictionary could help with that by eliminating duplicates for you
Dim valueDictionary As Object
Set valueDictionary = CreateObject("Scripting.Dictionary")
For counter = 2 To lastRow
    valueDictionary(Cells(counter, 2).Value) = 1
Next

Now arrBound = UBound(distVals) is just valueDictionary.Count
Now, your newRows array is a little confusing to me. Why not just build an array, maybe like this -
    Dim mySummedArray As Variant
    ReDim mySummedArray(1 To valueDictionary.Count, 1 To 2)
    ReDim arr(1 To valueDictionary.Count, 1 To 2)
    counter = 1
    Dim key As Variant
    For Each key In valueDictionary.keys
        mySummedArray(i, 1) = key
        i = i + 1
    Next
    For counter = 1 To valueDictionary.Count
        mySummedArray(counter, 1) = valueDictionary.keys(counter)
    Next
    Dim indexValue As String
    Dim arrayIndex As Long
    For counter = 1 To lastRow
        indexValue = Cells(counter, 1)
        arrayIndex = FindInArray(indexValue, mySummedArray)
        mySummedArray(arrayIndex, 2) = mySummedArray(arrayIndex, 2) + Cells(counter, 2)
    Next

Using
Private Function FindInArray(ByVal indexValue As String, ByVal arrayToSearch As Variant) As Long
    Dim i As Long
    For i = LBound(arrayToSearch) To UBound(arrayToSearch)
        If StrComp(indexValue, arrayToSearch(i, 1), vbTextCompare) = 0 Then
            FindInArray = i
            Exit For
        End If
    Next
End Function

Essentially you end up with a 2D array of unique values paired with their sum. But maybe that's not what you're doing? If not, sorry, but maybe the approach is still useful.


Touchless lightswitch with Raspberry Pi I am currently developing a touchless lightswitch via a Raspberry Pi, an ultrasonic module and a nice radio module.

However, I am quite a beginner in Python and it was a hard time (at least for me) to accomplish my goal. Whenever the measured distance is falling below the limit the switch is triggered "on" (1) and after that if the distance is falling short again the switch is triggered "off" (0).

#!/usr/bin/env python

import os
import sys
import time
import RPi.GPIO as GPIO

GPIOTrigger = 23
GPIOEcho    = 24

def wait(sec):
  while sec > 0:
    sys.stdout.write(str(sec) + '     \r')
    sec -= 1
    time.sleep(1)

def MeasureDistance():
  GPIO.output(GPIOTrigger, True)
  time.sleep(0.00001)
  GPIO.output(GPIOTrigger, False)
  StartTime = time.time()
  while GPIO.input(GPIOEcho) == 0:
    StartTime = time.time()
  while GPIO.input(GPIOEcho) == 1:
    StopTime = time.time()
  TimeElapsed = StopTime - StartTime
  Distance = (TimeElapsed * 34300) / 2
  return Distance
def main():
  STATUS = 0
  try:
    while True:
      Distance = MeasureDistance()
      if Distance > 10.0:
        time.sleep(0.01)
      else:
        if STATUS != 1:
      print("Passing by (%.1f cm)" % Distance)
          import subprocess
          subprocess.call(["/root/rcswitch-pi/send", "10000", "1", "1"])
          wait(5)
          STATUS = 1
        else:
          subprocess.call(["/root/rcswitch-pi/send", "10000", "1", "0"])
          wait(5)
          STATUS = 0

  except KeyboardInterrupt:
    print("Stopped by user")
    GPIO.cleanup()
if __name__ == '__main__':
  # use GPIO pin numbering convention
  GPIO.setmode(GPIO.BCM)
  GPIO.setup(GPIOTrigger, GPIO.OUT)
  GPIO.setup(GPIOEcho, GPIO.IN)
  GPIO.output(GPIOTrigger, False)
  main()

This works but I am unsure whether there is a nicer way of determining the last status of STATUS or if there are some pitfalls I might not see.

MeasureDistance. The way it is written, sometimes it measures the duration of the signal, sometimes the (desired) time for the echo to arrive (consider what happens if you enter the first while loop). Suggestion:

def MeasureDistance():
    StartTime = time.time()
    GPIO.output(GPIOTrigger, True)
    while GPIO.input(GPIOEcho) == 0:
        pass
    # Found the raising front
    StopTime = time.time()
    GPIO.output(GPIOTrigger, False)

Of course, if the echo arrives faster than the code executes (and Python is slow), you'd get the elapsed time 0. I suppose there's nothing you can do about that, other than resorting to interrupts. Also, you may want to put some safeguards (a timer maybe) in case the raising front never arrives.

main. I don't see the need to import subprocess in the middle of the code. Also, state variables tend to create an unmaintainable code. Suggestion:

    try:
        Distance = MeasureDistance()
        while True:
            while Distance > 10.0:
                Distance = MeasureDistance()
                wait(5)
            switch_on()
            while Distance < 10.0:
                Distance = MeasureDistance()
                wait(5)
            switch_off()
    except KeyboardInterrupt:

PS: a magic 34300 constant must go. Give it a nice symbolic name (yes we all know it has something to do with the speed of sound).

Update.
Usually in the situations like yours you may have a funny behaviour at the breaking distance. Due to random factors (like the code execution time, and many more others) the distance is measured with a certain error, and the switch will go on and off unpredictably. To deal with it, allow for a hysteresis:

            while Distance > breaking_distance - tolerance:
                Distance = MeasureDistance()
                wait(5)
            switch_on()
            while Distance < breaking_distance + tolerance:
                Distance = MeasureDistance()
                wait(5)
            switch_off()


Expressing Rebol Dates in EBNF I'm looking to define the Rebol date format in EBNF notation. I'd like as best as possible to only define valid dates—at least those that are valid in Rebol at the moment:

Date ::= DateDate ('/' Time DateZone?)?

DateDate ::=
      DateDay31 ('-' DateMonth31 '-' | '/' DateMonth31 '/') DateYear
    | DateDay30 ('-' DateMonth30 '-' | '/' DateMonth30 '/') DateYear
    | DateDay28 ('-' DateMonthFebruary '-' | '/' DateMonthFebruary '/') DateYear
    | "29" ('-' DateMonthFebruary '-' | '/' DateMonthFebruary '/') DateYearLeap
    | DateYear ('-' DateMonth31 '-' | '/' DateMonth31 '/') DateDay31
    | DateYear ('-' DateMonth30 '-' | '/' DateMonth30 '/') DateDay30
    | DateYearLeap ('-' DateMonthFebruary '-' | '/' DateMonthFebruary '/') DateDay29
    | DateYear ('-' DateMonthFebruary '-' | '/' DateMonthFebruary '/') DateDay28

/*
    Currently years cannot be negative and have a maximum value of 16383
    So the following two values are shortcuts.
*/
DateYear ::= Digit (Digit (Digit (Digit Digit?)?)?)?

DateYearLeap ::=
      '1' Digit Digit DateYearLeapEnd
    | Digit Digit DateYearLeapEnd
    | Digit DateYearLeapEnd
    | DateYearLeapEnd
    | [048]

DateYearLeapEnd ::= [02468] [048] | [13579] [26]

DateMonth ::= DateMonth31 | DateMonth30 | DateMonthFebruary

DateDay31 ::= '3' [01] | DateDay29

DateDay30 ::= '30' | DateDay29

DateDay29 ::= [12] Digit | '0'? [1-9]

DateDay28 ::= '2' [0-8] | '1' Digit | '0'? [1-9]

DateMonth31 ::=
    DateMonthJanuary |
    DateMonthMarch |
    DateMonthMay |
    DateMonthJuly |
    DateMonthAugust |
    DateMonthOctober |
    DateMonthDecember

DateMonth30 ::=
    DateMonthApril |
    DateMonthJune |
    DateMonthSeptember |
    DateMonthNovember

/* Currently only English month names are valid */
DateMonthJanuary ::= 'Jan' ('u' ('a' ('r' 'y'?)?)?)? | '0'? '1'

DateMonthFebruary ::= 'Feb' ('r' ('u' ('a' ('r' 'y'?)?)?)?)? | '0'? '2'

DateMonthMarch ::= 'Mar' ('c' 'h'?)? | '0'? '3'

DateMonthApril ::= 'Apr' ('i' 'l'?)? | '0'? '4'

DateMonthMay ::= 'May' | '0'? '5'

DateMonthJune ::= 'Jun' 'e'? | '0'? '6'

DateMonthJuly ::= 'Jul' 'y'? | '0'? '7'

DateMonthAugust ::= 'Aug' ('u' ('s' 't'?)?)? | '0'? '8'

DateMonthSeptember ::= 'Sep' ('t' ('e' ('m' ('b' ('e' 'r'?)?)?)?)?)? | '0'? '9'

DateMonthOctober ::= 'Oct' ('o' ('b' ('e' 'r'?)?)?)? | '10'

DateMonthNovember ::= 'Nov' ('e' ('m' ('b' ('e' 'r'?)?)?)?)? | '11'

DateMonthDecember ::= 'Dec' ('e' ('m' ('b' ('e' 'r'?)?)?)?)? | '12'

/* Zone Hours are currently -15 - 15, the following is a shortcut: */
DateZone ::= Sign Digit Digit? ':' ([03] '0' | [14] '5')

Time ::= TimeHour ':' TimeMinute (':' TimeSecond)?

/* Need to constrain to valid hours */
TimeHour ::= Sign Digit* | Sign? Digit+

/* Need to constrain to 0-59 */
TimeMinute ::= Sign? Digit Digit?

/* Need to constrain to 0-59.999999 */
TimeSecond ::= Sign? ((Digit Digit?)? '.' Digit+ | Digit Digit? '.'?)

Digit ::= [0-9]

Sign ::= [+-]

Notes:

Partial matches are bad: this rule should match a whole string—a partial match is failure, e.g. the following would only partially match 22 and would thus fail.
'2' | '22'
I've taken shortcuts on some values (see source comments), I would expect to flesh these out.
Although Rebol will interpret 12-010-2014 as a date (12-Oct-2014), I don't see any reason to support this.

To test this code in Rebol, save as a file and load as a parse rule to use against samples:

Rebol []
do http://reb4.me/x/ebnf.r
date-grammar: get in context load-ebnf %date.ebnf 'date

foreach test [
    "28-Feb-2016"
    "29-Feb-2000"
    "29-Feb-2011"
    "29-Feb-2010"
    "29-Feb-2016"
    "1-April-2015/12:00"
    "1-4-2015/12:00+5:00"
    "01-Apr-2015"
    "31-Apr-2015"
    "29-Feb-1900"
    "00-Feb-20"
    "15-Apr-16"
    "2015-04-01/12:15:10"
    "2015-04-01/12:15:10."
    "2015-04-01/12:15:."
    "2015-04-01/12:15:10.1234"
    "2015-04-01/12:15:10.1234-05:00"
    "2015-04-01/12:15:10.1234-0:00"
][
    print [either parse test date-grammar ["*"][" "] mold test date? try [load test]]
]

Is Janua really a valid month? If so, then you're fine, but I have a feeling that this would be more appropriate.

DateMonthJanuary ::= 'January' | 'Jan' | '0'? '1'

Which brings me to my next problem. It's case sensitive, right? So, 22-JAN-2015 wouldn't match. I believe it should, but I'm not familiar with Robol. Please correct me if I'm wrong.

I've seen this handled by defining case insensitive tokens for each letter.

A ::= 'a' | 'A'

And then define your rules from those tokens like this.

DateMonthJanuary ::= J A N U A R Y | J A N | '0'? '1'

My syntax may be a little off, I'm accustomed to ANTLR's flavor of EBNF, but that should illustrate the idea.

DateYearLeap ::=

I'm really not sure that I would handle that in your grammar. The logic is convoluted (not yours, just leap years in general) and I'm pretty sure this will match a few leap years that aren't. Much better for this one to be validated by your parser if that's possible.

/* Need to constrain to valid hours */
TimeHour ::= Sign Digit* | Sign? Digit+

/* Need to constrain to 0-59 */
TimeMinute ::= Sign? Digit Digit?

/* Need to constrain to 0-59.999999 */
TimeSecond ::= Sign? ((Digit Digit?)? '.' Digit+ | Digit Digit? '.'?)

I think you should be able to define tokens for these. It may take a bit of doing to get the precedence right, but start with something like this.

TimeMinute ::= Sign? [0-6] Digit
/* Optional Sign, Optional 10s Digit 0-6, 0-9*/


Adjacency List Graph representation on python I began to have my Graph Theory classes on university, and when it comes to representation, the adjacency matrix and adjacency list are the ones that we need to use for our homework and such. At the beginning I was using a dictionary as my adjacency list, storing things like this, for a  directed graph as example:

graph = {
    0: [1, 2, 4],
    1: [2, 3],
    2: [],
    3: [4],
    4: [0],
}

There was no problem, since the graphs I was dealing with had no weight in their edges, and if I wanted to represent an undirected graph, just had to "mirror" the edges. Now I'm facing a problem with the representation in adjacency list for weighted graphs, being directed or undirected.

So far, this is what I'm using:

class Graph:

    def __init__(self, vertices, is_undirected=True):
        self.__v = vertices  # number of vertices
        self.__edge_list = []  # store the edges and their weight
        self.__is_undirected = is_undirected # True for undirected graphs
        self.__adj_matrix = None # stores the adjacency matrix
        self.__adj_list = None # stores the adjacency list

    # method for adding an edge to the graph
    def add_edge(self, u, v, w=None):
        self.__edge_list.append([u, v, w if w else 1])
        # in case it is an undirected graph,
        # replicate edge in opposite way
        if self.__is_undirected:
            self.__edge_list.append([v, u, w if w else 1])

And this is the method for making my adjacency list using the __edge_list:

def make_adjacency_list(self):
        adj_list = {key: [] for key in range(self.__v)}
        for edge in self.__edge_list:
            # where edge[1] is the destiny and edge[2] the weight
            edge_val = {edge[1]: edge[2]}
            adj_list[edge[0]].append(edge_val)
        self.__adj_list = adj_list

Also, the graph will be generated from a file, formatted as such:

0 # 0 for directed graph
5 # number of vertices
0 2 4 # edge from u to v, with w weight
0 4 60
0 3 23
2 3 4
3 1 10
4 2 15

This results in the following:

0 = [{2: 4}, {4: 60}, {3: 23}]
1 = []
2 = [{3: 4}]
3 = [{1: 10}]
4 = [{2: 15}]

The problem with this is that is becomes very hard, at least for me, to recover the data for each edge from my adjacency list, so I was wondering if this the right way to do it, or if I can be more efficient in what I'm trying to do. My current algorithms for BFS(breadth first search), DFS( depth first search), Kruskal, Prim and Djikstra are having problems in this structure I made, but I can't see another way of doing it unless I move the adjacency list in a separate class.

Lastly, and code-improvement/advice in more pythonic ways of doing things would be welcome. I tried as best as I could to make this code clean, but now it is a mess(not proud of that). If there's need of any other bit of code or clarification I'll answer as best as I can. I tried going to the professor, but he's more of a Java person, so he couldn't help much.

Here are two ways you could represent a graph with weighted edges in Python:

Represent a graph as a mapping from a node \$n\$ to a mapping from neighbouring node \$m\$ to the weight \$w\$ of the edge from \$n\$ to \$m\$:

graph = {
    0: {2: 4, 4: 60, 3: 23},
    1: {},
    2: {3: 4},
    3: {1: 10},
    4: {2: 15},
}

Represent a graph as a pair of mappings: one from a node \$n\$ to a list of its neighbouring nodes, and the other from pairs of nodes \$n, m\$ to the weight \$w\$ of the edge from \$n\$ to \$m\$:

graph = {
    0: [2, 4, 3],
    1: [],
    2: [3],
    3: [1],
    4: [2],
}
weights = {
    (0, 2): 4,
    (0, 4): 60,
    (0, 3): 23,
    (2, 3): 4,
    (3, 1): 10,
    (4, 2): 15,
}

Either of these representations should work fine. Representation (2) would be good if you need to iterate over all the edges at some point in your algorithm. Representation (1) is the simplest and probably best if you have no special requirements.

Answers to comments

You don't even need .keys() — in Python, iterating over a dictionary yields its keys, so you can write:

for m in graph[n]:
    # m is a neighbour of n

Yes, defaultdict is a useful technique for building graphs. In representation (1) you'd start with:

graph = defaultdict(dict)

and then add an edge from \$n\$ to \$m\$ with weight \$w\$ by writing:

graph[n][m] = w

In representation (2) you'd start with:

graph = defaultdict(list)
edges = {}

and then add an edge from \$n\$ to \$m\$ with weight \$w\$ by writing:

graph[n].append(m)
edges[n, m] = w


Review URL-prettifying .htaccess file Can anyone please review the following .htaccess file and let me if it is correctly put together?

What I think it does:

So it direct to a custom 404 page
Remove the extensions from the end of the urls (example .php)
Remove /index
I don't really understand what the final rule does, can anyone please explain it if could?

DirectoryIndex index.html index.php
ErrorDocument 404 /404

RewriteEngine On
RewriteBase /

# Remove enter code here.php; use THE_REQUEST to prevent infinite loops
# By puting the L-flag here, the request gets redirected immediately
RewriteCond %{THE_REQUEST} ^GET\ (.*)\.php\ HTTP
RewriteRule (.*)\.php$ $1 [R=301,L]

# Remove /index
# By puting the L-flag here, the request gets redirected immediately
# The trailing slash is removed in a next request, so be efficient and dont put it on there at all
RewriteRule (.*)/index$ $1 [R=301,L]

# Remove slash if not directory
# By puting the L-flag here, the request gets redirected immediately
RewriteCond %{REQUEST_FILENAME} !-d
RewriteCond %{REQUEST_URI} /$
RewriteRule (.*)/ $1 [R=301,L]

# Add .php to access file, but don't redirect
# On some hosts RewriteCond %{REQUEST_FILENAME}.php -f will be true, even if
# no such file exists. Be safe and add an extra condition
# There is no point in escaping a dot in a string
RewriteCond %{REQUEST_FILENAME}.php -f
RewriteCond %{REQUEST_URI} !(/|\.php)$
RewriteRule (.*) $1.php [L]

This is basically a canonicalizing URL prettifier that redirects clients to a shorter URL.

The RewriteCond operating on %{THE_REQUEST} is weird, since it works directly at the HTTP protocol level.  I understand that you want to avoid redirecting POSTs, since a redirect would convert POSTs into GETs, and such breakage would be an unacceptable consequence for having nicer-looking URLs.  However, it would be better to test %{REQUEST_METHOD} instead.

In the worst case, you could end up redirecting the client three times:

GET /path/to/directory/index.php/:
GET /path/to/something/index.php
GET /path/to/something/index
GET /path/to/something

By combining the first two rules, you could eliminate the third request.

# Strip off /index.php or /index or .php
RewriteCond %{REQUEST_METHOD} =GET
RewriteRule (.*)(/index\.php|/index|\.php)$ $1 [R=301,L]

In the trailing slash cleanup, you have a superfluous RewriteCond:

# Remove slash if not directory
RewriteCond %{REQUEST_FILENAME} !-d
RewriteRule (.*)/$ $1 [R=301,L]

I would tweak your last rule a bit.  Do the string analysis check before the filesystem check.  Use $0 instead of %{REQUEST_URI} and $1.

RewriteCond $0 !(/|\.php)$
RewriteCond %{REQUEST_FILENAME}.php -f
RewriteRule .* $0.php


Implementation of a list-like array This is a follow up question for this question:

  Implementation of a dynamic list-like array

This is an implementation of an array that behaves like a list, granting me an easy and quick way to:

quickly add items
iterate over them without caring for empty spots in the array
quickly remove iterated items

After following user rolfl answer I revised my class to this

import java.lang.reflect.Array;
import java.util.Iterator;
import java.util.stream.IntStream;

public class BucketArray<T> implements Iterable<T> {

    private final T[] mItems;
    private final int[] mSlots;
    private int mSlotsTop = -1;

    public final int size;

    private int mIteratedIndex = -1;

    @SuppressWarnings("unchecked")
    public BucketArray(Class<T> cast, int size) {
        this.size = size;
        mItems = (T[]) Array.newInstance(cast, size);
        mSlots = IntStream.range(0, size).toArray();
        mSlotsTop = size - 1;
    }

    public int add(T item) {
        if (item == null)
            return -1;
        if (mSlotsTop < 0)
            return -1;
        final int slot = popSlot();
        mItems[slot] = item;
        return slot;
    }

    public void addAll(T... items) {
        for (T item : items)
            add(item);
    }

    public T get(int i) {
        if (i < 0 || i >= size)
            throw new IndexOutOfBoundsException();
        return mItems[i];
    }

    public int getIteratedIndex() {
        return mIteratedIndex;
    }

    public boolean remove(int i) {
        if (i < 0 || i > size)
            return false;
        if (mItems[i] == null)
            return false;
        mItems[i] = null;
        pushSlot(i);
        return true;
    }

    public void remove(T item) {
        remove(indexOf(item));
    }

    public boolean removeIterated() {
        return remove(mIteratedIndex);
    }

    public boolean isFull() {
        return mSlotsTop == -1;
    }

    public boolean isEmpty() {
        return mSlotsTop == size - 1;
    }

    public int numFreeSlots() {
        return mSlotsTop + 1;
    }

    public int indexOf(T item) {
        for (int i = 0; i < size; i++)
            if (item == mItems[i])
                return i;
        return -1;
    }

    private int popSlot() {
        return mSlots[mSlotsTop--];
    }

    private void pushSlot(int s) {
        mSlots[++mSlotsTop] = s;
    }

    @Override
    public Iterator<T> iterator() {
        return new Iterator<T>() {
            private int index = -1;
            {
                if (mIteratedIndex != -1)
                    index = mIteratedIndex;
            }

            @Override
            public boolean hasNext() {
                do {
                    index++;
                    if (index >= size) {
                        mIteratedIndex = -1;
                        return false;
                    }
                } while (mItems[index] == null);
                return true;
            }

            @Override
            public T next() {
                mIteratedIndex = index;
                return mItems[index];
            }
        };
    }
}

Major changes:

add() method now ignores null and returns the index of the
inserted item so it can be used with get() and remove().
After some thought, the remove() method mostly used in the context of an iteration, so I added  removeIterated() method that removes the currently iterated item in the scope of an iteration being made.
Name of the class changed to BucketArray from BagArray because it sounds better.

Usage example in deleting duplicated integer values:

public static void main(String[] args) {
        BucketArray<Integer> mBucket = new BucketArray<Integer>(Integer.class,20);
        mBucket.addAll(10, 20, 30, 20, 20, 30, 10, 40);
        // bucket is: 40, 10, 30, 20, 20, 30, 20, 10
        for (int a : mBucket)
            for (int b : mBucket)
                if (a == b)
                    mBucket.removeIterated();
        // bucket is now: 40, 10, 30, 20
    }

Potential bug #1

The problem of using a class variable mIteratedIndex that is modifiable in every new Iterator instance created is simply, multiple such instances cannot iterate through the contents reliably.

Potential bug #2

This is also related to Iterator in the hasNext() method: calling it twice without a next() effectively skips one element. Usually, the iteration state should not be modified when doing a hasNext(), but you have a index++ inside it.

Potential bug #3

The other problem with having index++ inside the hasNext() is that callers cannot reliably retrieve the next() element without calling hasNext() first.

Illustrating all bugs

public static void main(String[] args) {
    BucketArray<String> instance = new BucketArray<>(String.class, 3);
    instance.addAll("first", "middle", "last");
    Iterator<String> i1 = instance.iterator();
    i1.hasNext();
    i1.hasNext();
    System.out.println(i1.next());
    Iterator<String> i2 = instance.iterator();
    i2.hasNext();
    System.out.println(i2.next());
    System.out.println(i2.next());
    i2.hasNext();
    System.out.println(i2.next());
}

Output:

middle
first
first
Exception in thread "main" java.lang.ArrayIndexOutOfBoundsException: 3

i1.hasNext() has to be called before i1.next(), else there will be an ArrayIndexOutOfBoundsException: -1 error. When i1.hasNext() is called twice in succession, "middle" is returned from calling i1.next(), skipping the first element.

When we have a new i2 Iterator, it starts from where i1 left off, which bizarrely returns "first" since that was stored as the final element of the internal array - is this expected? Regardless, calling i2.next() twice returns the same value, when it shouldn't. Finally, calling the pair of hasNext()/next() methods triggers the ArrayIndexOutOfBoundsException: 3 error as i2 would have iterated past the contents of the array.


LeetCode "Jewels and Stones": counting certain characters in a string LeetCode "Jewels and Stones"

  You're given strings J representing the types of stones that are
  jewels, and S representing the stones you have.  Each character in
  S is a type of stone you have.  You want to know how many of the stones you have are also jewels.

  The letters in J are guaranteed distinct, and all characters in J
  and S are letters. Letters are case sensitive, so "a" is considered
  a different type of stone from "A".

  Example 1:

  Input: J = "aA", S = "aAAbbbb"
  Output: 3

  Example 2:

  Input: J = "z", S = "ZZ"
  Output: 0

  Note:

  S and J will consist of letters and have length at most 50. The
  characters in J are distinct.

My approach:

class Solution {
    public int numJewelsInStones(String J, String S) {

        int count = 0;
        for( int i = 0; i < J.length(); i++ )
        {
            for( int j = 0; j < S.length(); j++ )
            {
                if( J.charAt(i) == S.charAt(j) )
                    count++;
            }
        }
        return count;
    }
}

Time complexity: O(n^2)

Space complexity: O(1)

Another approach:

class Solution {
    public int numJewelsInStones(String J, String S) {
        Set<Character> jSet = new HashSet<>();
        for(Character ch : J.toCharArray()) {
            jSet.add(ch);
        }
        int count = 0;
        for(Character ch: S.toCharArray()) {
            if(jSet.contains(ch)) {
                count++;
            }
        }
        return count;
    }
}

Time complexity: O(n)
Space complexity: O(n)

I have the following questions regarding the above code snippets:

Which approach is better according to you?
Will the interviewer be more impressed by the 2nd method as it has used Set?
How can I further improve my code (sort the string and perform a binary search)-> O(n*log(n))?

Caution — naïve application of big-O analysis will lead you astray here!

First of all, you should be more precise about what you mean by "n".  In this problem, there are |J| and |S|: the lengths of J and S.  Your first approach is O(|J| |S|); your second one is O(|J| + |S|).

More importantly, big-O analysis only tells you how well an algorithm scales to handle large inputs.  In this challenge, though, |J| and |S| are at most 50 — very small inputs, by computer standards.  That means that the constant factors, which are disregarded in big-O analysis, actually matter.  (Another way to look at it: with those limits, |J| and |S| are both O(1), so any sane solution is also O(1)!)

Consider how much code is involved in making a HashSet.  A HashSet is actually a disguise for a HashMap.  A HashMap is implemented as an array of trees, each containing Map.Entry objects.  The array should be optimally sized to hold all the characters of J with no hashcode collisions, but you neglected to specify size hints when calling the HashSet<>() constructor.  Then, you have to insert each character of J, which involves boxing a char into a Character, calculating its hashcode, making a Map.Entry for it, and inserting it into the HashMap's table.  The problem is, Java makes it easy to execute a lot of operations without making you aware of how much work it actually is!

Since the challenge states that all characters are letters — which I interpret to mean the 52 letters in the English alphabet, you could achieve an efficient test of membership in J using a simple lookup table:

public int numJewelsInStones(String j, String s) {
    boolean[] jewels = new boolean[128];
    for (int i = 0; i < j.length(); i++) {
        jewels[j.codePointAt(i)] = true;
    }
    int count = 0;
    for (int i = 0; i < s.length(); i++) {
        if (jewels[s.codePointAt(i)]) {
            count++;
        }
    }
    return count;
}


Writing a generic casting extension-method I'm writing a C# extension-method that converts any object to a type. E.g. the method shall convert a string with value "y" to a true-boolean - so there will be some specific castings like bools, chars, DateTimes,...

Here are some examples of my target:

int number = 3;
string stringNumber = number.To<string>();

stringNumber now should be a string with value equals to "3". (Yeah, there's already the ToString(), it's just an example)
another one:

string myDateString = "Mon, 27 Jun 2007 12:34:56 CET";
DateTime myDate = myDateString.To<DateTime>();

myDate now should be a DateTime-object with value equals to a Date on 27th June of 2007 on time 12:34:56.

So, what I did is, I wrote following method:

public static T To<T>(this object @this)
{
    CastTo castedValue = null;

    if (typeof(T) == typeof(DateTime) || typeof(T) == typeof(DateTime?))
    {
        DateTime? result = null;

        if (@this is string actualStringValue)
            result = _tryCastStringToDateTime(actualStringValue);

        if (result != null)
            castedValue = new CastTo<DateTime> {Value = result.Value};
    }

    else if (typeof(T) == typeof(bool) || typeof(T) == typeof(bool?))
    {
        bool? result = null;

        if (@this is string actualStringValue)
            result = _tryCastStringToBoolean(actualStringValue);

        else if (@this is int actualIntegerValue)
            result = _tryCastIntegerToBoolean(actualIntegerValue);

        if (result != null)
            castedValue = new CastTo<bool> {Value = result.Value};
    }

    if (castedValue == null || castedValue.IsInitial)
        return (T) Convert.ChangeType(@this, typeof(T));

    return ((CastTo<T>) castedValue).Value;
}

those _tryCast....-methods simply cast an object to the specific type.

Here are my classes CastTo and CastTo<T>.

internal abstract class CastTo
{
    public abstract bool IsInitial { get; }

    public object Value { get; set; }
}

internal class CastTo<T> : CastTo
{
    private T _value;

    public override bool IsInitial => EqualityComparer<T>.Default.Equals(Value, default(T));

    public new T Value
    {
        get => _value;
        set
        {
            if (!EqualityComparer<T>.Default.Equals(value, default(T)))
                _value = value;
        }
    }
}

Are there any issues you'd suggest me to change?

Code-Style?
Performance?
Bad Practice?
Someone said, this is a misuse of generics. Is it really?

This design looks to me like a massive violation of KISS. For a start, I don't see the benefit of CastTo and CastTo<T>. It seems to me that they could be eliminated by tweaking the wrapped conversion methods to work more like int.TryParse with two return values: one to say whether the conversion succeeded, and the other to tell you what the result was. This doesn't necessarily need to use the same pattern of bool return value and out parameter for the result: it could use Tuple and C# 7's tuple unwrapping, for example. But it would simplify the 57 lines of code in the question to 19:

public static T To<T>(this object @this)
{
    if (typeof(T) == typeof(DateTime) || typeof(T) == typeof(DateTime?))
    {
        if (@this is string actualStringValue && _tryCastStringToDateTime(actualStringValue, out var result))
            return result;
    }

    else if (typeof(T) == typeof(bool) || typeof(T) == typeof(bool?))
    {
        if (@this is string actualStringValue && _tryCastStringToBoolean(actualStringValue, out var result))
            return result;

        else if (@this is int actualIntegerValue && _tryCastIntegerToBoolean(actualIntegerValue, out result))
            return result;
    }

    return (T) Convert.ChangeType(@this, typeof(T));
}

However, that code still raises red flags. There are situations where you have no choice but to use == typeof(...), but they are rare and mainly related to serialisation. Normally it's a sign that the code should be refactored. This method seems to be actively fighting against the type system. So I would agree completely with the person who told you that it's an abuse of generics. Consider also that if you expose the parse methods as

public static bool ToBool(this string str) { ... }
public static bool ToBool(this int i) { ... }
public static bool ToBool(this object obj)
{
    if (obj is string str) return ToBool(str);
    if (obj is int i) return ToBool(i);
    return (bool)Convert.ChangeType(obj, typeof(bool));
}

then in many actual use cases the compiler will be able to bind directly to the more specific method and skip the runtime checks.

(NB Code not tested, may have compile-time errors, but the principle is the important thing).


Kth largest element BST Would this take \$O(n)\$ in worst case? Can it be improved?

var tree = {
  val: 5,
  left: {
    val: 2, left: null, right: null
  },
  right: {
    val: 6,
    left: null,
    right: {
      val: 8,
      left: null,
      right: null
    }
  }
};

function inorderReverse(node, cb) {
  if (node.right !== null) inorderReverse(node.right, cb);
  cb(node);
  if (node.left  !== null) inorderReverse(node.left,  cb);
}

// Find the kth largest element
var k = 1;
var count = 1;
inorderReverse(tree, function(node) {
  if (count++ === k) console.log(node.val);
});

Assuming that you are forced for some reason to use that particular type of datastructure, I think you will end up with an O(k) algorithm.

To deal with the edge case, note that your code arguably works fine if k is too large--it just doesn't print anything.  You could also store the size of the tree and check that.  But you'll have to be careful to keep it updated when you manipulate the tree.

Alternately, you could change your tree-walking code to return a value as soon as it finds one:

function reverseFind(node, test) {
  if (node == null) return undefined;
  return reverseFind(node.right, test)
    || test(node) ? node : undefined
    || reverseFind(node.left,  test);
}

function kthLargest(tree, k) {
  var count = 1;
  var node = reverseFind(tree, function(node) {
    if (count++ === k) return true;
  });
  if (node) {
    console.log(node.val);
  } else {
    console.log('no value');
  }
}

This, at least, is O(k), not O(n).

If your data is reasonably static, it is probably better to just store your data in a sorted array.  Then you can get the kth largest element in constant time with much less code:

var sortedArray = [2, 5, 6, 8];
function kthLargest(sortedArray, k) {
  return sortedArray[sortedArray.length - k];
}

Note that this code assumes that there are no values in the array with negative indices (i.e., that sortedArray[-5] === undefined).  So you can deal with the edge case of invalid k by checking to see if kthLargest returns a value.


Grid UI layout for a database I've spent a few days making the transition from WinForms to WPF, and do not have much time for tutorials as the work needs to be done quickly.  I was wondering if anyone could take a look at a sample of my code and point out any conventions that I'm missing (and that are commonplace in modern WPF usage).  I've picked a random sample of some XAML code which I've written:

<Window x:Class=""
        xmlns="http://schemas.microsoft.com/winfx/2006/xaml/presentation"
        xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml"
         Width="300" Height="300">
    <Window.Resources>

    </Window.Resources>
    <StackPanel Orientation="Vertical">
        <Grid>
            <Grid.RowDefinitions>
                <RowDefinition>
                </RowDefinition>
                <RowDefinition>
                </RowDefinition>
                <RowDefinition>
                </RowDefinition>
                <RowDefinition>
                </RowDefinition>
                <RowDefinition>
                </RowDefinition>
            </Grid.RowDefinitions>
            <Grid.ColumnDefinitions>
                <ColumnDefinition Width="70">
                </ColumnDefinition>
                <ColumnDefinition Width="200">
                </ColumnDefinition>
            </Grid.ColumnDefinitions>
            Server:
            <ComboBox Grid.Column="1" Grid.Row="0" Name="cmbServer" Style="{StaticResource componentsStyle}"/>
            <CheckBox Name="chkWindowsAuthentication" Grid.Column="0" Grid.Row="1" IsChecked="False"/>
            Use Windows Authentication
            Username:
            <TextBox Grid.Column="1" Grid.Row="2" Name="txtUsername" Style="{StaticResource componentsStyle}"/>
            Password:
            <PasswordBox Grid.Column="1" Grid.Row="3" Name="txtPassword" Style="{StaticResource componentsStyle}"/>
            Database:
            <ComboBox Grid.Column="1" Grid.Row="4" Name="cmbDatabase" Style="{StaticResource componentsStyle}"/>
        </Grid>
        <StackPanel Orientation="Horizontal" Margin="10,30,10,10">
            <Button Margin="0,0,10,0" Style="{StaticResource buttonsStyle}">Connect</Button>
            <Button Style="{StaticResource buttonsStyle}">Reset</Button>
        </StackPanel>
    </StackPanel>
</Window>

The code is functional and displays the proper result, however I am worried that I am not using the right techniques.  Any input at all is greatly appreciated.

All in all, your code is quite well-structured. Here are a couple of suggestions for improvement:

Consider moving your styles to a resource dictionary:

<Window.Resources>
    <ResourceDictionary Source="Resources.xaml" />
</Window.Resources>

This will de-clutter your file and allow you to re-use the styles in other windows.
Collapse empty tags (use the self-closing syntax):

<Grid.RowDefinitions>
    <RowDefinition />
    <RowDefinition />
    <RowDefinition />
    <RowDefinition />
    <RowDefinition />
</Grid.RowDefinitions>

(the same goes for the ColumnDefinitions and any other empty tags)
Declare styles that affect all controls of one type without a key:

<Style TargetType="Button"> ... </Style>
<Style TargetType="Label"> ... </Style>

That way, you don't seperately have to set all the labels to labelStyle and all the  buttons to buttonsStyle, just remove all occurences of the following two attributes:

Style="{StaticResource labelStyle}"
Style="{StaticResource buttonsStyle}"

The default value of Grid.Column and Grid.Row is 0, you can get away without setting it explicitly. And CheckBox.Checked is false by default. So you can remove all occurences of the following:

Grid.Column="0"
Grid.Row="0"
IsChecked="False"

Generally, in WPF, widths and heights are not explicitly stated. You should try to use relative and automatic dimensions where possible (which often requires choosing the correct container).
For instance, you could use relative sizing for your grid columns:

<Grid.ColumnDefinitions>
    <ColumnDefinition Width="0.35*" />
    <ColumnDefinition Width="*" />
</Grid.ColumnDefinitions>

In other situations, working with Margin can be a replacement for explicit pixel sizes. Remember that hard-coded sizes are an obstacle to localization (where long words in foreign languages are cropped) and less flexible.

These suggestions shrink your Window .xaml file from 54 to 37 lines (Resources.xaml has 14).


Populating values into a C# object I have written a function to populate values into a class whose structure is as given below

 public class Country
    {
        public string Name { get; set; }
        public string Continent { get; set; }
        public string Zone { get; set; }
        public India IndiaInfo { get; set; }

    }

   public class India
    {
        public List<State> StateInfo{get; set;}
    }

    public class State
    {
        public string StateName { get; set; }
        public string StateCode { get; set; }
    }

The values are populated into the class object as shown below

public void PopulateCountry(string name, string Asia , string GMT , string AbcState, string RichState)
    {
        Country objCountry = new Country();
        objCountry.Continent = Asia;
        objCountry.Name = name;
        objCountry.Zone = GMT;

        **India objIndia = new India();
        objIndia.StateInfo = new List<State>;
        State objState = new State();
        objState.StateCode = AbcState;
        objState.StateName = RichState;
        objIndia.StateInfo.Add(objState);
        objCountry.IndiaInfo = objIndia;**

    }

In the above method to populate the object of class Country I have taken a brute force approach and put the values in the class and the code is giving results too.  However this code is not following some of the best practices and can be further enhanced /rewritten especially the one in Bold.  Can someone point out the things/features/practices of C# I am missing.

Edit# 1 Magus pointed out some of the conventions i was not following Regarding this particular method, PopulateCountry 1) can we have better methods to instantiate the state object/india object , taking into account their relationship as per class structure 2) the last line of code where i have directly assigned the object,is that fine and according to standards?

A few things straight away:

You have a class called India which really just holds a collection of States. I would suggest not using that class at all, instead creating a List<State> States instead.

Info as a suffix is fairly meaningless, particularly when the object is a collection.

You also do not need to put obj in front of objects you create. In C#, everything is an object, so that means absolutely nothing.

In addition, the names you've chosen for parameters are particularly bad. Asia might be a good value, but continent is a far better variable name.

Last of all, you may want to use object initializers:

public class Country
{
    public string Name { get; set; }
    public string Continent { get; set; }
    public string Zone { get; set; }
    public List<State> Country { get; set; }
}

public void PopulateCountry(string name, string continent, string timeZone, string stateCode, string stateName)
{
    var state = new State
    {
        StateCode = stateCode,
        StateName = stateName
    }
    var country = new Country
    {
        Continent = continent,
        Name = name,
        Zone = timeZone,
        Country = new List<State> { state }
    }
}

This isn't everything you can do. It may make more sense to make a Country class if you plan on adding other things to it. This at the very least makes your code simpler, though.

Alternatively, you could assign a country like this, anywhere:

var country = new Country
{
    Continent = continent,
    Name = name,
    Zone = timeZone,
    Country = new List<State>
    {
       new State
       {
           Name = stateName1,
           Code = stateCode1
       },
       new State
       {
           Name = stateName2,
           Code = stateCode2
       },
    }
}

Or even:

var country = new Country
{
    Continent = continent,
    Name = name,
    Zone = timeZone
}
country.Country = names.Zip(codes, (x, y) => new State { Name = x, Code = y };

Or something close to it, assuming the two are in the correct order. You have many options.


Bucketizing time (e.g., 4 min => 0-5 min) Specification:

Given a string (e.g., the content of an article), print the estimated reading time by bucketizing time into increasingly large chunks in the form "5-10 min" or "48 min-2 hr".

Notes:

Distribution of time into buckets is arbitrary, to make it "look nice" for the default 5-minute time chunks. This is okay.

60 minutes exactly is special-cased, so that it should be "30-60 min" instead of "30 min-1 hr".

The "real" application of this code is inside a plugin for something else; rather than printing verbose information like in the code below, it actually returns $time_label.

I don't normally work in PHP, so any "idiomatic" feedback is also welcome.

Sample Input:
print_reading_time("This is a test.");

print_reading_time(str_repeat("test ",   100));
print_reading_time(str_repeat("test ",  1000));
print_reading_time(str_repeat("test ", 10000));
print_reading_time(str_repeat("test ", 20000));

print_reading_time(str_repeat("test ", 10000), 8);

Sample Output:

estimated reading time: 0 min (0-5 min), based on 4 words at 180 WPM
estimated reading time: 1 min (0-5 min), based on 100 words at 180 WPM
estimated reading time: 6 min (5-10 min), based on 1000 words at 180 WPM
estimated reading time: 56 min (30-60 min), based on 10000 words at 180 WPM
estimated reading time: 111 min (2 hr), based on 20000 words at 180 WPM
estimated reading time: 56 min (48 min-2 hr), based on 10000 words at 180 WPM

Original:
<?php

function print_reading_time($content, $minimum_time_bucket = 5 /* minutes */) {
  $wpm = 180; // arbitrary value, based on adult averages
  $word_count = str_word_count($content);
  $minutes = round($word_count / $wpm);
  $time_label = minutes_to_time_label($minutes, $minimum_time_bucket);

  print("estimated reading time: $minutes min ($time_label), " .
      "based on $word_count words at $wpm WPM");
}

function minutes_to_time_label($minutes, $minimum_time_bucket) {
  // Buckets: <5, 5-10, 10-15, 15-30, 30-60, then 1-hour increments,
  // for the default 5-minute minimum bucket size.
  $buckets = array(0, 1, 2, 3, 6, 12);

  for ($i = 0; $i < count($buckets) - 1; $i++) {
    $lowerBound = $buckets[$i] * $minimum_time_bucket;
    $upperBound = $buckets[$i+1] * $minimum_time_bucket;

    if ($lowerBound <= $minutes && $minutes < $upperBound) {
      $lowerOptions = array();
      // If the amounts are both in minutes or both in hours, don't display
      // the units for the first amount (ie, show "5-10 min", not "5 min-10 min".
      if (($lowerBound <= 60 && $upperBound <= 60) || ($lowerBound > 60 && $upperBound > 60)) {
        $lowerOptions["no_units"] = true;
      }
      return minutes_to_time_str($lowerBound, $lowerOptions) . "-"
          . minutes_to_time_str($upperBound);
    }
  }

  // If we get to here, then the time is longer than our largest explicit bucket.
  // Divide everything beyond into hour-long buckets.
  return minutes_to_time_str($minutes, array("treat_60_min_as_an_hour" => true));
}

function minutes_to_time_str($minutes, $options = array()) {
  $options = array_merge(default_options(), $options);
  $amount = $minutes;
  $units = "min";

  if ($minutes > 60 || ($minutes === 60 && $options["treat_60_min_as_an_hour"])) {
    $amount = round($minutes / 60);
    $units = "hr";
  }

  return $amount . ($options["no_units"] ? "" : " $units");
}

function default_options() {
  return array(
    "no_units" => false,
    "treat_60_min_as_an_hour" => false,
  );
}

?>

Here are some thoughts for you. I'm guessing other folks will (hopefully) tackle things from a variety of perspectives.

print_reading_time

How about changing this function to reading_time and returning nothing but the estimated time.  Perhaps change the declaration to:

reading_time($content, $wpm=180)

This function can be adjusted based on the reader and does not really need to know anything about the time buckets or how the output will be generated. You are mixing in additional concepts and that is likely to make it harder to reuse or extend the functionality.

minutes_to_time_label

Based on your descriptive language I'd probably go with something like:

minutes_to_time_bucket($minutes, $initial_bucket_size)

From here I think I'd immediately jump into a determination of whether or not the estimate is greater than 1 hour.

if ( $minutes > 60 )
{
  $low = floor($minutes/60);
  $high = $low + 1;

  // return time label here
}

If we are below 60 minutes we can go into the particulars of how to scale the buckets (and do all of our work in minutes).

$bounds = array(0,5,10,15,30,60);
foreach ($bounds as $key => $val)
{
  if ( $minutes >= $val && $minutes <= $bounds[$key+1] )
  {
    // generate label using $bounds[$key] and $bounds[$key+1]
  }
}

// If we get here there is an error, quit with error message

If you want to support bounds more dynamically, you could generate the bounds array based on the initial_bucket_size passed in until you end up with a value of 60 or greater.

You could use an array of multipliers to do this. The multipliers using a bucket size of 5 would be 0, 1, 2, 3, 6, 12. If you optionally passed in the multipliers as an array you'd be able to change boundary definitions on the fly.

Note

I've assumed the options array and use of two functions related to label output was additional complexity to handle units. If you wanted that flexibility back you could add a parameter or two to the minutes_to_time_bucket function.

As an extension, you could perhaps support multiple languages by passing in the text to display for minutes and hours to override English defaults.

In short, my feedback would be to consider ways to keep your functions focused on a single concept. In this case we have a calculation of reading time and a method of generating a text time range based on a reading time. Some controlling code will mash those concepts together (not the functions themselves).

Beyond that, consider how to generalize those functions to support a wider variety of use within that focus. For example, an ability to use the readers own reading speed or output language doesn't change the focus of the function but will allow for use in a wider set of scenarios.


Keeping integer addition within bounds I have to read a int8_t from a buffer, read a uint8_t from the camera's current settings, add them together, keep new value within the bounds 0 to 100 and write the new value to the camera's current settings. This seems quite easy to me, and yet here I have a rather large bit of code.

The brace style is company-mandated.

Is there a way to remove some of the complexity from this code?

Ranges:
plusZoomPercentage: full range of int8_t.
currentZoomPercentage and getZoomPercentage(): 0 to 100, 0 and 100 included.
newZoomPercentage: To be between 0 and 100, 0 and 100 included.

Header file:

/*Zooms to the provided percentage of maximum zoom level.*/
void zoomToPercentage(uint8_t percentage);

/*Retrieves the current targeted percentage of maximum zoom level (if the camera is currently at 50% zoom, but is zooming towards 60%, return 60.)*/
uint8_t getZoomPercentage();

Calling implementation:

int8_t plusZoomPercentage;
if (!unabto_query_read_int8(readBuffer, &plusZoomPercentage))
{
    return AER_REQ_TOO_SMALL;
}
if (plusZoomPercentage == 0)
{
    return AER_REQ_RESPONSE_READY;
}
uint8_t currentZoomPercentage = getZoomPercentage();
uint8_t newZoomPercentage = currentZoomPercentage + plusZoomPercentage;
if (plusZoomPercentage > 0)
{
    /*Check for overflow or otherwise going out of bounds*/
    if (newZoomPercentage > 100 || newZoomPercentage < currentZoomPercentage)
    {
        zoomToPercentage(100);
    }
    else
    {
        zoomToPercentage(newZoomPercentage);
    }
}
else
{
    if (newZoomPercentage > currentZoomPercentage)
    {
        zoomToPercentage(0);
    }
    else
    {
        zoomToPercentage(newZoomPercentage);
    }
}
return AER_REQ_RESPONSE_READY;

It will be a bit shorter and simpler without the duplicated else blocks:

if (plusZoomPercentage > 0)
{
    /*Check for overflow or otherwise going out of bounds*/
    if (newZoomPercentage > 100 || newZoomPercentage < currentZoomPercentage)
    {
        newZoomPercentage = 100;
    }
}
else if (newZoomPercentage > currentZoomPercentage)
{
    newZoomPercentage = 0;
}
zoomToPercentage(newZoomPercentage);
return AER_REQ_RESPONSE_READY;


Group by lastName and variable firstName I'm trying to solve a problem of how to group name objects. A name has firstName and lastName properties. Need to group the name objects that have identical lastnames and a similar firstName(aka Tom Smith and Thomas Smith should be grouped together). Case doesn't matter. The firstName equivalency is provided by a string separated by ";"'s.
example:
input:
const FIRST_NAME_DICTIONARY =
  "Robert, Bob, Bobby; Liz, Elizabeth, Beth; Tom, Thomas";

const people = [
  { firstName: "robert", lastName: "smith" },
  { firstName: "Liz", lastName: "thomas" },
  { firstName: "robert", lastName: "smith" },
  { firstName: "Thomas", lastName: "hardin" },
  { firstName: "Elizabeth", lastName: "thomas" },
  { firstName: "bob", lastName: "smith" },
  { firstName: "Bobby", lastName: "smith" },
  { firstName: "ryan", lastName: "roberts" },
  { firstName: "bob", lastName: "wallace" },
  { firstName: "bobby", lastName: "smith" },
  { firstName: "beth", lastName: "roberts" },
  { firstName: "beth", lastName: "thomas" },
  { firstName: "Tom", lastName: "hardin" },
];

output:
[
    [
      { firstName: 'robert', lastName: 'smith' },
      { firstName: 'robert', lastName: 'smith' },
      { firstName: 'bob', lastName: 'smith' },
      { firstName: 'Bobby', lastName: 'smith' },
      { firstName: 'bobby', lastName: 'smith' }
    ],
    [
      { firstName: 'Liz', lastName: 'thomas' },
      { firstName: 'Elizabeth', lastName: 'thomas' },
      { firstName: 'beth', lastName: 'thomas' }
    ],
    [
      { firstName: 'Thomas', lastName: 'hardin' },
      { firstName: 'Tom', lastName: 'hardin' }
    ],
    [ { firstName: 'ryan', lastName: 'roberts' } ],
    [ { firstName: 'bob', lastName: 'wallace' } ],
    [ { firstName: 'beth', lastName: 'roberts' } ]
]

Here's what I have now:
const groupDuplicates = (list, dictionary) => {
  const mappedNames = mapDictionary(dictionary);
  const groupByNames = {};
  people.forEach((person) => {
    // gets the id equivalent for firstName or use the firstName it the id DNE
    const firstNameId =
      mappedNames[person?.firstName?.toLowerCase()] ||
      person?.firstName.toLowerCase(); // example 1 or ryan since ryan dne in the dictionary
    const stringifyKey = JSON.stringify([
      firstNameId,
      person.lastName.toLowerCase(),
    ]); // example "[1, smith]"

    // if key exists push to that array, otherwise create the key and then push
    if (groupByNames[stringifyKey]) {
      groupByNames[stringifyKey].push(person);
    } else {
      groupByNames[stringifyKey] = [person];
    }
  });

  // essentially convert object into array
  const keys = Object.keys(groupByNames);
  const groupByNamesArray = [];
  keys.forEach((key) => {
    groupByNamesArray.push(groupByNames[key]);
  });

  return groupByNamesArray;
};

// {
//     robert: 1,
//     bob: 1,
//     bobby: 1,
//     liz: 2,
//     elizabeth: 2,
//     beth: 2,
//     tom: 3,
//     thomas: 3
// }
const mapDictionary = (dictionary) => {
  const nameGroups = dictionary.split(";");
  let nameKey = 1;
  const mapNameToKey = {};
  nameGroups.forEach((nameGroup) => {
    const names = nameGroup.split(",");
    names.forEach((name) => {
      const noSpacesAndLowercase = name.trim().toLowerCase();
      mapNameToKey[noSpacesAndLowercase] = nameKey;
    });
    nameKey += 1;
  });
  return mapNameToKey;
};

Seems to work but is there a better way to accomplish this? I think the JSON.stringify seems a little hacky.

If you have a simple groupBy function, and group by a string key with format [lastName]|[firstNameKey or firstName], it'd be cleaner. The following is my version of the code:

// FUNCTIONS
const groupBy = (list, key) => {
  return list.reduce((current, value) => {
    (current[key(value)] = current[key(value)] || []).push(value);
    return current;
  }, {});
};

const mapDictionary =
  (str) => str
  .split(';')
  .map((val, idx) => {
    return {
      grp: idx + 1,
      val: val
    };
  })
  .reduce((current, group) => {
    const groupResult = group.val
      .split(',').reduce((gr, name) => {
        gr[name.trim().toLowerCase()] = group.grp;
        return gr;
      }, {});
    return {
      ...current,
      ...groupResult
    };
  }, {});

const groupDuplicate = (people, dictionary) => {
  return Object.values(groupBy(people, (person) => `${person.lastName}|${dictionary[person.firstName.toLowerCase()] || person.firstName}`));
};

// TEST CASE
const FIRST_NAME_DICTIONARY =
  "Robert, Bob, Bobby; Liz, Elizabeth, Beth; Tom, Thomas";

const people = [{
    firstName: "robert",
    lastName: "smith"
  },
  {
    firstName: "Liz",
    lastName: "thomas"
  },
  {
    firstName: "robert",
    lastName: "smith"
  },
  {
    firstName: "Thomas",
    lastName: "hardin"
  },
  {
    firstName: "Elizabeth",
    lastName: "thomas"
  },
  {
    firstName: "bob",
    lastName: "smith"
  },
  {
    firstName: "Bobby",
    lastName: "smith"
  },
  {
    firstName: "ryan",
    lastName: "roberts"
  },
  {
    firstName: "bob",
    lastName: "wallace"
  },
  {
    firstName: "bobby",
    lastName: "smith"
  },
  {
    firstName: "beth",
    lastName: "roberts"
  },
  {
    firstName: "beth",
    lastName: "thomas"
  },
  {
    firstName: "Tom",
    lastName: "hardin"
  },
];

const result = groupDuplicate(people, mapDictionary(FIRST_NAME_DICTIONARY));

// PRINT RESULT
console.log(result)


Implementation of inheritable pImpl idiom and MVC pattern I am trying to write an object "template" that use pImpl and MVC pattern to encapsulate everything not specific to the object. The following is how I tried to attempt it. I am sure that the code smells (if not stinks) but can't tell what's wrong with the design.

I would like to know what better alternatives with which I can do this.

#include <iostream>

using namespace std;

class Ancestor{}; //every object using the "template" inherits this class

//use a template as the type of IMPL is only known at compile time
template <typename IMPL> class Obj : public Ancestor {
    public:
        //dependency injection for easy TDD
        Obj(IMPL *impl = new IMPL()) : m_pImpl(impl) {};
        ~Obj() {delete m_pImpl; m_pImpl = nullptr;};
    protected:
        IMPL *m_pImpl;
};

// well a template is used for the same reason as for Obj<>
template <typename MODEL> struct MVCController {
    MODEL *m_pModel;
};

// same reason for a template
template <typename MODEL, typename CONTROLLER> struct MVCImpl {
    MODEL *m_pModel;
    CONTROLLER *m_pController;
    // leave alone the View for simplicity

    //constructor
    MVCImpl(MODEL *m = new MODEL(), CONTROLLER *c = new CONTROLLER()) :
    m_pModel(m), m_pController(c) {m_pController->m_pModel = m;};
};

struct someBaseModel{};

If I want to create an object called myObj with a public API void foo():

struct myModel : someBaseModel {
    // POD
};

struct myController : public MVCController<myModel> {
    void foo() {cout << "myController: foo" << endl;};
};

struct myImpl : public MVCImpl<myModel, myController> {};

class myObj : public Obj<myImpl> {
    public:
        // inherit myBaseObj to call its default ctor/dtor if no customization is needed
        myObj(myImpl *impl = new myImpl()) : Obj<myImpl>(impl) {};
        void foo();
};

void myObj::foo() {m_pImpl->m_pController->foo();};

If I want myDerivedObj to inherit myObj and add a void bar():

struct myDerivedController : public myController {
    void bar() {cout<<"myDerivedController: bar"<<endl;};
};

struct myDerivedImpl : public MVCImpl<myModel, myDerivedController> {};

// virtual inheritance is used as both inherited class inherits from Ancestor
class myDerivedObj : virtual public myObj, virtual public Obj<myDerivedImpl> {
    using Obj<myDerivedImpl>::m_pImpl;
    public:
        myDerivedObj(myDerivedImpl *impl = new myDerivedImpl()) :
            Obj<myDerivedImpl>(impl) {};
        // add any new API here
    void bar();
};

void myDerivedObj::bar() {m_pImpl->m_pController->bar();};

int main() {
    auto m2 = new myDerivedObj();
    m2->foo();
    m2->bar();

    return 0;
}

Output:

myController: foo
myDerivedController: bar

By using the above implementation I am trying to

hide all pImpl and MVC specific code in the templates
hide all generic "object behaviors" in Ancestor
leave the possibility for inheritance

Stop doing this:

using namespace std;

Its a bad habit that you need to stop. Prefix your stuff with std:: and don't be lazy. The whole reason its std and not standard_lib is so that it would not be an issue using the prefix everywhere. Anyway there are issues that will crop up in real code see Why is “using namespace std;” considered bad practice?

Why.

class Ancestor{}; //every object using the "template" inherits this class

Just saying they do is not a good comment.
A good comment would explain why they need to. Also since there is no interface you are implementing I don't see a reason for this (maybe a comment would clarify).

In this class:

template <typename IMPL> class Obj : public Ancestor {

You are doing resource management. But you don't implement the rule of three (as such it is broken). Please make sure you correctly do resource management. What is The Rule of Three?

While we are here. You may also want to look at move semantics (its 2014 so you should be using at least C++11 or C++14). This will make your code more efficient in the 3% of places that RVO and NRVO don't apply. Also containers are now move aware that makes them more efficient.

Pointers in your class. Not good.

// well a template is used for the same reason as for Obj<>
template <typename MODEL> struct MVCController {
    MODEL *m_pModel;
};

A pointer does not semantically help with determining ownership. I assume a controller is always linked to a model (ie it is never null) so I think a reference would be a better way to represent the connection to the model. If you can potentially have a NULL model (I would not go this way) then maybe a std::shared_ptr as many controllers can potentially use the same model.

A lot of implementations combine the controller and model so that is fine.

// same reason for a template
template <typename MODEL, typename CONTROLLER> struct MVCImpl {
    MODEL *m_pModel;
    CONTROLLER *m_pController;
    // leave alone the View for simplicity

    //constructor
    MVCImpl(MODEL *m = new MODEL(), CONTROLLER *c = new CONTROLLER()) :
    m_pModel(m), m_pController(c) {m_pController->m_pModel = m;};
};

But you have an bad obsession about creating these things dynamically. Please use to learn how to use automatic objects. This makes resource management much easier.

Also you do need to figure out how the view(s) are connected to your MC object.


Retry method execution on failure? I am working on an application that is required to be constantly running, and as a requirement should make multiple attempts for particular processes such as web API/database calls, just in case network connectivity goes down for a moment, or something similar.
In this case, I'm constantly having to write a basic algorithm:
for (int attempt = 0; i < Config.AttemptCount; i++) {
    try {
        doSomething...;
        return true;
    } catch (Exception e) {
        Log(e);
    } finally {
        if (Config.WaitTimeForRetry > 0)
            Thread.Sleep(Config.WaitTimeForRetry);
    }
}

return false;

This of course is the most basic example, but there are others where I'm returning actual objects, performing heavy operations, etc. I recently got the idea to make a single method that could perform this algorithm for me so I can focus on the overall functionality with each implementation instead. With that said, I present to you, a method I need a better name for, and probably needs a much better implementation, or should be replaced by something pre-existing if such a thing exists because I re-invent the wheel a lot, but none-the-less, it's here .
public static TOut RunProcessWithMultipleAttempts<TIn, TOut>(int numberOfAttempts, int waitTimeInMilliseconds, TIn input, Func<TIn, TOut> process) {
    TOut result = default;
    while (numberOfAttempts-- > 0) {
        try {
            result = process.Invoke(input);
            break;
        } catch (Exception e) {
            Log(e);
        } finally {
            if (waitTimeInMilliseconds > 0)
                Thread.Sleep(waitTimeInMilliseconds);
        }
    }

    return result;
}

To use this, I would write a method that does the work I wish to be performed:
static int attempts = 0;
private static string TrySomething(int i) {
    for (int index = 0; index < i; index++)
        if (attempts++ < 2)
            throw new Exception("Your request was denied.");

    return "Your request was successful.";
}

I would then utilize the RunProcessWithMultipleAttempts method to execute it:
static void Main(string[] args) {
    Console.WriteLine(RunProcessWithMultipleAttempts(3, 1000, 10, TrySomething));

    // Prevent the console from closing.
    Console.ReadKey();
}

In this example, replace the Log(e) call with Console.WriteLine(e) to allow it to work on your local machine.

A few questions in my mind:

How hard is this to scale?
What are the downsides to this method?
What alternatives are available?
Is this code easy to understand?
Not only in final usage, but initial implementation?

Oh, and perhaps my biggest question if I were to keep this implementation around:

What would a more concise and clear name be for this method?

How hard is this to scale?
Sorry but I don't understand what do you mean here. How do you want to scale a method?
What are the downsides to this method?

It does not work with Action

It does not work with Func<T1, T2, TResult>

It does not work with Func<Task>

It does not work with Func<Task<T>>

It blocks the calling thread unnecessary (Thread.Sleep)

You must throw exception to enforce retry (can't use any other condition, like Result != true)

It swallows the exception

in the worst case if all attempts fail then it will return a default instead of the exception

It does not allow to use exponential back-off algorithm

It does not allow infinite retry (performing yet another attempt until it succeeds)

It does not utilize Cancellation concept

It does not provide debug information (like how many times the retry has been performed)

What alternatives are available?

Polly's Retry << The suggested
EnterpriseLibrary.TransientFaultHandling.Core

Is this code easy to understand?
For me, yes.
Not only in final usage, but initial implementation?
Is this a question?
What would a more concise and clear name be for this method?

ExecuteWithRetry
ExecuteAtMostNTimes
RetryFunction
PerformCallWithRetryPolicy
etc.


Largest palindrome that can be made from a product of 2 3-digit numbers (Project Euler 4) This is my solution to Project Euler #4, which asks for the largest palindrome that can be made from a product of 2 3-digit numbers. I have done a brute force method, but it takes around 10-20 seconds to run. It works by looping through 2 variables, finding its product, and converting it to a string to check for palindromity. What steps can I take to improve the computational speed of this program?

#include <cmath>
#include <string>
#include <sstream>
#include <iostream>
int main()
{
int ans=0;
for(int i=100;i<1000;i++)
    for(int j=100;j<1000;j++)
    {
        int num_to_test=i*j;
        int num_of_digits=floor(log10(num_to_test))+1;
        string num_stringstream;
        ostringstream convert;
        convert << num_to_test;
        num_stringstream=convert.str();
        int counter=0;
        for(int i=0;i<num_of_digits;i++)
        {
            if(num_stringstream[i]==num_stringstream[num_of_digits-i-1])
            {
                counter++;
                if(counter==num_of_digits)
                    num_to_test>ans?ans=num_to_test:ans=ans;
            }
        }
    }
cout << ans << endl;
char quit;
cin >> quit;
}

Speeding things up
There are several things you could do to speed things up:

Instead of looping from 100..999, go from 999..100 instead.  That way, if you find a palindrome, you can break from the j loop because any lower values of j will not beat the one you just found.
Once you loop backwards, you can also check i*j versus the best answer you've found so far.  If i*j <= best, you can break from the j loop.
You don't have to use log10 to find the number of digits.  Since you are already converting the number to a string, just use num_stringstream.length().
The j loop can start at i instead of 999, because any j higher than i will already have been tested.

Cleaning things up
It would make your program look a lot more readable if you moved all of the palindrome checking logic to its own function.
This line here is very hard to read:

num_to_test>ans?ans=num_to_test:ans=ans;

Putting it all together
Here is what I think your code should look like.  I removed the cin << quit part because I wanted to time the program without requiring user input.
#include <string>
#include <sstream>
#include <iostream>

bool is_palindrome(int num)
{
    ostringstream convert;
    convert << num;
    string num_stringstream = convert.str();
    int num_of_digits = num_stringstream.length();
    for (int i=0; i < num_of_digits; i++)
    {
        if (num_stringstream[i] != num_stringstream[num_of_digits-i-1])
            return false;
    }
    return true;
}

int main(void)
{
    int best = 0;
    for (int i = 999; i >= 100; i--)
    {
        for (int j = i; j >= 100; j--)
        {
            int num_to_test = i * j;

            if (num_to_test <= best)
                break;

            if (is_palindrome(num_to_test)) {
                best = num_to_test;
                break;
            }
        }
    }
    cout << best << endl;
}

Using gcc -O3, this program ran in 0.03 seconds on my computer as opposed to 1.06 seconds, for around a 30x speedup.


PHP MySQL Database class This is a PHP database class.  Yes, I know it's using the MySQL functions, which are deprecated, but I shall be updating it to MySQLi soon. Can you please review this code and give any comment on any improvements or changes you think I should make?

<?php

namespace Revolution;
if(!defined('IN_INDEX')) { die('Sorry, you cannot access this file.'); }
class engine
{
    private $initiated;
    private $connected;

    private $connection;

    final public function __construct()
    {
        $this->Initiate();
    }

    final public function Initiate()
    {
        global $_CONFIG;
        if(!$this->initiated)
        {
            $this->setMySQL('connect', mysql_connect);
            $this->setMySQL('pconnect', mysql_pconnect);
            $this->setMysql('select_db', mysql_select_db);
            $this->setMySQL('query', mysql_query);
            $this->setMySQL('num_rows', mysql_num_rows);
            $this->setMySQL('fetch_assoc', mysql_fetch_assoc);
            $this->setMySQL('fetch_array',mysql_fetch_array);
            $this->setMySQL('result', mysql_result);
            $this->setMySQL('free_result', mysql_free_result);
            $this->setMySQL('escape_string', mysql_real_escape_string);

            $this->initiated = true;

            $this->connect($_CONFIG['mysql']['connection_type']);
        }
    }

final public function setMySQL($key, $value)
{
    $this->mysql[$key] = $value;
}

/*-------------------------------Manage Connection-------------------------------------*/

final public function connect($type)
{
    global $core, $_CONFIG;
    if(!$this->connected)
    {
        $this->connection = $this->mysql[$type]($_CONFIG['mysql']['hostname'], $_CONFIG['mysql']['username'], $_CONFIG['mysql']['password']);

        if($this->connection)
        {
            $mydatabase = $this->mysql['select_db']($_CONFIG['mysql']['database'], $this->connection);

            if($mydatabase)
            {
                $this->connected = true;
            }
            else
            {
                $core->systemError('MySQL Engine', 'MySQL could not connect to database');
            }
        }
        else
        {
            $core->systemError('MySQL Engine', 'MySQL could not connect to host');
        }
    }
}

final public function disconnect()
{
    global $core;
    if($this->connected)
    {
        if($this->mysql['close'])
        {
            $this->connected = false;
        }
        else
        {
            $core->systemError('MySQL Engine', 'MySQL could not disconnect.');
        }
    }
}

/*-------------------------------Secure MySQL variables-------------------------------------*/

final public function secure($var)
{
    return $this->mysql['escape_string'](stripslashes(htmlspecialchars($var)));
}

/*-------------------------------Manage MySQL queries-------------------------------------*/

final public function query($sql)
{
    return $this->mysql['query']($sql, $this->connection) or die(mysql_error());
}

final public function num_rows($sql)
{
    return $this->mysql['num_rows']($this->mysql['query']($sql, $this->connection));
}

final public function result($sql)
{
    return $this->mysql['result']($this->mysql['query']($sql, $this->connection), 0);
}

final public function free_result($sql)
{
    return $this->mysql['free_result']($sql);
}

final public function fetch_array($sql)
{
    $query = $this->mysql['query']($sql, $this->connection);

    $data = array();

    while($row = $this->mysql['fetch_array']($query))
    {
        $data[] = $row;
    }

    return $data;
}

final public function fetch_assoc($sql)
{
    return $this->mysql['fetch_assoc']($this->mysql['query']($sql, $this->connection));
}

}
?>

namespace Revolution;
class engine

I do not understand these names.  I would have expected to see something database related.  Neither Revolution nor engine says database to me.

final public function __construct()
{
    $this->Initiate();
}

final public function Initiate()
{
    global $_CONFIG;
    if(!$this->initiated)
    {
        $this->setMySQL('connect', mysql_connect);
        $this->setMySQL('pconnect', mysql_pconnect);
        $this->setMysql('select_db', mysql_select_db);
        $this->setMySQL('query', mysql_query);
        $this->setMySQL('num_rows', mysql_num_rows);
        $this->setMySQL('fetch_assoc', mysql_fetch_assoc);
        $this->setMySQL('fetch_array',mysql_fetch_array);
        $this->setMySQL('result', mysql_result);
        $this->setMySQL('free_result', mysql_free_result);
        $this->setMySQL('escape_string', mysql_real_escape_string);

        $this->initiated = true;

        $this->connect($_CONFIG['mysql']['connection_type']);
    }
}

As a general rule, you don't want to do your database connection in your constructor.  You should be able to create an instance without having it connected to a database.  What you might consider doing is creating a fetchConnectedInstance that runs the constructor and initializes it, returning the result.  You should pass $_CONFIG into this function and Instance.  That way you can connect to more than one database at once.  Your version is almost a Singleton but not quite.

I don't see the point in things like

        $this->setMysql('select_db', mysql_select_db);
        $mydatabase = $this->mysql['select_db']($_CONFIG['mysql']['database'], $this->connection);

Note that $this->mysql['select_db'] is longer than mysql_select_db, so it's clear that this is not a convenience abbreviation.  What is the benefit?  It almost seems like you are trying to make a base class that you can reuse for other database types.  Unfortunately, I don't know that you can rely on each database offering the same abilities.  For example, a key/value database is not going to offer some of the capabilities that a relational database will.

If you are going to alias something, then there should be a reason.  If there's a reason, you should comment on it so that the reader knows it.  As a general rule, if you are clever, explain to the reader how it's clever.  That way, the reader doesn't need to be clever just to read the code.

I don't like the use of $this->initiated.  It doesn't seem necessary.  Either don't care about that situation, or check something more direct like the existence of $this->mysql.

/*-------------------------------Secure MySQL variables-------------------------------------*/

final public function secure($var)
{
    return $this->mysql['escape_string'](stripslashes(htmlspecialchars($var)));
}

This is not the right way to handle this.  The htmlspecialchars function should not be called before putting something in the database.  It should be called after fetching something from the database and before outputting it in HTML.  It should be the last thing called before outputting.  Doing it this way risks someone finding a special input that will evade htmlspecialchars and then be enabled by the stripslashes/mysql_real_escape_string.  Also, htmlspecialchars can change the data if used at this point.  What if you want to store HTML in the database?  E.g. user entered content.

You also aren't using bind parameters at all.  In general, bind parameters are considered a safer way of handling things than using mysql_real_escape_string.

    if($this->mysql['close'])

What this does is check if $this->mysql['close'] has a true or false evaluation.  You may have meant to do at least

    if ($this->mysql['close']())

However, that's still incorrect.  That closes the last database connection that was open.  That may or may not be the right connection.  It really should be

    if ($this->mysql['close']($this->connection))

In the following two function signatures, you use $sql to represent two different things:

final public function query($sql)
final public function num_rows($sql)

In the first, $sql is a string representing a MySQL query (i.e. a SQL string).  In the second, it is a query result handle.  I find that $sql makes a lot of sense in the first signature and no sense in the second.  I would suggest changing the second to something like

final public function num_rows($query)

You also have a function that doesn't quite do what one would expect:

final public function fetch_assoc($sql)
{
    return $this->mysql['fetch_assoc']($this->mysql['query']($sql, $this->connection));
}

What you do here is run a query based on the $sql string and then return the first row of the result.  However, you now have no way to get subsequent rows, as you throw away the query result handle.  You take care of this situation with a while in

final public function fetch_array($sql)
{
    $query = $this->mysql['query']($sql, $this->connection);

    $data = array();

    while($row = $this->mysql['fetch_array']($query))
    {
        $data[] = $row;
    }

    return $data;
}

This will usually work but can cause problems if the number of rows is large.  You are essentially reading the entire result into memory and then processing it later.  If there's a relatively small amount of data, this is fine.  However, if there is a large amount of data, then you can overload the server if you have this happen in too many results at once.  For this reason, it is more common to allow the caller to take care of the while loop portion.  The caller can either save the data or use and discard the results immediately.

I don't want to say a lot about this, since you already noted it.  It's worth repeating though.  You would generally be better off using either mysqli or PDO to do this.  You might even find that they offer enough that you don't need an intervening class to manage the connection.