Module representing rational numbers Taking inspiration from SICP Exercise 2.1, I decided to implement a module in Haskell for rational numbers.

Some of my concerns include:

Is this idiomatic Haskell?
Besides Num, Eq, Ord, Show, and Read, are there other typeclasses I should be implementing?  I've got "free-floating" functions toInteger' and rationalDiv.  Is there some typeclass that would accommodate them?
Is this a good way to handle division by zero?
How is the code layout and organization?

module Rational'
( toInteger'
, rationalDiv
) where

data Rational' = Rational' { numer :: Integer
                           , denom :: Integer
                           }

instance Num Rational' where
  (+)           = rationalAdd
  (-)           = rationalSub
  (*)           = rationalMul
  abs           = rationalAbs
  signum        = rationalSignum
  fromInteger   = rationalFromInteger

instance Eq Rational' where
  (==)          = rationalEq

instance Ord Rational' where
  compare       = rationalCompare

instance Show Rational' where
  show          = rationalShow

instance Read Rational' where
  readsPrec     = rationalRead

divByZero n = error ("Division by zero: " ++ (show n) ++ "/0")

rationalAdd :: Rational' -> Rational' -> Rational'
a `rationalAdd` b = reduce $ Rational' (an * bd + bn * ad) (ad * bd)
  where (Rational' an ad) = a
        (Rational' bn bd) = b

rationalSub :: Rational' -> Rational' -> Rational'
a `rationalSub` b = a + (Rational' (-bn) bd)
  where bn = numer b
        bd = denom b

rationalMul :: Rational' -> Rational' -> Rational'
a `rationalMul` b = reduce $ Rational' (an * bn) (ad * bd)
  where (Rational' an ad) = a
        (Rational' bn bd) = b

rationalDiv :: Rational' -> Rational' -> Rational'
a `rationalDiv` b = a * (Rational' bd bn)
  where bn = numer b
        bd = denom b

rationalAbs :: Rational' -> Rational'
rationalAbs (Rational' n d) = reduce $ Rational' (abs n) (abs d)

{- You might expect the result of signum to be an Int (namely -1 | 0 | 1), but
 - the Num typeclass requires it to be the same type as the input. -}
rationalSignum :: Rational' -> Rational'
rationalSignum (Rational' n d)
  | n == 0              = 0
  | (n > 0) == (d > 0)  = 1
  | otherwise           = Rational' (-1) 1

rationalFromInteger :: Integer -> Rational'
rationalFromInteger n = Rational' n 1

{- Truncates towards 0 -}
toInteger' :: Rational' -> Integer
toInteger' (Rational' n d) = n `div` d

rationalEq :: Rational' -> Rational' -> Bool
a `rationalEq` b = (a `rationalCompare` b) == EQ

rationalCompare :: Rational' -> Rational' -> Ordering
a `rationalCompare` b
  | (ad == 0)                   = divByZero a
  | (bd == 0)                   = divByZero b
  | (signum ad) == (signum bd)  = (an * bd) `compare` (bn * ad)
  | otherwise                   = (bn * ad) `compare` (an * bd)
  where (Rational' an ad) = a
        (Rational' bn bd) = b

rationalShow :: Rational' -> [Char]
rationalShow a = (show $ numer a) ++ "/" ++ (show $ denom a)

rationalRead :: Int -> [Char] -> [(Rational', [Char])]
rationalRead _ r = [(Rational' (read ns) (read ds), "")]
  where (ns, (_:ds)) = break (=='/') r

reduce :: Rational' -> Rational'
reduce a = Rational' (n `div` common) (d' `div` common)
  where (Rational' n d) = a
        d'
          | (d == 0)    = divByZero n
          | otherwise   = d
        common = (gcd n d') * (signum d')

Not too bad.

You do not export Rational' from the module so no outside code can make one directly
numer and denom will conflict with a similar function name in any other code which imports your module. Perhaps rational'Numer and rational'Denom?
See http://www.haskell.org/tutorial/numbers.html for a good intro to numbers in Haskell. You missed the Fractional class which defines div, recip, and fromRational.
Why are rationalSub and rationalDiv not defined like add and mult? Be consistent.
In rationalSignum your comment says the reader would expect a 0 or 1 Int but the return needs to be of the Rational' type and then you actually return 0 or 1. Be consistent and return Rational' 0 1 and Rational' 1 1
rationalFromInteger could be rationalFromInteger = (flip Rational') 1
I would define reduce like this

reduce (Rational' n 0) = divByZero n
reduce (Rational' n d) = Rational' (n `div` common) (d `div` common)
  where common = (gcd n d) * (signum d)


Speeding up OpenCL matrix-vector multiplication So I'd like to get a performance boost above and beyond standard Julia matrix-vector multiply, using my Intel HD Graphics 4000 1536 MB GPU, but I can't do better than an order of magnitude worse performance.

The kernel I'm using is based on this approach.  I don't know how ArrayFire gets such fast speeds, they're clearly using some sort of black magic, but I don't know what it is.  Anyways here is the test code:

using OpenCL
const cl = OpenCL

function mvmulJulia(M::Int32, N::Int32)
    srand(1)
    A = rand(Float32, M, N)
    x = rand(Float32, N)
    t = @elapsed A * x
    println(t, " seconds for Julia")
    nothing
end

function mvmulGPU(M::Int32, N::Int32, P::Int32)
    @assert N % P == 0

    srand(1)
    TPG = div(N, P)
    A = rand(Float32, M, N)
    x = rand(Float32, N)

    device, ctx, queue = cl.create_compute_context()
    ctx = cl.Context(device)
    queue = cl.CmdQueue(ctx, :profile)

    A_buff = cl.Buffer(Float32, ctx, (:r, :copy), hostbuf=A)
    x_buff = cl.Buffer(Float32, ctx, (:r, :copy), hostbuf=x)
    y_buff = cl.Buffer(Float32, ctx, :w, M)

    const mvmulkernel = """
    kernel void mvmul(int M,
                        int N,
                        int P,
                        int TPG,
                        const global float *A,
                        const global float *x,
                        global float *y)
        {
            int i = get_global_id(0);
            int j = get_global_id(1);
            int tpg = get_local_id(1);

            local float sums[$(TPG)];
            float sum = 0.0f;

            for (int p=0; p<P; p++)
            {
                sum += A[M * (TPG * p + tpg) + i] * x[TPG * p + tpg];
            }

            sums[tpg] = sum;
            barrier(CLK_LOCAL_MEM_FENCE);

            if (j == 0)
            {
                float sumtotal = 0.0f;
                for (int t=0; t<TPG; t++)
                {
                    sumtotal += sums[t];
                }

                y[i] = sumtotal;
            }
        }
    """

    program = cl.Program(ctx, source=mvmulkernel) |> cl.build!
    kernel = cl.Kernel(program, "mvmul")
    evt = cl.call(queue, kernel, (M, N), (1, P), M, N, P, TPG, A_buff, x_buff, y_buff)

    t = round(evt[:profile_duration] * 1e-9, 6)
    println(t, " seconds on GPU")
    y = cl.read(queue, y_buff)
    println(isapprox(y, A * x))
    nothing
end

M = Int32(4000)
N = Int32(300)
P = Int32(50)

mvmulJulia(M, N)
mvmulGPU(M, N, P)

You can try out different M, N, and P at your leisure.

Ok, so I figured out what I was doing wrong.  Basically I was completely misunderstanding the way the work-groups and work-items were supposed to break up.

What I have in the code in my original post is 1 thread for each element of the matrix A, and then I broke each row of this matrix up into work-groups of size P.

What I was supposed to do instead was have P threads per row, which is a total of (M, P) threads, and then collect each row into a single work-group (of size P).  So instead of having N/P work-groups of size P per row, I now only have a single work-group of size P for each row.  Hopefully that makes sense to everyone.

So here is the corrected code.  I didn't put it into functions this time, so just run the script as is.  The script includes both mvmul1 and mvmul2 from that website I linked to in my original post.

using OpenCL
const cl = OpenCL
srand(1)

M = Int32(30)
N = Int32(300)
P = Int32(30)
A = rand(Float32, M, N)
x = rand(Float32, N)

mvmul1 = """
kernel void mvmul1(int M,
                        int N,
                        const global float *A,
                        const global float *x,
                        global float *y)
    {
        int i = get_global_id(0);
        float acc = 0.0f;

        for (int j=0; j<N; j++)
        {
            acc += A[M * j + i] * x[j];
        }

        y[i] = acc;
    }
"""

mvmul2 = """
kernel void mvmul2(int M,
                        int N,
                        int P,
                        const global float *A,
                        const global float *x,
                        global float *y)
    {
        int i = get_global_id(0);
        int j = get_global_id(1);

        local float sums[$(P)];
        float sum = 0.0f;

        for (int q=0; q<(N / P); q++)
        {
            sum += A[M * (j + P * q) + i] * x[j + P * q];
        }

        sums[j] = sum;
        barrier(CLK_LOCAL_MEM_FENCE);

        if (j == 0)
        {
            float sumtotal = 0.0f;
            for (int p=0; p<P; p++)
            {
                sumtotal += sums[p];
            }

            y[i] = sumtotal;
        }
    }
"""

device, ctx, queue = cl.create_compute_context()
ctx = cl.Context(device)
queue = cl.CmdQueue(ctx, :profile)

"""mvmul1"""

A_buff = cl.Buffer(Float32, ctx, (:r, :copy), hostbuf=A)
x_buff = cl.Buffer(Float32, ctx, (:r, :copy), hostbuf=x)
y_buff = cl.Buffer(Float32, ctx, :w, M)

program = cl.Program(ctx, source=mvmul1) |> cl.build!
kernel = cl.Kernel(program, "mvmul1")
evt = cl.call(queue, kernel, M, nothing, M, N, A_buff, x_buff, y_buff)

tout = round(evt[:profile_duration] * 1e-9, 6)
yout = cl.read(queue, y_buff)
t = @elapsed y = A * x
println("mvmul1 is ", round(t / tout, 3), " times as fast as Julia. ", isapprox(yout, y))

"""mvmul2"""

A_buff = cl.Buffer(Float32, ctx, (:r, :copy), hostbuf=A)
x_buff = cl.Buffer(Float32, ctx, (:r, :copy), hostbuf=x)
y_buff = cl.Buffer(Float32, ctx, :w, M)

program = cl.Program(ctx, source=mvmul2) |> cl.build!
kernel = cl.Kernel(program, "mvmul2")
evt = cl.call(queue, kernel, (M, P), (1, P), M, N, P, A_buff, x_buff, y_buff)

tout = round(evt[:profile_duration] * 1e-9, 6)
yout = cl.read(queue, y_buff)
t = @elapsed y = A * x
println("mvmul2 is ", round(t / tout, 3), " times as fast as Julia. ", isapprox(yout, y))

You'll notice that for M < N, mvmul2 does well while mvmul1 doesn't.

But if you change M, N and P such that M >= N, then mvmul1 does well while mvmul2 doesn't, interesting


Web service handler class I've implemented a class to perform a couple of tasks through a web service. I'll be really keen if you can take a look at, give me your professional opinion.

using MWM.NavisionWebService.WebReferenceTest;
using NLog;
using System;
using System.Net;
using System.Configuration;

namespace MWM.NavisionWebService
{
    public class WebServiceAccess : IDisposable
    {
        private readonly Logger _logger = LogManager.GetCurrentClassLogger();
        private readonly string _user;
        private readonly string _pass;
        private readonly NSO _nsoWebService;

    public WebServiceAccess()
    {
        _nsoWebService = new NSO { Credentials = GetCredentials() };
        _user = ConfigurationManager.AppSettings["CghWebServiceUserName"];
        _pass = ConfigurationManager.AppSettings["CghWebServicePassword"];
    }

    public void UpdateTechnician(string serviceOrderId, string technicianErpId)
    {
        try
        {
            int errorCode = _nsoWebService.UpdateResource(serviceOrderId, technicianErpId);

            if (errorCode != 0)
                HandleErrors(errorCode);
            else
                _logger.Trace(String.Format("Navision Technician updated to {0} for service order {1}.", technicianErpId, serviceOrderId));
        }
        catch (Exception ex)
        {
            _logger.Error("Error updating a technician in Navision. Error: " + ex.Message);
            throw;
        }
    }

    public void UpdateAppointmentDate(string serviceOrderId, DateTime startDate)
    {
        try
        {
            int errorCode = _nsoWebService.UpdateTime(serviceOrderId, startDate, startDate);

            if (errorCode != 0)
                HandleErrors(errorCode);
            else
                _logger.Trace(String.Format("Navision appointment updated to {0} for service order {1}.", startDate, serviceOrderId));
        }
        catch (Exception ex)
        {
            _logger.Error("Error updating an appointment in Navision. Error: " + ex.Message);
            throw;
        }
    }

    private ICredentials GetCredentials()
    {
        return new NetworkCredential(_user, _pass);
    }

    private void HandleErrors(int errorCode)
    {
        _logger.Error("Error updating Navision through the web service. Code: " + errorCode + " Message: " + GetErrorCodeDescription(errorCode));
    }

    private string GetErrorCodeDescription(int errorCode)
    {
        switch (errorCode)
        {
            case 0:
                return "OK";
            case 1:
                return "Could not find CCC Order No.";
            case 51:
                return "Could not write table.";
            default:
                return "Error code not found.";
        }
    }

    public void Dispose()
    {
        // Dispose from unmanaged resources
        _nsoWebService.Dispose();
    }
}
}

Your class will be difficult to test since you don't implement Inversion of Control. In particular due to this line in the constructor:

_nsoWebService = new NSO { Credentials = GetCredentials() };

But the same applies to Logger _logger = LogManager.GetCurrentClassLogger();: what if you want to write a unit test to check whether the correct message is logged?

There are several instances of this:

if (errorCode != 0)
   HandleErrors(errorCode);
else
   _logger.Trace(/* message */);

I'd rather see this reversed, because testing for a negative makes things more complicated than they should be IMHO (I've also added brackets, something that you should always do):

if (errorCode == 0)
{
   _logger.Trace(/* message */);
}
else
{
   HandleErrors(errorCode);
}

Considering that this log action is the only thing that happens, you could even do this:

if (errorCode == 0)
{
   _logger.Trace(/* message */);
   return;
}

HandleErrors(errorCode);

Your errorCode shouldn't be an int, but an enum with a description.

Isn't there an overload for _logger.Trace() and _logger.Error() that functions the same as string.Format()? Seems there is if you're using NLog.

NSO doesn't follow Microsoft's rules: capitalize only the first character of acronyms with three or more characters. But it is also a class name that doesn't tell me anything.

I also don't think _user and _pass should be in the WebServiceAccess class, nor GetCredentials(). These things belong to NSO.

You do not take into account that an exception may have an inner exception:

_logger.Error("Error updating a technician in Navision. Error: " + ex.Message);

HandleErrors doesn't really do any error handling, now does it? It merely logs the error.


VBA script too slow - Checks for duplicate school enrollments and keeps more recent enrollment I have over 260k lines of data. This code is likely to take more than 30hrs to run. Would like help to speed this up. Or is there a way to use something else like PowerQuery to accomplish this? I am very new so please break it down clearly.

There are 260k rows of student achievement data. Some students may have multiple lines with the same school, meaning that they have math, reading, science achievement at one school (3 rows of data, 1 student, 1 school). But then if they transfer schools, they will have two groups of data (1 student, 2 schools). I need to keep only the information for the most recent school. I want to delete based on date.

Sub RecentEnrollment()

Dim lrow As Long, lcol As Long, frow As Long
Dim i As Integer, r As Long, c As Long
Dim num As Long, pos As Long
Dim myrng As Variant
Dim namerng As Variant
Dim schrange As Variant
Dim RowFirst As Double, _
    RowLast As Double ' Double is used here to handle the large number of
            'rows. Integer and Long are small Bit sizes.

Application.ScreenUpdating = False

With Worksheets(1)
    'Find the last non-blank cell in Column "Student ID"
    lrow = .Cells(Rows.Count, 1).End(xlUp).Row
    lcol = .Cells(1, Columns.Count).End(xlToLeft).Column

    MsgBox "Last Row: " & lrow

    'Define the range to search for duplicates. myrng is student id-name
    'combos and schrange is studentID-sch combos. You may need to change the
    'sheet name.

For r = 2 To lrow

    'Define the range for the studentId-Name column to search for number of
    'times each appears 'num and it's position in the list 'pos

    Set namerng = .Range(Cells(r, 5), Cells(lrow, 5))

   'Find the first instance of the student name in the file

    RowFirst = .Cells.Find(What:=.Cells(r, 2), LookAt:=xlWhole,SearchDirection:=xlNext, MatchCase:=False).Row

   'Find the last instance of the student name in the file

    RowLast = .Cells.Find(What:=.Cells(r, 2), LookAt:=xlWhole, SearchDirection:=xlPrevious, MatchCase:=False).Row

    Set myrng = .Range(Cells(RowFirst, 5), Cells(RowLast, 5))
    Set schrange = .Range(Cells(RowFirst, 3), Cells(RowLast, 3))

For c = 1 To 12
    If Application.WorksheetFunction.CountIf(myrng, Cells(r, 5).Value) >= 1 Then
     'How many times does the student name-school combo appear?
      schs = Application.WorksheetFunction.CountIf(schrange, Cells(r, 3).Value)

     'How many times does the student name appear in the file?
      num = Application.WorksheetFunction.CountIf(myrng, Cells(r, 5).Value)

     'What is the position of this student record relative to the duplicated
      records
      pos = Application.WorksheetFunction.CountIf(namerng, Cells(r, 5).Value)

      'For every student record, print the most recent exit date for all
      enrollments
        MaxDate = WorksheetFunction.Max(.Range(.Cells(RowFirst, 9), .Cells(RowLast, 9)))
        Cells(r, 10) = MaxDate

    Else
        'Print most recent exit date
            Cells(r, 10) = MaxDate

    End If

        If num > schs Then

            If Cells(r, 9) = MaxDate Then

                Cells(r, 11) = "Keep"

            Else

                Cells(r, 11) = "Delete"

        End If

    End If

    Cells(r, 24) = Time
Next c

Next r

End With
Application.ScreenUpdating = True

End Sub

I had some difficulty in getting your code to run. There are a few variables that weren't defined (see comment #1 below). Additionally, you're working With Worksheets(1), but there are places where you're referencing Cells instead of .Cells. The former references whatever sheet is currently active, whereas the latter directly references Worksheets(1). This can be a subtle source of many problems, and it's something to watch out for.

Your current solution using Find , CountIf, and Max relies heavily on Excel worksheet functions and interactions with the data directly on the worksheet. With larger data sets, this can be a big source of the slowness in your code.

I'm suggesting a different approach which converts the entire student data range to a memory-based array and works from there. Working completely out of memory is much quicker and side-steps the need for Excel access individual cells and ranges.

So, a few comments on your code and implementation:

Please use Option Explicit in all your VBA code. It will help keep your sanity longer.
Declare your variables close to where you're first using them. It's easier to see exactly what you intend by the variable at that point.
Read up on using memory-based arrays in VBA.

I've split the solution into two parts: setup and execution. The setup routine determines the area of data to be used -- in this case it's the range studentData.

 Option Explicit

Sub ProcessStudentRecords()
    Dim studentWS As Worksheet
    Set studentWS = ThisWorkbook.Sheets("Sheet1")

    Dim lastRow As Long
    Dim lastCol As Long
    With studentWS
        lastRow = .Cells(.Rows.Count, 1).End(xlUp).Row
        lastCol = .Cells(1, .Columns.Count).End(xlToLeft).Column
    End With
    Dim studentData As Range
    Set studentData = studentWS.Range("A2").Resize(lastRow - 1, lastCol)

    Dim filteredStudents As Variant
    filteredStudents = FilterForRecentEnrollment(studentData)

    '--- now set our destination...
    Dim destinationWS As Worksheet
    Set destinationWS = ThisWorkbook.Sheets("Filtered")

    '--- ... and define the range where it's going to the correct size
    Dim destRange As Range
    Set destRange = destinationWS.Range("A2").Resize(UBound(filteredStudents, 1), UBound(filteredStudents, 2))

    '--- copy the filtered results to the range and we're done
    destRange = filteredStudents
End Sub

You can see that after the student data is filtered, the function returns a memory-based array. It's a simple couple of steps to write that data back to a worksheet range.

The function FilterForRecentEnrollment does all the heavy lifting by copying your original data into the memory-based array. Your data is large (at 260,000+ rows), but this should still be quicker than relying on Excel moving data directly in and out of the cells.

Moving the data into a memory-based array is quick and easy:

    '--- work with the student data in a memory array for speed
    Dim studentData As Variant
    studentData = studentRange

A Dictionary in VBA is an object that actively manages large amounts of data and you to very quickly access any item in the collection with a unique Key. A key is simply a string. You create the key by using information from your data either a single unique field or a combination of fields that will be unique. As best as I could tell, a "unique" identifier for your data is a combination of the Student ID and the Student Name. Using a key made from these fields, you can test if that entry already exists (and then test the dates).

So, in order to save data in the dictionary, we'll create a simple data-only class called StudentInfo. In your VBA Editor, right-click in the VBA Project "tree" where you see the Microsoft Excel Objects and Modules, then select "Insert-->Class Module". In the Properties window, name the class StudentInfo. This class looks like this:

'--- Class Module:  StudentInfo
Option Explicit

'--- ideally, these should be properties
Public SchoolName As String
Public StudentID As String
Public IDWithSchool As String
Public Student As String
Public IDWithName As String
Public Level As String
Public Grade As String
Public Status As String
Public Exited As Date

Public Property Get NumberOfFields() As Long
    NumberOfFields = 9
End Property

So what we'll do in the Filter routine is to scan a row of your student data, capture all the information into an StudentInfo object, and store it in the Dictionary. Of course, we'll only store the latest exit date of info in the dictionary. It looks like this:

Set rowInfo = New StudentInfo
rowInfo.SchoolName = studentData(i, 1)
rowInfo.StudentID = studentData(i, 2)
rowInfo.IDWithSchool = studentData(i, 3)
rowInfo.Student = studentData(i, 4)
rowInfo.IDWithName = studentData(i, 5)
rowInfo.Level = studentData(i, 6)
rowInfo.Grade = studentData(i, 7)
rowInfo.Status = studentData(i, 8)
rowInfo.Exited = studentData(i, 9)

'--- create a key value that is a unique combination of values that defines
'    a single (non-duplicate) entry
Dim dictionaryKey As String
dictionaryKey = rowInfo.StudentID & rowInfo.Student
If Not students.Exists(dictionaryKey) Then
    students.Add dictionaryKey, rowInfo
Else
    '--- this entry exists, but which has a later exit date?
    Dim existingInfo As StudentInfo
    Set existingInfo = students(dictionaryKey)
    If rowInfo.Exited > existingInfo.Exited Then
        '--- the new row's exit date is later than the existing row,
        '    so replace it
        Set students(dictionaryKey) = rowInfo
    Else
        '--- keep the existing info and ignore the current row
    End If
End If

All that code is inside a loop to catch all the student information and filter it.

Now that we have a collection (Dictionary) of filtered student data, we know exactly how many rows and columns in the data. So we can create a memory-based array and copy the data into that. Again, we're working directly in memory, so it's much faster than working with the sheet for large amounts of data.

'--- now that we have the filtered student data, we'll create an array
'    to return the data
Dim filteredData() As Variant
ReDim filteredData(1 To students.Count, 1 To rowInfo.NumberOfFields)

'--- now populate!
Dim key As Variant
i = 1
For Each key In students.Keys
    Set rowInfo = students(key)
    filteredData(i, 1) = rowInfo.SchoolName
    filteredData(i, 2) = rowInfo.StudentID
    filteredData(i, 3) = rowInfo.IDWithSchool
    filteredData(i, 4) = rowInfo.Student
    filteredData(i, 5) = rowInfo.IDWithName
    filteredData(i, 6) = rowInfo.Level
    filteredData(i, 7) = rowInfo.Grade
    filteredData(i, 8) = rowInfo.Status
    filteredData(i, 9) = rowInfo.Exited
    i = i + 1
Next key

I tested this on the limited sample data you posted and the filtering works. Some decisions you can make would be to adjust the student information you're actually saving in the class and then transferring to the destination. You'll also possibly decide that a different combination of fields is necessary for a unique key. You'll have to determine this based on your real data set.

Here's the full code for the module (don't forget to create the class, as above):

Option Explicit

Sub ProcessStudentRecords()
    Dim studentWS As Worksheet
    Set studentWS = ThisWorkbook.Sheets("Sheet1")

    Dim lastRow As Long
    Dim lastCol As Long
    With studentWS
        lastRow = .Cells(.Rows.Count, 1).End(xlUp).Row
        lastCol = .Cells(1, .Columns.Count).End(xlToLeft).Column
    End With
    Dim studentData As Range
    Set studentData = studentWS.Range("A2").Resize(lastRow - 1, lastCol)

    Dim filteredStudents As Variant
    filteredStudents = FilterForRecentEnrollment(studentData)

    '--- now set our destination...
    Dim destinationWS As Worksheet
    Set destinationWS = ThisWorkbook.Sheets("Filtered")

    '--- ... and define the range where it's going to the correct size
    Dim destRange As Range
    Set destRange = destinationWS.Range("A2").Resize(UBound(filteredStudents, 1), UBound(filteredStudents, 2))

    '--- copy the filtered results to the range and we're done
    destRange = filteredStudents
End Sub

Public Function FilterForRecentEnrollment(ByRef studentRange As Range) As Variant
    '--- works through each row in the data range and captures a new
    '    dictionary entry for each new student/school combination.
    '    the value in the dictionary is the information from that row

    '--- work with the student data in a memory array for speed
    Dim studentData As Variant
    studentData = studentRange

    '--- create a dictionary that will help determine if there are duplicates
    Dim students As Scripting.Dictionary
    Set students = New Scripting.Dictionary

    Dim rowInfo As StudentInfo
    Dim i As Long
    For i = LBound(studentData, 1) To UBound(studentData, 1)
        Set rowInfo = New StudentInfo
        rowInfo.SchoolName = studentData(i, 1)
        rowInfo.StudentID = studentData(i, 2)
        rowInfo.IDWithSchool = studentData(i, 3)
        rowInfo.Student = studentData(i, 4)
        rowInfo.IDWithName = studentData(i, 5)
        rowInfo.Level = studentData(i, 6)
        rowInfo.Grade = studentData(i, 7)
        rowInfo.Status = studentData(i, 8)
        rowInfo.Exited = studentData(i, 9)

        '--- create a key value that is a unique combination of values that defines
        '    a single (non-duplicate) entry
        Dim dictionaryKey As String
        dictionaryKey = rowInfo.StudentID & rowInfo.Student
        If Not students.Exists(dictionaryKey) Then
            students.Add dictionaryKey, rowInfo
        Else
            '--- this entry exists, but which has a later exit date?
            Dim existingInfo As StudentInfo
            Set existingInfo = students(dictionaryKey)
            If rowInfo.Exited > existingInfo.Exited Then
                '--- the new row's exit date is later than the existing row,
                '    so replace it
                Set students(dictionaryKey) = rowInfo
            Else
                '--- keep the existing info and ignore the current row
            End If
        End If
    Next i

    '--- now that we have the filtered student data, we'll create an array
    '    to return the data
    Dim filteredData() As Variant
    ReDim filteredData(1 To students.Count, 1 To rowInfo.NumberOfFields)

    '--- now populate!
    Dim numFields As Long
    numFields = rowInfo.NumberOfFields

    Dim key As Variant
    i = 1
    For Each key In students.Keys
        Set rowInfo = students(key)
        filteredData(i, 1) = rowInfo.SchoolName
        filteredData(i, 2) = rowInfo.StudentID
        filteredData(i, 3) = rowInfo.IDWithSchool
        filteredData(i, 4) = rowInfo.Student
        filteredData(i, 5) = rowInfo.IDWithName
        filteredData(i, 6) = rowInfo.Level
        filteredData(i, 7) = rowInfo.Grade
        filteredData(i, 8) = rowInfo.Status
        filteredData(i, 9) = rowInfo.Exited
        i = i + 1
    Next key
    FilterForRecentEnrollment = filteredData
End Function


Singleton base class I just wrote a singleton base class for a hobby project I'm currently working on. Python is not my first language, which is why I am not sure if everything is done the pythonic way.

I also used multiple resources/tutorials/articles for Python2 and 3. My singleton is supposed to be Python3 compatible. The implementation should be thread safe, which is why I enter a lock while creating the instance.

To allow custom initializations I decided to let the user provide a create_class function where the object could be created and initialized. The base class is more like a "singleton-factory" than a "classical" singleton where the object itself provides both (logic and singleton).

Please tell me what can I do better, more efficient, and more pythonic.

#!/usr/bin/env python3
# -*- coding: utf-8 -*-

"""
  this module implements provides a baseclass for
  a singleton.

  it allows you to simply create singleton objects
  by overriding the PyRlSingleton class.

  it gives you access to the singleton instance in
  a explicit way via a "get_instance()" function.

  if you mistakingly try to create your singleton
  class it raises an exception telling you that this
  is not allowed.

  you have to provide a "create_instance()" function
  which creates your singleton implementation; this
  is automatically called when the singleton is used
  the first time.

  the creating process is thread safe, so you can use
  it in a multithreading environment.

  see the "main()" function for a simple example.

  have fun!
"""

import _thread # allocate_lock()

class PyRlSingletonMeta(type):
  def __call__(cls, *args, **kwargs):
    """
      this method disallows creation of a
      singleton via a ClassName() call
    """
    raise Exception("not allowed, use get_instance()")

class PyRlSingleton(object, metaclass = PyRlSingletonMeta):
  __lock = _thread.allocate_lock()
  __inst = None

  @classmethod
  def create_instance(cls):
    """
      this is the default create_instance method
      you should provide your own via overriding.
    """
    raise Exception("create_instance() missing in {0}".format(cls.__name__))

  @classmethod
  def get_instance(cls):
    """
      this classmethod returns always the same instance
      of cls. it creates an object via calling the
      create_instance class method when it is first
      called.
    """
    if cls.__inst == None:
      with cls.__lock:
        if cls.__inst == None:
          cls.__inst = cls.create_instance()
    return cls.__inst

def main():
  """ test the singleton in a simple example, a logger """

  class LoggerImpl(object):
    """ my class supposed to be a singleton """
    def log(self, text):
      print(text)

  class Logger(PyRlSingleton):
    """
      the singleton itself. i have to override the
      create_instance() class method and let it return
      my LoggerImpl.
    """
    @classmethod
    def create_instance(cls):
      """ create and initialize your implementation here. """
      return LoggerImpl()

  Logger.get_instance().log("hello world")
  l1 = Logger.get_instance()
  l2 = Logger.get_instance()
  if l1 != l2:
    raise Exception("what?")
  if l1 != Logger.get_instance():
    raise Exception("ugh!?")

if __name__ == '__main__':
  main()

This line isn't needed in Python 3:

# -*- coding: utf-8 -*-

so you should probably just remove it. UTF-8 is assumed.

Your docstring is indented - why? It's not technically wrong, but it seem like a really arbitrary choice.

I'm not sure how to say this, but none of your sentences are capitalized. If this were a professional code-base, I would absolutely require good grammar and care of documentation. It is, after all, the primary text that represents the code.

I see _thread - you should be using the main threading interface. In particular, use threading.Lock instead of _thread.allocate_lock.

Since you're using Python 3, don't inherit from object - it's a no-op.

You have Logger.get_instance() return an instance of a different class. By this point, one should probably just make a get_logger function instead. Since that's not ideal, perhaps just overload __new__. You can do this with a metaclass, but a decorator is much simpler:

import threading
from functools import wraps

def singleton_new(make_instance):
    lock = threading.Lock()
    instance = None

    @wraps(make_instance)
    def __new__(cls):
        nonlocal instance

        if instance is None:
            with lock:
                if instance is None:
                    instance = make_instance(cls)

        return instance

    return __new__

This is as simple to use as

class Logger:
    @singleton_new
    def __new__(cls):
        return super().__new__(cls)

    def log(self, text):
        print(text)

and doesn't even affect the class' attributes, due to the magic of closures. One could use a class decorator instead, but I think this looks nicest. Note that this also allows usage on classmethods like

@classmethod
@singleton_new
def get_instance(cls):
    return super().__new__(cls)

Past that, I advise you to stick to PEP 8 where you can, which primarily means minor changes for your code.

On a design level, I strongly suggest rethinking using singletons at all. Globals are almost always better in such cases, and for the case of logging singletons aren't apropriate anyway (look instead at neat things like Logbook).

Even if you do conclude, for some reason, that you want singletons, at least consider the Borg pattern instead, which is mildly less aggrivating.


Extracting book information from a web page, with possible missing attributes I'm just starting to code and I found a solution to first check if a div class exist before to try to get_text (and avoid an error if it doesn't). But the code doesn't looks really nice repeating the same parameters.

The code is working but if there is something I could learn, it would be better.

headers = {'User-Agent': 'Mozilla/5.0'}
response = requests.get(url_book, headers=headers)
soup = BeautifulSoup(response.text, 'html.parser')

book_info = soup.find_all('div', {'class': 'jr_pgContainer jr_itemDetail hReview-aggregate'})

for content in book_info:
    title = content.find('span', {'class': 'fn'}).get_text()
    if content.find('div', {'class': 'fieldRow jr_autorestr'}):
        author = content.find('div', {'class': 'fieldRow jr_autorestr'}).get_text().replace("Autore","")
    else:
        author = 'None'
    if content.find('div', {'class': 'fieldRow jr_editore'}):
        editor = content.find('div', {'class': 'fieldRow jr_editore'}).get_text().replace("Casa editrice","")
    else:
        editor = 'None'
    if content.find('div', {'class': 'fieldRow jr_narrstrcontinente'}):
        classification = content.find('div', {'class': 'fieldRow jr_narrstrcontinente'}).get_text().replace("Classificazione","")
    else:
        classification = 'None'
    if content.find('div', {'class': 'contentFulltext'}):
        description =  content.find('div', {'class': 'contentFulltext'}).get_text().replace("\t","").replace("\n","")
    else:
        description = 'None'
    if content.find('h4', {'class': 'jr_reviewTitle'}):
        title_review = content.find('h4', {'class': 'jr_reviewTitle'}).get_text()
    else:
        title_review = 'None'
    if content.find('div', {'class': 'jr_reviewComment'}):
        review = content.find('div', {'class': 'jr_reviewComment'}).get_text().replace("\t","").replace("\n\n","")
    else:
        review = 'None'

#CREATE AND PRINT DICTIONARY
book_detail = {'title: ': title, 'author: ': author, 'editor: ': editor, 'classification: ': classification, 'description: ': description, 'revew title: ': title_review, 'review: ': review }

for keys,values in book_detail.items():
    print(keys)
    print(values)
    print("************************************")

The obvious thing to do when you have repeated functionality is to extract it into a function. In this case, it looks like the parameters for the function are the content itself, the element type to find, the classname, and a list of text to replace.

You can also tidy things up by only doing the find once.

def find_text(content, element, classname, *replacements):
    item = content.find(element, {'class': classname})
    if item:
        text = item.get_text()
        for replacement in replacements:
            text = text.replace(replacement, "")
        return text
    else:
        return 'None'

Now your main loop looks like:

for content in book_info:
    title = find_text('span', 'fn')
    author = find_text('div', 'fieldRow jr_autorestr', "Autore")
    classification = find_text('div', 'fieldRow jr_narrstrcontinente', "Classificazione")
    ... etc ....


Django ListView with MySQL query for the queryset (Note: Even if you don't know Django, it's probably the query that needs work anyway, so you probably don't need to know Django/Python)

I have a ListView that displays all the entries in a mapping table between the pizza table and the topping table. Unfortunately, I can't just use Django's helpfulness and just give it the model to use (I'm skipping why for the sake of brevity), and have to use get_queryset instead. Unfortunately again, the load time is slow - there are almost 700 pizzas in the table and I need to use it in my sql query to order by pizza name. Here's what I'm currently doing:

class MapPizzaToppingListView(ListView):
    model = MapPizzaTopping
    paginate_by = 20
    template_name = 'onboardingWebApp/mappizzatopping_list.html'

    def get_queryset(self):
        cursor = connections["default"].cursor()
        cursor.execute("""SELECT map.*
                          FROM map_pizza_topping as map
                          INNER JOIN pizza
                              ON (map.pizza_id = pizza.id)
                          ORDER BY pizza.name""")
        queryset = dictfetchall(cursor)
        for row in queryset:
            pizza = Pizza.objects.get(id=row['pizza_id'])
            topping = Topping.objects.get(id=row['topping_id'])
            row['pizza_name'] = "%s (%s)" % (pizza.name, pizza.merchant.name)
            row['topping_name'] = topping.name
        return queryset

It takes a few seconds to load now, but we're going to be adding tons more pizzas eventually, so this really needs to be sped up. Unfortunately, my MySQL knowledge, though decent, isn't really to a point where I know how to make it better. What can I do to improve it?

Some more information that could be useful - the map_pizza_topping table just has a pizza_id column, a topping_id column, and a quantity column. The pizza table just has name, price, customer_id, and merchant_id columns, and the topping table just has name and is_dairy columns.

This is slow NOT because of your SQL.
The SQL in the post is a simple JOIN,
on id columns that are indexed by Django by default.

Without doubt, the cause of the slowness is this:

for row in queryset:
    pizza = Pizza.objects.get(id=row['pizza_id'])
    topping = Topping.objects.get(id=row['topping_id'])

For every row returned by the first query,
you load a Pizza and a Topping.
Django might be smart enough to not re-fetch the same topping multiple times,
but for every unique Pizza and every unique Topping,
Django will have to run an additional query.
With 700 pizzas, you're looking at at least 700 queries, which is clearly not efficient.

  Unfortunately, I can't just use Django's helpfulness and just give it the model to use (I'm skipping why for the sake of brevity)

It would be best to get to the bottom of this.
Take a look at the documentation of the select_related method on querysets,
I think the solution for your case should be somewhere around there.
I really think there's a solution within the realm of "Django's helpfulness",
without resorting to such queries, you just need to figure it out.
(You might want to ask some questions on that on stackoverflow.com)

Another workaround might be to stop calling Pizza.objects.get and Topping.objects.get:
include in your query all the fields you need for pizzas and toppings,
and build simple dictionaries from them.
Rest assured, when you stop calling Pizza.objects.get and Topping.objects.get 700 times you will notice a massive speed improvement.


Replacing English words in a string with their Spanish equivalents by dictionary lookup I want to improve my code's performance and running time, looking for write my loops better.

For example, I have a dictionary that contains words as keys, and their translation in Spanish as values.

{
    'Hello' : 'Hola',
    'Goodbye' : 'Adios',
    'Cheese' : 'Queso'
}

I also have a given English sentence, and I want to iterate over any word in my dict and replace it with the Spanish translation.
For this scenario I consider that up to one word could be exist in the given sentence.

I wrote a basic code that do that, but I am not sure that it is best practice:

words_list = {
        'Hello' : 'Hola',
        'Goodbye' : 'Adios',
        'Cheese' : 'Queso'
    }
sentence = "Hello, I want to talk Spanish"

for english_word in words_list.keys():
    if english_word in sentence:
        sentence = sentence.replace(english_word, words_list[english_word])
        break

print sentence

How can I write it better?

Problems

not properly handling words, just substrings. Currently, you are using str.replace() that would replace substrings in a string, not complete words, which means that it would behave incorrectly when your replacement is a part of another word - e.g. "Cheese" would be replaced from "Cheesecake" becoming "Quesocake"
just a single substring is replaced. you have that break statement which basically stops all the replacements after the first one occurred
handling words in different cases. You are also not properly handling the titled or upper-cased words


How to refactor old browser detection JavaScript? I'm looking to make this code more sensible, while making sure it still works as intended. The alert message at the bottom should trigger when the user is running a version of Microsoft Internet Explorer less than IE 9. But there's a lot of incomprehensible shorthand in the main code, that I need to be refactored, and simplified into long-hand, so it will make more sense to me.

Here is the code:

/*
----------------------------------------------------------
A short snippet for detecting versions of IE in JavaScript
without resorting to user-agent sniffing
----------------------------------------------------------

If you're not in IE (or IE version is less than 5) then:

ie === undefined

If you're in IE (>=5) then you can determine which version:

ie === 7; // IE7

Thus, to detect IE:

if (ie) {}

And to detect the version:

ie === 6 // IE6
ie > 7 // IE8, IE9 ...
ie < 9 // Anything less than IE9

----------------------------------------------------------
*/

/* IE Detection Snippet. DO NOT EDIT [BELOW] THIS LINE. [BEGIN] */

var ie = (function()
{
    var undef;
    var v = 3;
    var div = document.createElement('div');
    var all = div.getElementsByTagName('i');

    while
        (
            div.innerHTML = '',
            all[0]
        );

    return v > 4 ? v : undef;

}());

/* IE Detection Snippet. DO NOT EDIT [ABOVE] THIS LINE. [END] */

if (ie)
{
    if (ie < 9)
        {
            alert( "You are running an older version of Microsoft Internet Explorer:\r\nVersion " + ie.toFixed(1) + ", which is NOT supported.\r\n\r\nPlease upgrade your web browser to its latest version, or if you're still using Windows XP, to Google Chrome, Mozilla Firefox, Apple Safari, or Opera, to view this website, properly.\r\n\r\nClick \"OK\" to read instructions on how to download and install a newer, more compatible, web browser.");
            window.parent.location = "/libs/old_browser_detect/current/upgrade_browser.php";
        };
};

Original Source: https://j11y.io/javascript/detect-ie-in-js-using-conditional-comments/

Also, is it possible to change the variable names to ones, both, more relevant, and unique, to prevent coding conflicts?

@Przemek, Will these changes work?

/*

----------------------------------------------------------
A short snippet for detecting versions of IE in JavaScript
without resorting to user-agent sniffing
----------------------------------------------------------

If you're not in IE (or IE version is less than 5) then:

ieVersion === undefined

If you're in IE (>=5) then you can determine which version:

ieVersion === 7; // IE7

Thus, to detect IE:

if (ieVersion) {}

And to detect the version:

ieVersion === 6 // IE6
ieVersion > 7 // IE8, IE9 ...
ieVersion < 9 // Anything less than IE9

----------------------------------------------------------

*/

/* IE Detection Snippet. DO NOT EDIT [BELOW] THIS LINE. [BEGIN] */

var ieVersion = (function()
    {
        var elmt = document.createElement('span');
        var version = 3;

        do
            {
                elmt.innerHTML = '';
            }
        while (elmt.getElementsByTagName('span')[0] && version <= 9);

        return version > 4 ? version : false;
    }());

/* IE Detection Snippet. DO NOT EDIT [ABOVE] THIS LINE. [END] */

if ((ieVersion) && (ieVersion < 9))
    {
        alert( "You are running an older version of Microsoft Internet Explorer:\r\nVersion " + ieVersion.toFixed(1) + ", which is NOT supported.\r\n\r\nPlease upgrade your web browser to its latest version, or if you're still using Windows XP, to Google Chrome, Mozilla Firefox, Apple Safari, or Opera, to view this website, properly.\r\n\r\nClick \"OK\" to read instructions on how to download and install a newer, more compatible, web browser.");
        window.parent.location = "/libs/old_browser_detect/current/upgrade_browser.php";
    };

Remarks

It sure is an interesting, hackery approach. Taken into account that this code must be compatible with archaic browsers, I would say it's rather fine. Few remarks:

For readability while() with no body could be rewritten to do {...} while().
Fail safe v <= 9 to avoid potential infinite loop could be added, in case something would change regarding browsers' behavior.
Naming could be better: div could become element to abstract used tag from the purpose of this variable's existence, v could become less-cryptic version.
Variable undef is unnecessary, it's only use occurrence can be replaced with false.
Formatting issues.
Inconsistent indentation, bracket policy and quotation marks.
Unnecessary addition of level of indentation; below ifs can be combined:

if (ie)
{
    if (ie < 9)
        {

If you would use single quotation marks in your alert(), you would have to perform only one escaping, instead of two.
window.parent should be replaced with window.top since parent could be iframe as well.

Rewrite

(untested for positive cases)

var ieVersion = (function() {
  var element = document.createElement('span');
  var version = 3;

  do {
    element.innerHTML = '';
  }
  while (element.getElementsByTagName('span')[0] && version <= 9);

  return version > 4 ? version : false;
}());

if (ieVersion && ieVersion < 9) {
  alert('You are running an older version of Microsoft Internet Explorer:\r\nVersion ' + ie.toFixed(1) + ', which is NOT supported.\r\n\r\nPlease upgrade your web browser to its latest version, or if you\'re still using Windows XP, to Google Chrome, Mozilla Firefox, Apple Safari, or Opera, to view this website, properly.\r\n\r\nClick "OK" to read instructions on how to download and install a newer, more compatible, web browser.');
  window.top.location.href = '/libs/old_browser_detect/current/upgrade_browser.php';
}

Different approaches

Feature-detection

I think it would be best to achieve your goal by using regular feature-detection based method:

var ieVersion = (function getLegacyIEVersion() {
  if (!document.compatMode) { return 5; }
  if (!window.XMLHttpRequest) { return 6; }
  if (!document.querySelector) { return 7; }
  if (!document.addEventListener) { return 8; }
  //if (!window.atob) { return 9; }
  return false;
})();

var IEThreshold = 9;
if (ieVersion !== false && ieVersion < IEThreshold) {
  console.log('Legacy IE');
}
else {
  console.log('Not a legacy IE');
}
/* Demo console styling */
.as-console-wrapper { top: 0; }

Conditional compilation

Another way of achieving the same, would be to use conditional compilation to detect JS engine version. Change ieVersion() function from previous code to:

var ieVersion = /*@cc_on (function() {
  switch(@_jscript_version) {
    case 1.0: return 3; break;
    case 3.0: return 4; break;
    case 5.0:
    case 5.1: return 5; break;
    case 5.5: return 5.5; break;
    case 5.6: return 6; break;
    case 5.7: return 7; break;
    case 5.8: return 8; break;
    case 9: return 9; break;
    case 10: return 10; break;
  }
})() || @*/ false;

Note however, that this method uses @_jscript_version which returns version of JS engine of the browser itself, while in cases of running in compatibility mode it would be desirable to get an actually used engine version, instead. Also, minifiers would get rid of this code, since for browsers other than IE < 11 this is just a comment.


Reordering a list in python so that no neighboring elements are equal Basically I need to order a list, so that an element is not the same as the element that comes after or before it.

For example [1,2,2,3,1,3,3] should come out as [3,1,2,3,1,2,3], [3,2,3,1,2,3,1] or [2,1,3,1,3,2,3] and so on.

I searched for a solution but couldn't find one so I wrote this function myself.

def unorder(alist):
    multi_lvl_list = []
    unord_list = []

    alist.sort()

    # makes a new list that holds lists where same elements
    # are grouped together
    start = 0
    for i in range(len(alist)):
        if i == len(alist)-1:
            if alist[i] != alist[i-1]:
                multi_lvl_list.append([alist[i]])
            else:
                multi_lvl_list.append(alist[start:])
        elif alist[i] != alist[i+1]:
            multi_lvl_list.append(alist[start:i+1])
            start = i+1
        i += 1

    multi_lvl_list.sort(key=len)

    '''
    goes over every list and pops out one element to a third list
    if there are many same elements they will be put
    to the end of the list
    '''
    while len(multi_lvl_list) > 0:
        for idx, val in enumerate(multi_lvl_list):
            unord_list.append(val.pop())
            if len(val) == 0:
                del multi_lvl_list[idx]

    '''
    go over the list in reverse. if x == x-1 try to put x in the
    beginning of the list. becourse there are
    more likely different elements
    '''
    for i in range(len(unord_list)-1, -1, -1):
        if unord_list[i] == unord_list[i-1]:
            for j in range(len(unord_list)):
                if j == 0:
                    if unord_list[i] != unord_list[0]:
                        unord_list.insert(0, unord_list[i])
                        del unord_list[i+1]
                elif unord_list[i] != unord_list[j] and unord_list[i] != unord_list[j+1]:
                    unord_list.insert(j+1, unord_list[i])
                    del unord_list[i+1]
        else:
            break

    return unord_list

It seems to work in cases where it's possible to make such a list, but I'm pretty sure there is a more efficient way and the code can be optimized.

As the Python version was not specified, I assume Python 3; for Python 2 you need to adjust the imports from itertools

There are a number of ways you can attack this problem. One way becomes apparent if we think of an input that has no solution, which would be if more than half + 1 (rounded up) of the elements would be equal to each other, e.g. you can find a solution for [1, 1, 1, 2, 2] but not for [1, 1, 1, 1, 2, 2].

The solution would be that you group the elements and sort them by decreasing group length, then interleave first half of the elements with the rest; so, if your list is [3, 2, 2, 1, 3, 1, 1], at first you need to somehow group them, then sort into decreasing order, so that you get for example [1, 1, 1, 3, 3, 2, 2]; then you split it into 2 almost equal parts: [1, 1, 1, 3] and [3, 2, 2], and interleave these into the result: [1, 3, 1, 2, 1, 2, 3].

There are many ways to write this algorithm, here is one, using Python standard library extensively:

from collections import Counter
from itertools import chain, repeat, starmap, zip_longest, islice

def funny_op(l):
    # use Counter to count the number of each distinct element, then
    # most_common() to sort them by decreasing frequency
    counts = Counter(l).most_common()

    # counts is a list of (element, count) pairs; with the example input
    # it could be [(1, 3), (3, 2), (2, 2)]

    # we then make an iterator that iterates over each distinct
    # element, by repeating each `element` `count` times
    sorted_els = chain.from_iterable(starmap(repeat, counts))

    # now, for example input sorted_els would be an iterator yielding
    # 1, 1, 1, 3, 3, 2, 2

    # we split it into half; we round up so that the first
    # list gets the extra element
    halfway = (len(l) + 1) // 2

    # the elements that go to even indices

    even = list(islice(sorted_els, halfway))

    # and the remaining fill up the odd indices of the result list
    odd = sorted_els

    # then we interleave elements from even and odd. In case the
    # original list had odd number of elements, there'd be an
    # extra None, which we remove by islicing only len(l) elements;
    # and return the result as a list

    return list(islice(chain.from_iterable(zip_longest(even, odd)), len(l)))

To test:

print(funny_op([2, 2, 1, 1, 1]))

prints:

[1, 2, 1, 2, 1]


Linked list that support modifications during iteration Update:

Fixed last item is not removed during iteration
Fixed clear() during iteration does nothing
Fixed addition of removed item is ignored

The lasted code can be found here. Unitest can be found here.

As part of my Signals library, I needed to create a custom Linked List with the below properties

It should allow "addition" and "removal" of items during iteration, aka when iterator() is called, like in the for loop.
It should ignore-Addition of duplicated items.
It should ignore-Removal of nonexisting items.
it should be a thread-safe

I ended up with the below implementation. Please tell me what do you think in terms of

clean code
bugs
performance

package com.gazman.signals

import java.util.*

internal class ListenersList<T> : Iterable<T?> {
    private val none = Node<T>(null)
    private var head: Node<T>? = null
    private var tail: Node<T>? = null
    private var map = IdentityHashMap<T, Node<T>>()

    fun isNotEmpty(): Boolean {
        return map.isNotEmpty()
    }

    fun add(listener: T) {
        synchronized(this) {
            if (map.containsKey(listener)) {
                return
            }
            val node = Node(listener)
            map[listener] = node

            if (tail == null) {
                head = node
                tail = node
            } else {
                node.previous = tail
                tail?.next = node
                tail = node
            }
        }
    }

    fun remove(listener: T) {
        synchronized(this) {
            val node = map[listener]
            node?.previous?.next = node?.next
            node?.next?.previous = node?.previous
        }
    }

    override fun iterator(): Iterator<T?> {
        return object : Iterator<T?> {
            var node: Node<T>? = none

            override fun hasNext() = node != null && node != tail

            override fun next(): T? {
                node = if (node == none) {
                    this@ListenersList.head
                } else {
                    node?.next
                }
                return node?.value
            }

        }
    }

    fun clear() {
        synchronized(this) {
            head = null
            tail = null
            map.clear()
        }
    }
}

Node
package com.gazman.signals

internal class Node<T>(val value: T?) {
    var previous: Node<T>? = null
    var next: Node<T>? = null
}

I haven't done much Kotlin, however something looked wrong and I was curious, so I wrote some tests.  I had a couple of failures, so you may have issues, or it may be that I'm expecting the wrong behaviour.  There's three scenarios to consider.  I've defined a list of string to make it easy to test.
val list = ListenersList<String>()

clear
You've said that you want the code to be tolerant of adding/removing items during iteration.  I assumed that it would therefor also be tolerant of clear being called (it seems equivalent to calling remove on every item).  So, I would expect it to iterate up to the point of the clear being called.  This isn't what happens.  Instead, it iterates past the end of the list.  So, for example:
@Test
fun clearMiddleWorks() {
    list.add("a")
    list.add("b")
    list.add("c")

    var items = ""

    for (item in list) {
        if(item == "a")
            list.clear()
        items += item
    }

    assertEquals("a", items)
}

This test fails, because instead of exiting the loop after "a", it continues building up the string, and fails on "a" != "abcnull"
end removal
This issue exists in the code you've posted, however it looks like you've cleaned it up in your latest github version.  Removing the last item in the list during iteration, fails to terminate as expected:
@Test
fun removalOfEndWorks() {
    list.add("a")
    list.add("b")
    list.add("c")

    var items = ""

    for (item in list) {
        if(item == "b") {
            list.remove("c")
        }
        items += item
    }

    assertEquals("ab", items)
}

This fails with "ab" != "abnull"
remove and add conflict
This may actually be expected behaviour, however it seems odd to me.  If I remove an item from the list, whilst I'm iterating over it, then add an item with the same value, I'd expect that item to be treated the same was as if I'd added a new item, however it isn't.  So, this test passes:
@Test
fun removalAdditionOtherInMiddleWorks() {
    list.add("a")
    list.add("b")
    list.add("c")

    var items = ""

    for (item in list) {
        if(item == "a") {
            list.remove("b")
            list.add("d")
        }
        items += item
    }

    assertEquals("acd", items)
}

And this test fails:
@Test
fun removalAdditionOfMiddleWorks() {
    list.add("a")
    list.add("b")
    list.add("c")

    var items = ""

    for (item in list) {
        if(item == "a") {
            list.remove("b")
            list.add("b")
        }
        items += item
    }

    assertEquals("acb", items)
}

It fails because the added in "b" is ignored, so "acb" != "ac".  Is this expected?


Hold a list of items stored in recursive class? Alright, well the title doesn't really explain anything, so I'll show a pared down version of my code:

class Handler {
    Dictionary<object, Leaf> AllItems = new Dictionary<object, Leaf>(); //This list

    Leaf RootLeaf = new Leaf();
    public void Add (object obj)
    {
        Leaf AddedTo = RootLeaf.Add(obj); //Adds to a leaf, then returns the leaf it was added to
        AllItems.Add(obj, AddedTo); //adds references to the obj and the leaf it's in.
    }
}

class Leaf {
    private Leaf _Parent;
    public Leaf SubLeaf;
    public List<object> Children = new List<object>();

    public Leaf (Leaf parent)
    {
        this._Parent = parent;
    }

    public void Add (object obj)
    {
        if (Children.Count > 5)
        {
            SubLeaf = new Leaf();
            return SubLeaf.Add(obj); //recursively return the reference to the end leaf
        }
        Children.Add(obj);
        return this; //start the recursive return
    }

    public Leaf Parent { get { return this._Parent; } }
}

Note: The above isn't real code, just an example of what I plan to do.

So the idea of the above is to organize all the items into leaves, but I need a quick way to loop through every object, and its leaf.  Would doing the above (storing a reference of every object & its parent in a list in the Handler class) cause a drop in performance?  Or would there be a better way to do this?

I hope this was clear enough, if not, please ask any questions.

Well, it's not a tree, it's a linked list, and it won't work properly for more than seven items. After that you will overwrite the SubLeaf variable in the first node, and previously added items will be removed from the list, and AllItems will contain lost items with disowned nodes.

To make the list work you have to check if the node has a next node before creating a new one.

public class Node {

  private Node _next;
  private List<object> _items;

  public Node Previous { get; private set; }

  public Leaf (Leaf previous) {
    _previous = previous;
    _next = null;
    _items = new List<object>(6);
  }

  public void Add(object obj) {
    if (_items.Count == 6) {
      if (_next == null) {
        _next = new Leaf(this);
      }
      return _next.Add(obj);
    }
    _items.Add(obj);
    return this;
  }

}

Note: Consider using an array in the node instead of a list. Creating a list that uses lists seems redundant...

Also, keeping a reference to the last node in the handler would help when you want to add nodes, so that you don't have to loop through all nodes each time.

Putting the objects and their nodes in a list is not needed for looping through them, as you already have all the nodes and the objects. Adding the list would roughly tripple the memory usage and the time to add an item. You would only need another list if you need two completely different ways of accessing the items, for example both predictable enumerating and hash lookup.

To enumerate the list as pairs of objects and nodes, you can put an enumerator in the Node class which gets all objects and their nodes from the current node and forward, so you just use it on the first node to get all objects:

public IEnumerable<KeyValuePair<object,Node>> GetAll() {
  Node node = this;
  while (node != null() {
    foreach (object obj in node._items) {
      yield return new KeyValuePair<object,Node>(obj, node);
    }
    node = node._next;
  }
}


String calculator kata in Ruby I've completed the string calculator kata in Ruby (spec from here - http://osherove.com/tdd-kata-1/). All tests currently pass.

Could it be refactored further to improve readability? And could I make my testing better?

string_calculator.rb

class StringCalculator

  def initialize
  end

  def int_add(string_of_numbers)
    raise 'only accepts a string' unless string_of_numbers.is_a?(String)
    string_array = string_of_numbers.split(/[^0-9-]+/)
    integer_array = string_array.map(&:to_i)
    raise "cannot accept negatives - #{check_for_negatives(integer_array)}" if
check_for_negatives(integer_array)
    integer_array.inject(0){|sum,x| x <= 1000? sum + x : sum }
  end

  def check_for_negatives(integer_array)
     negatives_array = integer_array.select{ |i| i<0 }
    if negatives_array.length > 0
      return negatives_string = negatives_array.join(",")
    else
      return false
    end
  end

end

string_calculator_spec.rb

require 'string_calculator'

describe StringCalculator do

  subject(:calculator) { described_class.new }

  it 'should accept a string' do
    expect{ calculator.int_add('1,2,3') }.not_to raise_error
  end

  it 'should not accept other data types' do
    expect{ calculator.int_add(123) }.to raise_error('only accepts a string')
    expect{ calculator.int_add(['123']) }.to raise_error('only accepts a
string')
  end

  it 'should return 0 for an empty string' do
    expect(calculator.int_add('')).to eq(0)
  end

  it 'should return a number if the passed string contains no delimiters' do
    expect(calculator.int_add('123')).to eq (123)
  end

  it 'should return the sum of the numbers in the passed string, if the passed
string contains comma delimiters' do
    expect(calculator.int_add('12,34')).to eq(46)
  end

  it 'should return the sum of the numbers in the passed string, if the passed
string contains new line delimiters' do
    expect(calculator.int_add("12\n34\n56")).to eq(102)
  end

  it 'should handle multiple random delimiters' do
    expect(calculator.int_add("//;\n1;2")).to eq(3)
  end

  it 'should not accept negative numbers' do
    expect{ calculator.int_add("123,-2") }.to raise_error("cannot accept
negatives - -2")
  end

  it 'should ignore numbers larger than 1000' do
    expect(calculator.int_add("//;\n1;2:1001")).to eq(3)
  end

end

The changes I will propose are from ruby-style-guide since that is accepted by the majority of Ruby developers.

What you can improve:

Remove initialize method since there is no need for it in your case.
Use white space to separate groups of logic, something along these lines:

raise 'only accepts a string' unless string_of_numbers.is_a?(String)

string_array = string_of_numbers.split(/[^0-9-]+/)   integer_array = string_array.map(&:to_i)

raise "cannot accept negatives -#{check_for_negatives(integer_array)}" if  check_for_negatives(integer_array)

integer_array.inject(0){|sum,x| x <= 1000? sum + x : sum }

It's not a good practice to hard code the data in the name, instead of string_of_numbers replace with numbers, and instead of integer_array you can figure something else (naming is hard).
Use public and private API methods, since int_add is the only method your class is using, you can rename it to add, and make check_for_negatives private so the reader will know to rely only on add as a stable method.
Ruby returns the last expression, so you don't need an assignment for return negatives_string = negatives_array.join(","), it can be negatives_array.join(",")
You can reduce the check_for_negatives method to this:

def check_for_negatives(numbers)
  negatives = numbers.select{ |i| i < 0 }
  # Ruby return the last statement which is the negatives string,
  # or nil, that is treated as false
  negatives.join(",") if negatives.length > 0
end


Performance for tags for cars in Java private static boolean trovato , ok = false;

public static void main(String args[]) throws IOException {

HashMap<String, HashSet<String>> map = new HashMap<String, HashSet<String>>();
BufferedReader stdin = new BufferedReader(new InputStreamReader(System.in));

String line;
line = stdin.readLine();
String a = "a" ;
String ii = "i" ;
String s = "s" ;
int cont = 0 ;

while (!line.equals("<END>")) {

String stringa = "";
String elemento = "" ;

if (line.startsWith(ii)) {
map = new HashMap<String, HashSet<String>>();
}
else if (line.startsWith(a)) {
String[] letteraPerLettera = line.split("");

for (int i = 0  ; i < letteraPerLettera.length - 2; i++) {

if (!letteraPerLettera[i].equals(" ")) {

if (ok)
{
element = line.substring(2,i-2) ;

trovato  = true ;
ok = false ;
}

if (trovato )
{
stringa+=letteraPerLettera[i];
if (map.containsKey(stringa)) {
map.get(stringa).add(elemento);
} else
{
HashSet <String> hs = new HashSet<String> () ;
map.put(stringa, hs );
map.get(stringa).add(elemento);
}
}

}

else {

cont ++ ;

if (cont == 2)

ok = true ;

stringa = "";

}

}

}

else if (line.startsWith(s)) {
String subtag = line.substring(2,line.length()-3);
if (map.containsKey(subtag)) {

System.out.println(map.get(subtag).size());
} else {
System.out.println("missing");
}
}

cont = 0 ;
trovato = false ;
line = stdin.readLine();
}
stdin.close();
}

My code has an input like this:

i3
a Duna automobile Deserto -1
a Nissan auto automobile -1
s aut -1

i2
a Pesca Sport Frutta -1
s sport -1

END

i = 3 means that there are 3 lines.
"a" means that I'm adding an element (first occurrence after "a") and the other words after the element are my "tags".
"s" means that I have to find the tags that have as substring the word "aut" for the first block and "sport" for the second block.

At the end the output is like this:

￼2
1

"2" means I have two elements "duna" & "nissan" that have tags (auto, automobile, auto) that start with the word "aut".
"1" means I have the element "pesca" that has tag (sport) that starts with the sub-tag "sport".

With END, the program is finished.

Variables, scope, naming

Since you have only one method (main), there's no need to declare trovato and ok outside of it. Move them inside main, and make them local variables.

On the other hand, the variables a, ii, and s are for constant strings, indicating special commands in your command line interface, so it would be good to make them constants, loud and clear at the top of the class, with meaningful names, for example:

public static final String ADD_COMMAND = "a";
public static final String ITEM_COMMAND = "i";
public static final String SEARCH_COMMAND = "s";

The same goes for the <END> marker, which didn't have its own variable and it deserves to have one:

public static final String END_MARKER = "<END>";

Your program becomes instantly more readable. These constants are now also protected from mistakes: you cannot accidentally reassign their values, as they are final.

Furthermore, the variables trovato, cont and ok are actually only used when you're processing ADD_COMMAND. You should declare and initialize them there. This will also make it clear and easy to see that you don't need to reset their values at the end of the loop.

Good coding practices

Use interface types when declaring variables (and function parameters). Instead of this:

HashMap<String, HashSet<String>> map = new HashMap<String, HashSet<String>>();
// ...
map = new HashMap<String, HashSet<String>>();
// ...
HashSet<String> hs = new HashSet<String>();

Do like this:

Map<String, Set<String>> map = new HashMap<String, Set<String>>();
// ...
map = new HashMap<String, Set<String>>();
// ...
Set<String> hs = new HashSet<String>();

And try to think of better names for your maps and sets instead of map and hs. For example cars and tags.

BufferedReader stdin = new BufferedReader(new InputStreamReader(System.in));

Calling this BufferedReader as stdin is confusing and it invites mistakes. stdin is really System.in itself. A better name would be reader, or bufferedReader.

Always use braces with if statements. Instead of:

if (cont == 2)
    ok = true;

Do like this:

if (cont == 2) {
    ok = true;
}

Simplify

Notice the duplicated line in this code:

stringa += letteraPerLettera[i];
if (map.containsKey(stringa)) {
    map.get(stringa).add(elemento);
} else {
    Set<String> hs = new HashSet<String>();
    map.put(stringa, hs);
    map.get(stringa).add(elemento);
}

You can simplify:

stringa += letteraPerLettera[i];
if (!map.containsKey(stringa)) {
    Set<String> hs = new HashSet<String>();
    map.put(stringa, hs);
}
map.get(stringa).add(elemento);

This is not very efficient though, because you search for stringa twice: first with containsKey and then again with get. This is equivalent and more efficient:

Set<String> hs = map.get(stringa);
if (hs == null) {
    hs = new HashSet<String>();
    map.put(stringa, hs);
}
hs.add(elemento);

Last words...

Even with the above improvements, I find this code almost unreadable. They way you process the lines, it's just far more complicated than it needs to be. For example, instead of processing the ADD_COMMAND letter by letter, it would make more sense, and more readable to process word by word, like this:

String[] parts = line.split(" ");

// 0: the command, 1: the car name, 2, ...: tags, length-1: -1 (???)
for (int i = 2; i < parts.length - 1; ++i) {
    String car = parts[1];
    String tag = parts[i];
    for (int j = 1; j <= parts[i].length(); ++j) {
        String tagPrefix = tag.substring(0, j);
        Set<String> cars = map.get(tagPrefix);
        if (cars == null) {
            cars = new HashSet<String>();
            map.put(tagPrefix, cars);
        }
        cars.add(car);
    }
}

This is almost the same as the original program, except it also fixes a bug: the original program drops the last letter of car names (resulting in "Dun", and "Nissa"), this one doesn't.

On a related note, do you really need the -1 at the end of lines? It would be easy to make it work without them.

Instead of using a BufferedInputReader, a Scanner can be easier to work with. I recommend to take a look at it (hint: Scanner scanner = new Scanner(System.in))


Climbing the Leaderboard: HackerranK, Terminated due to timeout I have implemented my solution and I thought it was pretty efficient and passes most of the cases but seems like it fails some of the test cases when the input size is very large. I basically push every item from the Alice's array to our array and sort it.I'd appreciate if somebody tell me how I could increase the efficiency get rid of the timeout error.

  Alice is playing an arcade game and wants to climb to the top of the leaderboard. Can you help her track her ranking as she beats each level? The game uses Dense Ranking, so its leaderboard works like this:

  The player with the highest score is ranked number 1 on the
  leaderboard. Players who have equal scores receive the same ranking
  number, and the next player(s) receive the immediately following
  ranking number. For example, four players have the scores 100, 90, 90,
  and 80. Those players will have ranks 1, 2, 2, and 3, respectively.

  When Alice starts playing, there are already n people on the
  leaderboard. The score of each player i is denoted by si. Alice plays
  for m levels, and we denote her total score after passing each level j
  as alicej. After completing each level, Alice wants to know her
  current rank.

  You are given an array, scores, of monotonically decreasing
  leaderboard scores, and another array, alice, of Alice's cumulative
  scores for each level of the game. You must print m integers. The jth
  integer should indicate the current rank of alice after passing the
  jth level.

To illustrate;

Score is :100 100 50 40 40 20 10

Alice is: 5 25 50 120

The output is going to be;

6,4,2,1

My implementation:

function ladder(arr1,arr2){
  while(arr2.length){
    let i=0
    let rank=1
    let score=arr2.shift()
    arr1.push(score)
    arr1.sort(function(a,b){
      return b-a
    })
    while(arr1[i]!==score){
      if(arr1[i]!==arr1[i+1]){
        rank++
      }

      i++
    }
    console.log(rank)
  }
}

There are some important inefficient elements in the posted implementation.

Inserting into a sorted array

To insert Alice's next score into the array of sorted scores,
the posted appends Alice's score at the end and then sorts the array.
What's the typical time complexity of sorting an array?

   \$O(n \log n)\$

Is it possible to do better?

   When you have a list of sorted values, you can use binary search to find the position to insert a value so that the list stays sorted.

The general idea of binary search goes something like this: pick the middle position m of the array. Is it bigger than the value? Then pick the middle position between 0 and m. Or else if the value was smaller, then pick the middle between m and the end of the array. Follow the same logic, in each step dividing the interval to check by 2, quickly converging on the target position.

What would be the time complexity to find that position using binary search?

   \$O(\log n)\$

That's a lot faster than \$O(n \log n)\$.

By the way, since Alice's score only increases,
the range of values to search for the correct position shrinks,
so the fast solution just keeps getting faster for finding the higher scores.

By the way, once you found the right position, do you actually need to insert into the array?

   No, in this particular challenge, you could just overwrite the position with the new value.

Why is that even a question, why not just insert the value and be done with it?

   Inserting a value into an array typically requires copying the values that come after the insertion position, which is expensive. Depending on the programming language and how arrays work, a reallocation of the entire array (copying all elements) may also happen, which is even more expensive.

And keep in mind that we have to do this m times, where m is the number of scores of Alice.

What is the overall time complexity of the solution with sorting?

   \$O(m n \log n)\$

What is the overall time complexity when we replace sorting with binary search? (Without shrinking the search range, to keep the theory simple.)

   \$O(m \log n)\$

If we take into account the shrinking of the search range, then we can achieve a performance faster than \$O(m + n)\$.

Processing the two lists together

An alternative linear time solution is possible if we process the two lists together:

Compute the ranks of the leaderboard (see the next section of my answer)
Iterate over the scores of Alice, in increasing order
From the end of the leaderboard, find the next score that is  greater than or equal to Alice's current score

If that score is equal to Alice's, then Alice's rank is the same as of that score
Otherwise (the score is greater than Alice's), Alice's rank is 1 + the rank of that score

Repeat for all scores of Alice. Note that when looking for the next leaderboard score, you can continue from the position of the last one found, do not restart from the end

This algorithm visits all elements of the two arrays once, performing a constant number of operations for each. What is the overall time complexity?

   \$O(m + n)\$

This is not as good as using binary search that can skip many leaderboard scores at once, but probably fast enough for this exercise, and simpler to implement.

Computing the rank

The current implementation recomputes the ranks up until Alice's position for every single position.
That's unnecessary.
You could compute the ranks just once up front, and store in an array.
Then, once you know Alice's position, you could use the index of that position to find the rank directly.

Style

It's recommended to declare variables right before you need them.
For example i and rank are declared too early,
when they are only used after the sorting step.
The reason to delay declaration as much as possible is to reduce the possibility of mistakes: a variable is not defined yet, a use by mistake is likely going to crash badly and so you discover the problem early.
Otherwise it may get overlooked.

A common convention to improve readability is to add spaces around operators, after commas, before ( and after ), for example:

function ladder(arr1, arr2) {
  while (arr2.length) {
    let i = 0


C program to print Pascal's triangle Worked 2 hours on this. It print Pascal's triangle. Change MAX_LEN to any desired triangle size. What do you think about the quality of the code?

#include "stdafx.h"
#include <stdio.h>
#include <malloc.h>

int main() {
        const unsigned int MAX_LEN = 20;
        unsigned char rowNum, rowWidth, rowOffset, i;
        unsigned int *numPointer, *num;
        unsigned int maxNum = 0;

        for (char len = 1; len <= MAX_LEN; len++)
            maxNum += len;
        num = (unsigned int*)malloc(sizeof(unsigned int)*maxNum);
        if (num == NULL)
            return 1;

        for (rowNum = 0; rowNum < MAX_LEN; rowNum++) {
            rowOffset = 0;
            for (i = 0; i <= rowNum; i++) {
                rowOffset += i;
            }
            num[rowOffset] = 1;
            for (i = 1; i < rowNum; i++) {
                num[rowOffset + i] = num[rowOffset + i - rowNum] + num[rowOffset + i - rowNum - 1];
            }
            num[rowOffset + rowNum] = 1;
        }

    numPointer = num;
    for (rowNum = 0; rowNum < MAX_LEN; rowNum++) {
        for (rowWidth = 0; rowWidth <= rowNum; rowWidth++) {
            printf_s("  %d  ", *numPointer);
            numPointer++;
        }
        printf_s("\n");
    }
        return 0;
}

Some things to consider:

for (char len = 1; len <= MAX_LEN; len++) is extremely dangerous as you could easily overflow len here since MAX_LEN has type unsigned. Currently, the value of MAX_LEN is 20, and your code is safe; however, if you decide to change this value at any point, you will likely overflow len here, leading to undefined behavior either because of overflow of a signed type if char is signed or because of the fact that infinite loops (into which this one will turn) that do not do IO are ub in the case of an unsigned char type.
In C, you do not need to cast the return value of malloc. Simple assignment will do. In fact, don't actively cast the return value of malloc because it clutters your code with casts, which is a symptom of bad code in general, and can hide a bug related to implicit functions (only before C11).
Actually, that loop from point 1 I mentioned can be removed altogether. This follows from recognizing that you are adding all integer values starting from one up to MAX_LEN, for which there is an easy to compute formula: S = n(n + 1)/2. Translated into code, this would be

maxNum = MAX_LEN * (MAX_LEN + 1) / 2;

which is not only shorter, but also more efficient to compute.
Why do you define almost all your variables at the top of your function, although they are not used until much later? This is a bad practice, because it increases the mental load everyone reading your code has to deal with. I don't want to remember the types of eight or nine different variables while focusing on code that doesn't even use them. Just define them right were they need to be defined.
Overall, you use char too much. Only use char when you are working with characters or single bytes, otherwise use int, unsigned and everything larger or equal in bit size. You are not gaining anything through your use of char except a high chance to trigger ub several times over in your code (You are likely not even saving space because of alignment concerns on most common modern platforms.)
Don't printf a single character; that's wasteful. Use putchar for that.
Don't include stdafx.h. Unless you have a concrete compilation performance concern, it won't do anything for you. And even then, it is not standard, and most people who use compiler other than MSVC will likely complain about it.
Split your code up into functions. main() is not the right place to write problem-solving code, only to put things together. Do yourself a favor and organize your code a little more, it will help you keep a good overview over which part does what and make other readers not turn away on sight of that huge main god-function.


Make 10 from the numbers 1,1,5 and 8 using the 4 operations, by brute force I tried to calculate the following problem by brute force using R (under the assumption that a single pair of parenthesis would suffice).

  Make 10 from the numbers 1, 1, 5, 8. You can use the operations + – *
  / ().  You have to use all the numbers, and use each number exactly
  once. The operations can be repeated (like 1 + 1 + 5 + 8) and you
  don’t have to use each operation.

I did the following and got the correct answer, but I would like your ideas for better ways of addressing that problem.

## Helper functions for inserting characters into strings
# http://stackoverflow.com/questions/13863599/insert-a-character-at-a-specific-location-in-a-string/25416334#25416334

  split_str_by_index <- function(target, index) {
    index <- sort(index)
    substr(rep(target, length(index) + 1),
           start = c(1, index),
           stop = c(index -1, nchar(target)))
  }

  #Taken from https://stat.ethz.ch/pipermail/r-help/2006-March/101023.html
  interleave <- function(v1,v2)
  {
    ord1 <- 2*(1:length(v1))-1
    ord2 <- 2*(1:length(v2))
    c(v1,v2)[order(c(ord1,ord2))]
  }

  insert_str <- function(target, insert, index) {
    insert <- insert[order(index)]
    index <- sort(index)
    paste(interleave(split_str_by_index(target, index), insert), collapse="")
  }

  # insert_str("1+1+5+8", c("(", ")"), c(2, 3))

  #################

  library(gtools)
  library(dplyr)

  symbols <- expand.grid(data.frame(matrix(rep(c("+","-","*","/"),3),ncol=3)))

  numbers <- unique(permutations(4,4,c(1,1,5,8),set=FALSE))

  # Insert symbols between the numbers
  lst <- lapply(1:nrow(numbers) , function(i) mapply(paste0,numbers[i,],symbols))
  res1 <- data.frame(do.call(rbind, lst))

  # paste strings omitting the trailing symbol
  res2 <- res1 %>%
    transmute(expr=do.call(paste0,.)) %>%
    transmute(expr=substr(expr,1,7))

  # positions of opening and closing bracket
  x<-expand.grid(c(1,3,5),c(4,6,8))
  x<-x[x[1]<x[2]-1,]

  # Generate x rows for each row of res2
  res3 <- as.data.frame(matrix(rep(NA,nrow(res2)*nrow(x)),ncol=1))
  for (i in seq(nrow(res2))){
    temp <- res2[i,]
    res3[((i-1)*nrow(x)+1):(i*nrow(x)),] <- sapply(1:6, function(j) insert_str(temp,c("(", ")"),x[j,]))
  }

  res3  %>%
    rowwise() %>%
    mutate(result=eval(parse(text=V1))) %>%
    filter(result==10)

I like your code but don't have a solution that improves on brute force. Just a few comments:

(1) You can eliminate your for loop assigning to res3 like so:

res3 = data.frame(V1 = unlist(lapply(res2$expr, function(temp) sapply(1:6, function(j) insert_str(temp,c("(", ")"),x[j,])))))

Whether this is a good idea is debatable, but it's slightly faster when I test it as compared to the for loop (a difference of about 10%). Perhaps lapply would become more advantageous as you increased the number of digits or operations to be explored.

(2) The obvious speed-related comment is to use data.table. This is another change that will more noticeably improve performance as you move to a larger-scale problem since you can benefit from the in-place operations.

(3) You are working with a manageable set of numbers here, but in the future you might want to seek solutions in parts. For example, check half the possibilities and only check the other half if you don't find a solution. Similarly, you don't have to assume there's only one set of parentheses but could make the choice to check the second set if your first search yielded no results.

(4) When you know the numbers in advance or you're coding to a specific set of numbers, it might be helpful to eliminate all rows that have a certain combo that couldn't possibly be in a correct answer. For example, I'd get rid of anything with 8*5 or 5*8 since nothing you can do with the 1's can bring that expression down to anything close to 10. Similarly I'd get rid of anything with 1/1 and 1*1 since these also can't lead to a solution.  Doing this would eliminate 1152 out of 4608 possibilities. (Note that I'm relying on standard order of operations rules.)

I don't feel any of these comments are super helpful but wanted to write since I enjoyed reading your code and since Code Review doesn't always give much feedback to us R users. Hopefully others will add more interesting things.


Array Implementation of Queue I've implemented the queue data structures using array in java. Anything I need to change in my code?

Queue.java

import java.util.Arrays;

public class Queue<T> {

    private int front;
    private int rear;
    int size;
    T[] queue;

    public Queue(int inSize) {
        size = inSize;
        queue = (T[]) new Object[size];
        front = -1;
        rear = -1;
    }

    public boolean isempty() {
        return (front == -1 && rear == -1);
    }

    public void enQueue(T value) {
        if ((rear+1)%size==front) {
            throw new IllegalStateException("Queue is full");

        } else if (isempty()) {
            front++;
            rear++;
            queue[rear] = value;

        } else {
            rear=(rear+1)%size;
            queue[rear] = value;

        }
    }

    public T deQueue() {
        T value = null;
        if (isempty()) {
            throw new IllegalStateException("Queue is empty, cant dequeue");
        } else if (front == rear) {
            value = queue[front];
            front = -1;
            rear = -1;

        } else {
            value = queue[front];
            front=(front+1)%size;

        }
        return value;

    }

    @Override
    public String toString() {
        return "Queue [front=" + front + ", rear=" + rear + ", size=" + size
                + ", queue=" + Arrays.toString(queue) + "]";
    }

}

QueueImpl.java

public class QueueImpl {

    public static <T> void main(String[] args) {
        Queue newQueue = new Queue(5);
        newQueue.enQueue(10);
        newQueue.enQueue(20);
        newQueue.enQueue(30);
        newQueue.enQueue(40);
        newQueue.enQueue(50);
        System.out.println((T) newQueue.toString());
        System.out.println((T) newQueue.deQueue().toString());
        System.out.println((T) newQueue.deQueue().toString());
        System.out.println((T) newQueue.toString());
        newQueue.enQueue(60);
        newQueue.enQueue(70);
        System.out.println((T) newQueue.toString());
        System.out.println((T) newQueue.deQueue().toString());
        System.out.println((T) newQueue.deQueue().toString());
        System.out.println((T) newQueue.deQueue().toString());
        System.out.println((T) newQueue.deQueue().toString());
        System.out.println((T) newQueue.deQueue().toString());
        System.out.println((T) newQueue.toString());

    }

}

Your fields size and queue have package access, they need to be private so only your class can control them.

private final int size;
private final T[] queue;

And because their values are known at initialization, its good practice to declare them final.

your isempty method is not Camelcase, use isEmpty instead.
Some validation on the size parameter is needed as well.

 if(size<=0){
  throw new IllegalArgumentException("Size cannot be less than or equal to zero");
 }

Something I love about the this keyword is that it helps you finding a name for your variables

 public Queue(int size) { // size is more relevant than inSize
   if(size<=0){
       throw new IllegalArgumentException("Size cannot be less than or equal to zero");
    }
    this.size = size;
    queue = (T[]) new Object[size];
    front = -1;
    rear = -1;
}


Proper naming for a Time class Which of these names is better: time.getHours() or time.hours()? And why?

public class Time implements Serializable {
    private final int hours;
    private final int minutes;

    public static Time from(Calendar calendar) {
        int hoursIn24HourFormat = calendar.get(Calendar.HOUR_OF_DAY);
        int minutes = calendar.get(Calendar.MINUTE);
        return new Time(hoursIn24HourFormat, minutes);
    }

    public Time(int hours, int minutes) {
        this.hours = hours;
        this.minutes = minutes;
    }

    // or may be getHours() name is better?
    public int hours() {
        return hours;
    }

    // or may be getMinutes() name is better?
    public int minutes() {
        return minutes;
    }

    @Override
    public boolean equals(Object obj) {
        if (obj == null) {
            return false;
        }
        if (obj == this) {
            return true;
        }
        if (!(obj instanceof Time)) {
            return false;
        }
        Time other = (Time) obj;
        return (hours == other.hours) &&
               (minutes == other.minutes);
    }

    @Override
    public int hashCode() {
        return toMinutesOfDay();
    }

    private int toMinutesOfDay() {
        return hours * 60 + minutes;
    }

    @Override
    public String toString() {
        return twoDigitString(hours) + ":" + twoDigitString(minutes);
    }

    private static String twoDigitString(int timeComponent) {
        return (timeComponent < 10)
                ? ("0" + timeComponent)
                : String.valueOf(timeComponent);
    }

    public boolean before(Time time) {
        return toMinutesOfDay() < time.toMinutesOfDay();
    }

    public boolean after(Time time) {
        return toMinutesOfDay() > time.toMinutesOfDay();
    }

    public boolean within(Time fromIncluded, Time toIncluded) {
        return (!fromIncluded.after(this)) && (!this.after(toIncluded));
    }
}

It is a matter of taste, so there is no 'better'. But more common seams to be time.getHours(); because often the conventions of JavaBean are used.

If you work in a team, I would discuss the naming conventions to use with them.


Change the background color of the console with the help of enum I've recently learned the concept of enum and method calling with the enum. From what I learned I've done this simple snippet which changes the background color of the console with the help of enum and method.

    public static void SetColor(RanngDe R){
        switch (R){
            case RanngDe.Blue:
                 Console.BackgroundColor = ConsoleColor.Blue;
                 break;

            case RanngDe.Green:
                 Console.BackgroundColor = ConsoleColor.Green;
                 break;
            default:
                 Console.BackgroundColor = ConsoleColor.Yellow;
                 break;
        }
    }

// Enum Declaration
public enum RanngDe{
    Blue=0,
    Green=1,
    Yellow=3
}

I'm calling RanngDe method in the main method in switch ...case block as menu driven. I know, I've used switch ...case in void SetColor(RanngDe R) method, which is my major concern, because this makes my code redundant as I'm calling it in the menu driven program.

Is this approach acceptable as good practice? If not then, how should I make this better?

As has been mentioned using a Dictionary<RanngDe,Color> will make your code much simpler:

public enum RanngDe
{
    Blue = 0,
    Green = 1,
    Yellow = 3
}
static Dictionary<RanngDe, ConsoleColor> colors = new Dictionary<RanngDe, ConsoleColor>()
{
    {RanngDe.Blue,ConsoleColor.Blue },
    {RanngDe.Green,ConsoleColor.Green },
    {RanngDe.Yellow,ConsoleColor.Yellow }
};
public static void SetColor(RanngDe R)
{
    Console.BackgroundColor = colors[R];
}

Depending on your implementation you might be able to do away with the method completely and just use the assignment.

I came up with another approach.  If you change the values of Ranngde to be the same as the ConsoleColor enum, you can cast the Ranngde value to ConsoleColor:

public enum RanngDe
{
    Blue = ConsoleColor.Blue,
    Green = ConsoleColor.Green,
    Yellow = ConsoleColor.Yellow
}
public static void SetColor(RanngDe R)
{
    Console.BackgroundColor = (ConsoleColor)R;
}


'iplayer' style content accordion/slider I was wondering if someone could have a look at this BBC iPlayer style content accordion/slider I've just made, and help me to get it working properly? I'm no expert in jQuery and I'm sure I haven't done this correctly, or if there are more efficient ways of doing it.

The problem seems to be the clicking.  It sometimes requires two clicks, and the sliding of the content, the left panel seems to re-size when animating.

Here is all of the code.

Snippet from jsfiddle:

$('#accordion > li.bg1 .heading').toggle(
    function () {
        $('#accordion > li.bg2').stop().animate({'width':'718px'},1000);
        $('#accordion > li.bg1').stop().animate({'width':'50px'},1000);
        $('#accordion > li.bg1 .heading').stop(true,true).fadeOut(1000);
        $('#accordion > li.bg1 .heading-side').stop(true,true).fadeIn(2000);
        $('a.see-more-p').css('display','none');
     },
    function () {
        $('#accordion > li.bg1').stop().animate({'width':'374px'},1000);
        $('#accordion > li.bg2').stop().animate({'width':'394px'},1000);
        $('#accordion > li.bg1 .heading-side').stop(true,true).fadeOut(1000);
        $('#accordion > li.bg1 .heading').stop(true,true).fadeIn(2000);
        $('a.see-more-p').css('display','block');
    }
 );
$('#accordion > li.bg2 .heading, a.see-more-p').toggle(
    // same as above
);
$('#accordion > li.bg1 .heading-side').toggle(
    // same as above
);

I'd normally suggest you post this on StackOverflow  as it doesn 't work properly.

Firstly:

You have the same two functions repeated over and over .... don't.  Put them in one big selector.

$('#accordion > li.bg1 .heading, #accordion > li.bg2 .heading, a.see-more-p, #accordion > li.bg1 .heading-side').toggle

You have two headings for each. Semantically this is wrong. change your .heading-side to .heading.side. then all you need is an addClass('side'); and removeClass('side');
also you will need:

ul.accordion li .heading.side .right{ display: none; }

to hide the >.
To get the fadeIn/Out:

.fadeOut(500).queue(function(next) {
    $(this).addClass("side");
    next();
}).fadeIn(500);

That looks confusing with the .queue(... but that's just to make sure it happens between the fading so the user doesn't see any oddness.
The toggle only toggles for that specific context (i.e. the element being clicked) so it will toggle for each element, hence the double clickyness.
My solution is to roll your own toggling:

$(function() {
    var first = true;
    $('#accordion > li.bg1 .heading, #accordion > li.bg2 .heading, a.see-more-p, #accordion > li.bg1 .heading-side')
    .click(function() {
        if (first) {
            //do first stuff
        }else{
            // do second stuff
        }
        first = !first; // < --- Toggling done here!!!!
    });
});

Viola! see jsFiddle for my awesomeness!