Insert Selected item in listbox using Ado.net and stor proc I create this class method to Enter Selected Items Form CheckboxList to associated resource I use ADO.net & store Proc to insert data can someone help me to improve it and make to so clean instead of my Hacks BTW I use Microsoft Enterprise Lib

public  static void Insert(List<int> listSkills, int rID)
        {
            var connection = ConfigurationManager.ConnectionStrings["SiteSqlServer"];
            Database objDB = new SqlDatabase(connection.ConnectionString);
            //int val = 0;
            using (DbCommand cmd = objDB.GetStoredProcCommand("Insert_Skills_Resources"))
            {
                foreach (var s in listSkills)
                {
                    try
                    {
                        objDB.AddInParameter(cmd, "@SkillID", DbType.Int32, s);
                        objDB.AddInParameter(cmd, "@ResourceID", DbType.Int32, rID);

                        objDB.ExecuteNonQuery(cmd);
                        cmd.Parameters.Clear();

                    }
                    catch (Exception ex)
                    {

                        throw ex;

                    }
                }

            }

If you are using the Enterprise Library it is often better to set up a default connection in the configuration file:

<configuration>
  <configSections>
    <section name="dataConfiguration"
       type="Microsoft.Practices.EnterpriseLibrary.Data.Configuration.DatabaseSettings,
            Microsoft.Practices.EnterpriseLibrary.Data, Version=5.0.414.0,
            Culture=neutral, PublicKeyToken=null"
        requirePermission="false"/>
  </configSections>

  <dataConfiguration defaultDatabase="SiteSqlServer" />

  <!-- Rest of your settings here, including your SiteSqlServer connection string -->
</configuration>

Now you don't need to look up the connection string - you can rely on the Enterprise library to get it for you:

public static void Insert(IEnumerble<int> skills, int resourceId)
{
    // No dependency on SQL anymore - the connection string is found
    // from the dataConfiguration section in your config file. If you change
    // to another (supported) provider, you don't need to change this code.
    Database db = DatabaseFactory.CreateDatabase();

    // Add in your schema to the stored proc name (I've assumed dbo).
    using (var cmd = db.GetStoredProcCommand("dbo.Insert_Skills_Resources"))
    {
        // This method populates the parameters collection of the DbCommand
        // by interrogating the database about the command.
        db.DiscoverParameters(cmd);

        // As the parameters are now known, we can just set the value of
        // them directly, no need specify direction or type etc.
        cmd.Parameters["ResourceID"].Value = resourceId;

        foreach (var skill in skills)
        {
            cmd.Parameters["SkillID"].Value = skill;
            db.ExecuteNonQuery(cmd);
                    // Don't need to clear the parameters now.
        }
        // <Pointless try catch removed.>
    }
}

EDIT:

Sorry, it's been a year or more since I used the Enterprise Library I can't remember whether you need the @ at the start of the parameter name or not - it might be cmd.Parameters["@SkillID"].

EDIT 2:

I remembered why I couldn't remember if you need the @ or not (if that makes sense). You do need the @ BUT, you should use the Database.BuildParameterName method to create the parameter in the format the current provider expects - for sql server this is an @ symbol before the parameter name.

This is how you should do it:

cmd.Parameters[db.BuildParameterName("ResourceID")].Value = resourceId;

With this final correction the code is actually ignorant of the provider!


Copy a contiguous sub-column of cells This code searches for the word "Disbursements" and copies all of the rows with data below it, including the row with the specified word. After the rows are copied to a new sheet, the unnecessary rows below the contiguous range of data are deleted. In order to search for the last cell, I used a function that finds the last cell with data.
Instead of deleting the rows below the contiguous range, should I just copy the contiguous rows to begin with? Is there a better way to delete the rows below the first blank row?
Sub Reformat_ZZ_CM_BNREG()

Application.ScreenUpdating = False
Application.Calculation = xlManual
Application.DisplayAlerts = False

'declare variables to search for within bank register
Dim searchText As String
Dim ws As Worksheet
Dim searchCell As Range
Dim newWS As Worksheet
Dim lastCell As String
Dim firstCell As String
Dim lastRow As Long

'set variables to sheet name, search text, and searchCell
searchText = "Disbursements"
Set ws = Sheets("ZZ_CM_BNREG")
lastCell = FindLast(3, "ZZ_CM_BNREG")
'Set lastCell = Range(Cells.Find("*", , xlFormulas, , xlRows, xlPrevious, , , False), Cells.Find("*", , xlFormulas, , xlColumns, xlPrevious, , , False))
Set newWS = Sheets.Add(After:=Sheets("ZZ_CM_BNREG"))
newWS.Name = "Bank Register"

'Unmerge all cells in Column A & get cell address of searchCell
With ws

    ws.Activate

    Range("A:A").UnMerge

    Set searchCell = .Cells.Find(What:="Disbursements", _
        SearchFormat:=True)

    If searchCell Is Nothing Then
        MsgBox ("Error")
    Else
        firstCell = searchCell.Address
    End If

End With

'copy range using above function to get last cell in range
'paste range onto the new sheet
'we are starting with the cell containing disbursements
ws.Range(firstCell, lastCell).Copy _
    Destination:=newWS.Range("A1")

DeleteRowsAndSheet

End Sub

Sub DeleteRowsAndSheet()

Dim LR As Long
LR = Sheets("Bank Register").Range("A1").End(xlDown).Row

Sheets("Bank Register").Activate
Rows(LR + 1 & ":" & LR + 20).Delete

Sheets("Bank Register").Range("1:1").Delete

Sheets("ZZ_CM_BNREG").Delete

Application.ScreenUpdating = True
Application.Calculation = xlAutomatic
Application.DisplayAlerts = True

End Sub

I will address your code in sections and at the end I will provide a full updated code based on the touched subjects.
Application state
You are turning the Application 'off' with:

Application.ScreenUpdating = False
Application.Calculation = xlManual
Application.DisplayAlerts = False

at the beginning of your method and then you turn it back 'on' with:

Application.ScreenUpdating = True
Application.Calculation = xlAutomatic
Application.DisplayAlerts = True

at the end of another method.
It's a good idea to turn things back on within the scope of the same method. In your case you would add the latter 3 lines at the end of the Reformat_ZZ_CM_BNREG method immediately after calling the DeleteRowsAndSheet method. You should not rely on another method to 'clean-up' for you. What if you decide that you don't need to call that method in the future? There is a high chance you will forget to move the 'clean-up' code back in the main method.
How about turning things 'off' and then back 'on'. It's not a good idea to do this, even if it works for you in this situation, because in time you can get used to this kind of approach and repeat it without giving too much thought. Imagine you have another method called Main that turns the application 'off' and then calls your Reformat_ZZ_CM_BNREG method and then does other stuff like calling a method called MyMethod and then finally turns the application back 'on'. Something like:
Sub Main()
    Application.ScreenUpdating = False
    Application.Calculation = xlManual
    Application.DisplayAlerts = False

    Reformat_ZZ_CM_BNREG
    MyMethod

    Application.ScreenUpdating = True
    Application.Calculation = xlAutomatic
    Application.DisplayAlerts = True
End Sub

What if MyMethod expects the calculation to be 'off'? Well, Reformat_ZZ_CM_BNREG actually turns things back 'on' and you can clearly see this is not what MyMethod (or even Main expects).
What if the Calculation needs to be Manual all the time because this is what the user of the Workbook wants?
The best approach, which you should always use, is to make no assumption of what the current state is because you have no control of what other higher-level methods might expect/do with the state. Your method should look like this:
Sub Reformat_ZZ_CM_BNREG()
    Dim appScrUpdate As Boolean
    Dim appCalc As XlCalculation
    Dim appDispAlert As Boolean

    'Store current Application state
    appScrUpdate = Application.ScreenUpdating
    appCalc = Application.Calculation
    appDispAlert = Application.DisplayAlerts

    'Turn state off
    Application.ScreenUpdating = False
    Application.Calculation = xlManual
    Application.DisplayAlerts = False

    'Actual code
    '...
    '...
    '...

RestoreState:
    Application.ScreenUpdating = appScrUpdate
    Application.Calculation = appCalc
    Application.DisplayAlerts = appDispAlert
End Sub

RestoreState: line label can be used in case you want to early exit the method using something like GoTo RestoreState which would still do the 'clean-up'
In short, you restore the state as it was, making no assumptions, to make sure you do not impact logic somewhere else.
There is a lot of boilerplate code to do this so I use a class instead so that I can have multiple instances with stored state at any given time. Here is what I usually use, and the code above would get simplified to something like:
Sub Reformat_ZZ_CM_BNREG()
    Dim app As New ExcelAppState
    app.StoreState
    app.Sleep

    'Actual code
    '...
    '...
    '...

    app.RestoreState
End Sub

Indentation
For me, it is much easier to read this:
Function DoSomething() As Integer
    Dim iNumber As Integer
    For iNumber = 1 To 5
        DoSomething = DoSomething + iNumber
    Next iNumber
End Sub

compared to this:
Function DoSomething() As Integer
Dim iNumber As Integer
For iNumber = 1 To 5
    DoSomething = DoSomething + iNumber
Next iNumber
End Sub

especially when I have a large number of methods. Although your code is working, please consider this aesthetic aspect when writing code. All code that is within the scope of the method should have an extra indentation level than the method definition.
Hardcoding
You are using the same string literals in multiple places: "Disbursements", "ZZ_CM_BNREG", "Bank Register". Consider declaring them as constants so that you only need to change them (if needed) in a single place:
Const searchText As String = "Disbursements"
Const sourceShtName As String = "ZZ_CM_BNREG"
Const targetShtName As String = "Bank Register"

If you ever decide to receive these as method parameters then all you need to do is to move the constants (without the Const keyword) into the method definition without needing other changes e.g. Sub Reformat_ZZ_CM_BNREG(ByVal searchText As String, ByVal sourceShtName As String, ByVal targetShtName). Might not be useful here, but you get the idea.
Worksheet assumptions
You are making the following assumptions:

a worksheet named "ZZ_CM_BNREG" exists
a worksheet named "Bank Register" does not exist
sheet named "ZZ_CM_BNREG" is a Worksheet (it could be a chart sheet)
the workbook is unprotected

Obviously, it works for you but it's better to be a bit precautious to avoid unexpected errors and behaviour. Consider this: if any of the assumptions would fail then a runtime error would be raised, and you would not get the chance to restore the application state (or do other clean-up operations that might be required).
Consider an auxiliary function to retrieve the worksheet:
Public Function GetWorksheetByName(ByVal wsName As String, ByVal book As Workbook) As Worksheet
    On Error Resume Next
    Set GetWorksheetByName = book.Worksheets(wsName)
    On Error GoTo 0
End Function

you could simply use it like this:
Set ws = GetWorksheetByName(sourceShtName, ThisWorkbook)
If ws Is Nothing Then
    'Do something e.g. show a message box if needed, restore state or exit method
    '...
End If

Notice the auxiliary function uses the Worksheets collection instead of the Sheets collection.
How about these 2 lines?

Set newWS = Sheets.Add(After:=Sheets("ZZ_CM_BNREG"))
newWS.Name = "Bank Register"

As mentioned, these would fail if the workbook is protected or if another sheet with the same name already exists. You can guard with something like:
On Error Resume Next
Set newWs = ThisWorkbook.Worksheets.Add(After:=ws)
If Err.Number <> 0 Then
    MsgBox "Cannot insert new worksheet", vbExclamation, "Failed"
    Err.Clear
    'Restore state or exit method
End If
newWs.Name = targetShtName
If Err.Number <> 0 Then
    MsgBox "Cannot rename worksheet", vbExclamation, "Failed"
    'This might not be critical so just resume
End If
On Error GoTo 0

Notice that I haven't retrieved the source worksheet again, unnecessarily. Instead I simply used After:=ws
Finding desired range

In order to search for the last cell, I used a function that finds the last cell with data.

I am not sure what the 3 value does when calling the FindLast method with FindLast(3, "ZZ_CM_BNREG"). I suspect you are starting from the 3rd row.
The commented line Set lastCell = Range(Cells.Find("*", , xlFormulas, , xlRows, xlPrevious, , , False), Cells.Find("*", , xlFormulas, , xlColumns, xlPrevious, , , False)) returns a Range object while the variable was declared as text with Dim lastCell As String. The commented line simply does not compile if I uncomment it. It's missing a trailing .Address and the Set needs to be removed. Anyway, I assume your FindLast function returns the text address of the last row and column with data in a similar manner with the modified commented line.
You are looking for the desired text with:

'Unmerge all cells in Column A & get cell address of searchCell
With ws

   ws.Activate

   Range("A:A").UnMerge

   Set searchCell = .Cells.Find(What:="Disbursements", _
       SearchFormat:=True)

   If searchCell Is Nothing Then
       MsgBox ("Error")
   Else
       firstCell = searchCell.Address
   End If

End With

but although you are unmerging column A you are searching for your text within the whole worksheet with ws.Cells.Find(What:="Disbursements", SearchFormat:=True) (ws is implied from the With block).
Assuming there is no "Disbursements" value in columns A and B but there is such a value in column C then your code copies data starting with column C and lose the information in column A and B. As I am unsure if this is intended functionality or not, I made the assumption that you want all columns of data regardless where the keyword is found.

Instead of deleting the rows below the contiguous range, should I just copy the contiguous rows to begin with? Is there a better way to delete the rows below the first blank row?

Yes, it's faster to simply copy only what you need instead of copying useless data that you are going to delete anyway. Your second question suggests that you are only interested in data up to (but not including) the first blank row.
I do not know what you consider a "blank row" to be. I can see that you find the last row (in the target sheet) with Sheets("Bank Register").Range("A1").End(xlDown).Row but that means you are actually looking for the first blank cell in the 'A' column and you don't care if there is data in the rest of the columns. Anyway, I assumed that you are interested in actual "blank rows" (i.e. no data in any of the columns) not just on the 'A' column. Here is a function that can check if a row is empty for a 2D array:
Private Function Is2DArrayRowEmpty(arr() As Variant, ByVal rowIndex As Long, Optional ByVal ignoreEmptyStrings As Boolean = False) As Boolean
    Dim j As Long
    Dim v As Variant

    For j = LBound(arr, 2) To UBound(arr, 2)
        v = arr(rowIndex, j)
        Select Case VBA.VarType(v)
            Case VbVarType.vbEmpty
                'Continue to next element
            Case VbVarType.vbString
                If Not ignoreEmptyStrings Then Exit Function
                If v <> vbNullString Then Exit Function
            Case Else
                Exit Function
        End Select
    Next j
    Is2DArrayRowEmpty = True 'If code reached this line then row is Empty
End Function

Reading the worksheet's used range into an array should give us the flexibility to find the desired range faster.
It's not really needed to insert a new worksheet (and to turn the application state to 'off' and later back 'on') if we cannot find any data that we want to take across. It makes more sense to first search for the data and only then proceed with the other operations.
Final code
Code in a standard module:
Option Explicit

Sub Reformat_ZZ_CM_BNREG()
    Const searchText As String = "Disbursements"
    Const sourceShtName As String = "ZZ_CM_BNREG"
    Const targetShtName As String = "Bank Register"

    Dim sourceWS As Worksheet
    Dim targetWS As Worksheet

    Set sourceWS = GetWorksheetByName(sourceShtName, ThisWorkbook)
    If sourceWS Is Nothing Then
        MsgBox "Missing worksheet", vbExclamation, "Cancelled"
        Exit Sub
    End If

    Dim rngUsed As Range

    On Error Resume Next
    Set rngUsed = sourceWS.UsedRange
    On Error GoTo 0
    If rngUsed Is Nothing Then
        MsgBox "Sheet has no data", vbExclamation, "Cancelled"
        Exit Sub
    ElseIf rngUsed.Count = 1 Then
        MsgBox "Sheet has only 1 cell of data", vbExclamation, "Cancelled"
        Exit Sub
    End If

    Dim arrData() As Variant: arrData = rngUsed.Value2
    Dim lowRowIndex As Long: lowRowIndex = LBound(arrData, 1)
    Dim uppRowIndex As Long: uppRowIndex = UBound(arrData, 1)
    Dim v As Variant
    Dim i As Long
    Dim firstRow As Long
    Dim lastRow As Long

    i = lowRowIndex
    For Each v In arrData 'Traverses array in column-wise order (faster than For...To)
        If VarType(v) = vbString Then
            If InStr(1, v, searchText, vbTextCompare) > 0 Then
                firstRow = i + 1
                Exit For
            End If
        End If
        i = i + 1
        If i > uppRowIndex Then 'Next column follows - You might want to Exit For if you are only interested in the first column
            i = lowRowIndex
        End If
    Next v

    If firstRow = 0 Then
        MsgBox "Text not found", vbExclamation, "Cancelled"
        Exit Sub
    End If

    'Find last non-blank row
    lastRow = uppRowIndex
    For i = firstRow To uppRowIndex
        If Is2DArrayRowEmpty(arrData, i, True) Then
            lastRow = i - 1
            Exit For
        End If
    Next i
    If lastRow < firstRow Then
        MsgBox "No rows found after desired row", vbExclamation, "Cancelled"
        Exit Sub
    End If

    Dim app As New ExcelAppState: app.Sleep: app.StoreState

    'Prepare Target
    On Error Resume Next
    Set targetWS = ThisWorkbook.Worksheets.Add(After:=sourceWS)
    If Err.Number <> 0 Then
        MsgBox "Cannot insert new worksheet", vbExclamation, "Failed"
        Err.Clear
        app.RestoreState
        Exit Sub
    End If
    targetWS.Name = targetShtName
    If Err.Number <> 0 Then
        MsgBox "Cannot rename worksheet", vbInformation, "Renaming skipped"
        'This might not be critical so just continue
    End If
    On Error GoTo 0

    'Write data
    With rngUsed
        Range(.Cells(firstRow, 1), .Cells(lastRow, .Columns.Count)).Copy _
            Destination:=targetWS.Cells(1, 1)
    End With

    sourceWS.Delete
    app.RestoreState
End Sub

Public Function GetWorksheetByName(ByVal wsName As String, ByVal book As Workbook) As Worksheet
    On Error Resume Next
    Set GetWorksheetByName = book.Worksheets(wsName)
    On Error GoTo 0
End Function

Private Function Is2DArrayRowEmpty(arr() As Variant, ByVal rowIndex As Long, Optional ByVal ignoreEmptyStrings As Boolean = False) As Boolean
    Dim j As Long
    Dim v As Variant

    For j = LBound(arr, 2) To UBound(arr, 2)
        v = arr(rowIndex, j)
        Select Case VBA.VarType(v)
            Case VbVarType.vbEmpty
                'Continue to next element
            Case VbVarType.vbString
                If Not ignoreEmptyStrings Then Exit Function
                If v <> vbNullString Then Exit Function
            Case Else
                Exit Function
        End Select
    Next j
    Is2DArrayRowEmpty = True 'If code reached this line then row is Empty
End Function

and of course, a simplified version of the ExcelAppState class (that I linked to above). This one uses only the 3 application properties that you were using:
Option Explicit

Private m_calculationMode As XlCalculation
Private m_screenUpdating As Boolean
Private m_displayAlerts As Boolean

Private m_hasStoredState As Boolean
Private m_hasStoredCalcMode As Boolean

Public Sub StoreState()
    With Application
        On Error Resume Next 'In case no Workbook is opened
        m_calculationMode = .Calculation
        m_hasStoredCalcMode = (Err.Number = 0)
        On Error GoTo 0
        m_screenUpdating = .ScreenUpdating
        m_displayAlerts = .DisplayAlerts
    End With
    m_hasStoredState = True
End Sub

Public Sub RestoreState(Optional ByVal maxSecondsToWait As Integer)
    If Not m_hasStoredState Then
        Err.Raise 5, TypeName(Me) & ".RestoreState", "State not stored"
    End If
    With Application
        If m_hasStoredCalcMode Then
            On Error Resume Next
            If .Calculation <> m_calculationMode Then .Calculation = m_calculationMode
            On Error GoTo 0
        End If
        If .ScreenUpdating <> m_screenUpdating Then .ScreenUpdating = m_screenUpdating
        If .DisplayAlerts <> m_displayAlerts Then .DisplayAlerts = m_displayAlerts
    End With
    m_hasStoredState = False
End Sub

Public Sub Sleep()
    With Application
        On Error Resume Next
        If .Calculation <> xlCalculationManual Then .Calculation = xlCalculationManual
        On Error GoTo 0
        If .ScreenUpdating Then .ScreenUpdating = False
        If .DisplayAlerts Then .DisplayAlerts = False
    End With
End Sub

Public Sub Wake(Optional ByVal maxSecondsToWait As Integer = 10)
    With Application
        On Error Resume Next
        If .Calculation <> xlCalculationAutomatic Then .Calculation = xlCalculationAutomatic
        On Error GoTo 0
        If Not .ScreenUpdating Then .ScreenUpdating = True
        If Not .DisplayAlerts Then .DisplayAlerts = True
    End With
End Sub


Getting startup folders There are 2 versions of the get_startup_folders() method below, both work as expected and are called only once.

The 1st version has several minor pieces of repeated code which are removed by the use of an inner function in the 2nd version.

I've read several times that the use of inner functions is often frowned on by Python pros, except for in a narrow range of specific cases (e.g. closures and factory functions). I was wondering whether the 2nd version below would be considered superior to the 1st version by experienced Python programmers.

1st version:

def get_startup_folders(self):

    folders = []

    for item in STARTUP_FOLDERS:

        if item == "home":
            if self.home_folder not in folders:
                folders.append(self.home_folder)

        elif item == "file":
            if self.file_folder:
                if self.file_folder not in folders:
                    folders.append(self.file_folder)

        elif item == "project":
            for folder in self.project_folders:
                if folder not in folders:
                    folders.append(folder)

        elif item == "user":
            for folder in self.user_folders:
                if folder not in folders:
                    folders.append(folder)

    # Fail-safe: If empty then use the home folder as a fall back;
    # self.home_folder is guaranteed to contain the home folder.

    if len(folders) == 0:
        folders.append(self.home_folder)

    return folders

2nd version:

def get_startup_folders(self):

    folders = []

    def add_folder(folder):
        if folder and folder not in folders:
            folders.append(folder)

    for item in STARTUP_FOLDERS:

        if item == "home":
            add_folder(self.home_folder)

        elif item == "file":
            add_folder(self.file_folder)

        elif item == "project":
            for folder in self.project_folders:
                add_folder(folder)

        elif item == "user":
            for folder in self.user_folders:
                add_folder(folder)

    # Fail-safe: If empty then use the home folder as a fall back;
    # self.home_folder is guaranteed to contain the home folder.

    if len(folders) == 0:
        add_folder(self.home_folder)

   return folders

Use a set

Sets guaranty the unicity of the items it holds, adding the same item twice will result in the second add doing nothing:

def get_startup_folders(self):
    folders = set()
    for item in STARTUP_FOLDERS:
        if item == "home":
            folders.add(self.home_folder)
        elif item == "file":
            folders.add(self.file_folder)
        elif item == "project":
            folders.update(self.project_folders)
        elif item == "user":
            folders.update(self.user_folders)

    # Fail-safe: If empty then use the home folder as a fall back;
    # self.home_folder is guaranteed to contain the home folder.
    if not folders:
        folders.add(self.home_folder)

    return folders
    # or if you trully need a list: return list(folders)

If order matters, there is no such thing as OrderedSet, but collections.OrderedDict should be close enough. You just need to handle values that you won't keep:

import itertools
from collections import OrderedDict

def get_startup_folders(self):
    folders = OrderedDict()
    for item in STARTUP_FOLDERS:
        if item == "home":
            folders[self.home_folder] = None
        elif item == "file":
            folders[self.file_folder] = None
        elif item == "project":
            folders.update(zip(self.project_folders, itertools.repeat(None)))
        elif item == "user":
            folders.update(zip(self.user_folders, itertools.repeat(None)))

    # Fail-safe: If empty then use the home folder as a fall back;
    # self.home_folder is guaranteed to contain the home folder.
    if not folders:
        return [self.home_folder]

    return list(folders.keys())


Working with Mobile Device Orientation Detection One hassle with working with mobile websites is inconsistency between how the device reports the screen's width and height.  James Pearce wrote a fantastic article about the headaches that come with detecting screen width and height from JavaScript.  Furthermore, not all mobile devices support window.onOrientationChange and window.orientation members (and those that do don't necessarily do so consistently either).

Because of this, I'm working on a snippet to detect orientation and polyfill if it's not supported.  The issue that I have, though, is dealing with identifying when the orientation changes on multiple devices.

The below snippet works on small screens (e.g. Android and iPhones):

var newOrientation
, currentWidth = document.getElementsByTagName("body")[0].clientWidth;
// since there's bugs around devices supporting and reporting screen.width, using body instead

// set orientation
// TODO: Correct hard coded breakpoint.
if (window.hasOwnProperty("orientation")) { newOrientation = window.orientation; }
if (typeof newOrientation === "undefined") { newOrientation = (currentWidth > 400) ? 1 : 0; }

The code declares the newOrientation to test for the change against the currentOrientation variable (not shown).
Because of issue with screen.width, I'm grabbing the body tag's clientWidth property as a benchmark (CSS has body.width set to 100%)
Next, I set newOrientation to window.orientation if the browser supports such.
Finally, I check if newOrientation has been defined yet and if not, change it if the width is over a certain breakpoint (in this case, 400 which covers most phones).

I'm trying to find a better way to do point 4 since it doesn't scale well if I apply it to a Kindle Fire (600x1024) or an iPad2 (768x1024).  One thing preventing me from comparing height to width is how long pages would skew this logic.

Does anyone have any suggestions how I might be able to modify this to compensate for larger screen widths?

Update
After reviewing the linked article more and also taking into consideration GGG's suggestions, the code was refactored to the following:

var newWidth = window.outerWidth
  , newOrientation = window.orientation || (newWidth !== currentWidth) ? 1 : 0;

if (newOrientation !== currentOrientation) {
    currentOrientation = newOrientation;
    currentWidth = newWidth;
}

First, the code above can be simplified a lot, at least for the purposes of this "review," and rewritten as:

var currentWidth = document.body.clientWidth,
    newOrientation = window.orientation || (currentWidth > 400);

Second, I'd probably be really sure I'm getting a good width value by doing something like this:

var currentWidth = window.innerWidth || document.body.offsetWidth || document.documentElement.offsetWidth,
    newOrientation = window.orientation || (currentWidth > 400);

Next, none of these devices are perfectly square, right, so instead of checking against a constant, why not check if it's wider than it is tall?

var currentWidth = window.innerWidth || document.body.offsetWidth || document.documentElement.offsetWidth,
    currentHeight = window.innerHeight || document.body.offsetHeight || document.documentElement.offsetHeight,
    newOrientation = window.orientation || (currentWidth > currentHeight);

If I understand your question correctly, this should fix the problem with the larger screens.

Also note that this approach will only work reliably if the viewport is the same size as the physical screen. As you probably know, this can be accomplished with the viewport meta tag:

<meta name="viewport" content="width=device-width, initial-scale=1, maximum-scale=1">

If you're not setting a viewport, you'll get weird behavior. For example, some mobile browsers seem to keep the viewport settings from the last loaded site when a site with no viewport meta tag loads.


SqlAlchemy model for users and timezones I have a following schema, I would like to query user and all timezones belongs to it including timezone name. Could you please validate if my schema constructed correctly and most efficient way?

class Timezone(Base):
    __tablename__='timezone'
    id = Column(Integer, primary_key=True)
    name = Column(String(40), unique=True)

class UserTimezone(Base):
    __tablename__='user_timezone'
    id = Column(Integer, primary_key=True)
    user_id = Column(Integer, ForeignKey('users.id', ondelete='CASCADE'))
    timezone_id = Column(Integer, ForeignKey('timezone.id', ondelete='CASCADE'))

    timezone = relationship(Timezone, backref="timezone")

class User(Base):
    __tablename__='users'
    id = Column(Integer, primary_key=True)
    first_name = Column(String(64), index=False, unique=False)
    last_name = Column(String(64), index=False, unique=False)
    email = Column(String(120), index=True, unique=True)
    password = Column(String(120), index=False, unique=False)

    timezones = relationship(UserTimezone, backref="user_timezone")

I wouldn't bother making a separate table for timezones. That just makes you do joins unnecessarily. Just put the timezone name directly as an attribute of the UserTimezone table.


Display message that content depends on variable value I start external process and after its completion print a message to the user. The message content depends on exit code that this process returns.
I have defined this codes in enum:

public enum ExitCode {
    Success = 0,
    Warning = 1,
    Error = 2
}

I call some method only if the process exited with success.

I have 2 approaches.
First is to use switch statement

switch (process.ExitCode)
{
    case ExitCode.Success:
        Console.WriteLine("Success message.");
        CallSomeMethod();
        break;
    case ExitCode.Warning:
        Console.WriteLine("Warning message.");
        break;
    case ExitCode.Error:
        Console.WriteLine("Error message.");
        break;
    default:
        Console.WriteLine("Unknown exit code.");
        break;
}

Second way is to create dictionary with (StatusCode, messageContent) pairs

if (dictionary.TryGetValue(process.ExitCode, out msg))
{
    Console.WriteLine(msg);
    if (process.ExitCode == ExitCode.Success)
    {
        CallSomeMethod();
    }
}
else
{
    Console.WriteLine("Unknown exit code.");
}

Which version is more elegant? Feel free to reject both and propose better solution.

This answer applies only when you want to have well defined messages across whole application.

I would use neither.
Let me first explain why and then my proposition.

Critique

Assuming you are going to use your enum elsewhere some duplication might occur. Imagine souch example: you decided to use switch in some method in catch closure and this looks good because it is clean enough. However since you built an entire enum for simply displaying the message to the user you probably would want to use it in other method or class which is very handy.

What do I propose ?

I propose something what I like to call "descriptors", it is somewhat between dictionary and switch.
What would you do is create attributes over each enum with it's description :

public enum ExitCode {
    [Description("Success message")]
    Success = 0,
    [Description("Warning message")]
    Warning = 1,
    [Description("Error message")]
    Error = 2
}

and then add an extension method to your application (Source)

public static class EnumHelper
{
    public static string GetDesc(this ExitCode enumVal)
    {
        var type = typeof(ExitCode);
        var memInfo = type.GetMember(enumVal.ToString());
        var attributes = memInfo[0].GetCustomAttributes(typeof(DescriptionAttribute), false);
        return ((DescriptionAttribute)attributes[0]).Description;
    }
}

or similar. Hence you have beautiful alternative to your switch:

if(process.ExitCode == ExitCode.Success){
    CallSomeMethod();
}
Console.WriteLine(process.ExitCode.GetDesc());

Code is untested but you might get the idea. Furthermore, if you want to have more freedom with messages you can add second argument to extension method:public static string GetDesc(this ExitCode enumVal, string additionalComment)
and use it as such:

Console.WriteLine(process.ExitCode.GetDesc("More comment needed");


Calculate future date based on business days I've written a program for calculating the future date after certain number of business days.

function add_business_days($startdate,$businessdays,$holidays,$dateformat){
   $i=1;
   $dayx = strtotime($startdate);

  while($i < $businessdays){
     $day = date('N',$dayx);
     $date = date('Y-m-d',$dayx);
     if($day < 6 && !in_array($date,$holidays))$i++;
     $dayx = strtotime($date.' +1 day');
  }
  return date($dateformat,$dayx);
}

But the problem with this function is that it doesn't check the last date of business day for Saturday and Sunday. Also, the last day could be the holiday. Can anyone provide me a better solution for this?

I've made a solution to this problem as

function add_business_days($startdate,$businessdays,$holidays,$dateformat){
   $i=1;
   $dayx = strtotime($startdate);

  while($i <= $businessdays){
     $day = date('N',$dayx);
     $date = date('Y-m-d',$dayx);
     if($day < 6 && !in_array($date,$holidays))$i++;
     $dayx = strtotime($date.' +1 day');
  }
  return date($dateformat,strtotime(date('Y-m-d', $dayx) . ' -1 day'));
}

But it doesn't seems good.

Usage Example

I'm using this method for calculating the shipment date on which a product will be delivered to the customer.

$startdate = "2014-05-28";  //Order placing date
$businessdays = 7; //number of days for delivery
$holidays = array('2014-05-30','2014-06-03');  //array of dates having holidays. excluding business days.
$dateformat = 'd-m-Y';  //output date format for displaying.
$delivery = add_business_days($startdate, $businessdays, $holidays, $dateformat);

The $delivery is supposed to have date 09-06-2014 but the first code returns 07-06-2014 which doesn't check for the last date in loop for weekends (Saturday or Sunday). I've made the solution in the second program by execution the loop one more time and then return the previous date. But this approach doesn't seem good. Any suggestions on not going over and returning the previous date?

I propose a class, BusinessDayCalculator, which would handle all this logic in one place. Here's an example:

<?php

class BusinessDaysCalculator {

    const MONDAY    = 1;
    const TUESDAY   = 2;
    const WEDNESDAY = 3;
    const THURSDAY  = 4;
    const FRIDAY    = 5;
    const SATURDAY  = 6;
    const SUNDAY    = 7;

    /**
     * @param DateTime   $startDate       Date to start calculations from
     * @param DateTime[] $holidays        Array of holidays, holidays are no conisdered business days.
     * @param int[]      $nonBusinessDays Array of days of the week which are not business days.
     */
    public function __construct(DateTime $startDate, array $holidays, array $nonBusinessDays) {
        $this->date = $startDate;
        $this->holidays = $holidays;
        $this->nonBusinessDays = $nonBusinessDays;
    }

    public function addBusinessDays($howManyDays) {
        $i = 0;
        while ($i < $howManyDays) {
            $this->date->modify("+1 day");
            if ($this->isBusinessDay($this->date)) {
                $i++;
            }
        }
    }

    public function getDate() {
        return $this->date;
    }

    private function isBusinessDay(DateTime $date) {
        if (in_array((int)$date->format('N'), $this->nonBusinessDays)) {
            return false; //Date is a nonBusinessDay.
        }
        foreach ($this->holidays as $day) {
            if ($date->format('Y-m-d') == $day->format('Y-m-d')) {
                return false; //Date is a holiday.
            }
        }
        return true; //Date is a business day.
    }
}

$calculator = new BusinessDaysCalculator(
    new DateTime(), // Today
    [new DateTime("2014-06-01"), new DateTime("2014-06-02")],
    [BusinessDaysCalculator::SATURDAY, BusinessDaysCalculator::FRIDAY]
);

$calculator->addBusinessDays(3); // Add three business days

var_dump($calculator->getDate());

In the above example the nonBusinessDays of the week are SATURDAY and FRIDAY, and 1st and 2nd of June are holidays. In that case, 3 business days from now is June 5th.

This can probably be improved further, let your imagination run wild :P


Writing a proper (and simple) auth/login class in PHP How to write and design a simple (but still proper and secure) login class for PHP?

Currently I'm checking whether there is a login request (user entered data into login form) or the session is already existent and contains $_SESSION['authenticated'] == TRUE;. If both checks failed the user is not (properly) logged in. In code:

class Auth {
    private function __construct() {
        // new login
        if(isset($_POST['login'], $_POST['username'], $_POST['password']) && validCSRFToken()) {
            $user = $_POST['username'];
            $password = $_POST['password'];

            if(($row = querySingle('
               SELECT `id`, `password`
               FROM `users` u
               WHERE `nick` = "?"',
               $user)))
               // Crypt::hash checks the salted password
               && Crypt::hash($password, $row['password'])) {
                // credential change (guest -> user) regenerate session id
                session_regenerate_id(TRUE);

                $_SESSION['authenticated'] = TRUE;
                $_SESSION['userid'] = $row['id'];
                $_SESSION['user'] = $user;

                // prevent 'your browser has to re-send some data to display this page'
                redirect($_SERVER['REQUEST_URI']);
            } else {
                $this->logout();
                throw new AuthException('login failed. wrong user/password');
            }
        } else if($_SESSION['authenticated'] && $_SESSION['userid'] > 0 && !empty($_SESSION['user'])) {
            // user already logged in, nothing to see here, move on
        } else {
                    // not logged in!
                    throw new AuthException('not logged in. please login');
        }
    }

    public function logout() {
        $this->authenticated = FALSE;
        $this->user = '';
        $this->userid = 0;

        $this->session->destroy();
        $this->session->regenerate_id(TRUE);
    }
}

An object of the Auth class has to be created on every page that requires authentication from users:

try {
  $auth = new Auth();

  /*
   * rest of the page
   */

} catch(AuthException $ex) {
  // something went wrong during login
  displayError();
  displayLoginForm();
  exit; // halt further execution of script
}

Is this the right way to do it? Do you see anything (seriously) flawed with this approach? Can users impersonate other users? Users getting logged out randomly? Users getting logged out by malicious attackers? (although CSRF checks are in place). I'm afraid, I might be missing something obvious.

Any improvements apreciated, thanks!

(Please don't suggest using a PHP framework like CodeIgniter, Zend Framework, Kohana, etc.)

At the first look, no big hole shines through. Some points that can be considered, on the order of appearance:

Instead of directly using the $_POST, you can take these as parameters, thus making it look like more of an API. Example usage: when you want to directly log in people who have just  registered. You will not be tied to some particular $_POST parameters.
Crypt::hash ...: Not seeing the interior of this, one can't comment much but from the looks, the salt seems constant, unless you are querying the users table again inside this. A different (and preferably with a length matching the password) salt for each user is generally the way to go in security-conscious developer environments.
Similar to the #1, you can use a wrapper class for $_SESSION usage.
else if($_SESSION['authenticated'] && $_SESSION['userid'] > 0 && !empty($_SESSION['user'])): Instead of directly querying the $_SESSION, you can make another function like $this->isLoggedIn(), so you'd be more resistant to future session and log in changes (along with some other benefits).
redirect($_SERVER['REQUEST_URI']); Relying on a $_SERVER variable always looked insecure and unstable to me. You can use some checks here or use some of your internal navigation variables instead, in case you don't want to make it completely generic in this way.


Byte Stream implementation in C This is a byte stream implementation in C. Its usage and purpose should be evident from main.

Implementation is not finished; this is just start. Comments are still welcome.

.h:

#include <stdint.h>

typedef struct
{
    uint8_t * data;
    uint32_t length;
    uint32_t offset;
} ByteStream;

int32_t BSInitWithSize(ByteStream *bs, uint32_t len);
uint32_t BSNrOfRemainingBytes(ByteStream *bs);
int32_t BSFreeUnderlyingArray(ByteStream *bs);
void BSPrintContents(ByteStream *bs);
void BSRewind(ByteStream *bs);
int32_t BSPutU8(ByteStream *bs, uint8_t v);
int32_t BSGetU8(ByteStream *bs, uint8_t *out);
int32_t BSPutU16LE(ByteStream *bs, uint16_t v);

.c:

#include "ByteStream.h"
#include <stdlib.h>
#include <stdio.h>
#include <string.h>

int32_t BSInitWithSize(ByteStream *bs, uint32_t len)
{
    if (NULL == bs)
        return -1;

    // Allocate data array of sufficient length
    bs->data = (uint8_t*)malloc(len); // Note: Probably should check return value here
    memset(bs->data,0,len); // set values to nil

    bs->length = len;
    bs->offset = 0;

    return 0;
}

uint32_t BSNrOfRemainingBytes(ByteStream *bs)
{
    return bs->length - bs->offset;
}

void BSRewind(ByteStream *bs)
{
    bs->offset=0;
}

int32_t BSPutU8(ByteStream *bs, uint8_t v)
{
    if(BSNrOfRemainingBytes(bs)<1)
        return -1;

    bs->data[bs->offset]=v;
    bs->offset++;

    return 0;
}

int32_t BSGetU8(ByteStream *bs, uint8_t *out)
{
    if(BSNrOfRemainingBytes(bs)<1)
        return -1;

    bs->offset++;
    *out= bs->data[bs->offset-1];

    return 0;
}

int32_t BSPutU16LE(ByteStream *bs, uint16_t v)
{
    if(BSNrOfRemainingBytes(bs)<2)
        return -1;

    // This code compiles on LE machine, so
    // copying a integer to buffer will write
    // the integer into the buffer in a little endian way
    // exactly what this method is supposed to do
    // Note: Probably this is not portable
    memcpy(&bs->data[bs->offset],&v,2);
    bs->offset+=2;

    return 0;

}

void BSPrintContents(ByteStream *bs)
{
    int i = 0;
    for(i=0; i<bs->offset; i++)
    {
        printf("%02X ",bs->data[i]);
    }
}

int32_t BSFreeUnderlyingArray(ByteStream *bs)
{
    // Free underlying data array of BS object if it is not NULL
    if(NULL != bs->data)
    {
        free(bs->data);
        bs->data=NULL; // Set to NULL
        return 0;
    }

    return -1;
}

main:

#include "stdafx.h"
#include <stdio.h>
#include <stdlib.h>
#include <string.h>

extern "C"
{
#include "ByteStream.h"
}

int _tmain(int argc, _TCHAR* argv[])
{
    ByteStream b;
    memset(&b,0,sizeof(b));

    // Init bs
    BSInitWithSize(&b,14);

    // Add values
    for(int i=0;i<12;i++)
    {
        if(-1 == BSPutU8(&b, (uint8_t)i))
            printf("Error putting data \n");

        printf("Remaining Free Bytes: %d\n",BSNrOfRemainingBytes(&b));
    }

    // Rewind
    printf("---\n");
    BSRewind(&b);

    // Print values
    for(int i=0;i<12;i++)
    {
        uint8_t t;

        if(-1 == BSGetU8(&b, &t))
        {
            printf("ERROR Getting value \n");
        }
        else
            printf("Value at %d is %d \n", i, t);
    }

    // Put short integer in a little endian way
    if(-1 == BSPutU16LE(&b,765))
        printf("Error putting short \n");

    BSPrintContents(&b);

    // Free underlying buffer
    BSFreeUnderlyingArray(&b);

    return 0;
}

In many cases I decided to return error code from functions, and pass return values as out parameters.

A few things:

You are casting the result of your calls to malloc. That is something you shouldn't do actually.
If possible, try not to used fixed-size integer types (uint32_t, int32_t, etc...) for they are only conditionally supported by the compiler. For example, I doubt that you need a fixed-size type for your error codes. At least, use int_least32_t instead.
When you are not using any format string, you should consider using puts to printf (e.g. puts("---") instead of printf("---\n"). That's both shorther and safer.
Also, you initialize i to 0 twice in BSPrintContents. That seems pretty useless.


A solution to the Journey Planner programming challenge Here is my Javascript solution to the following problem:

  Given an un-ordered list of flights taken by someone compute the
  person's itinerary, given a starting airport.

  If none valid itinerary, return none.   If more than one journey is
  valid, print any valid solution.

Examples

Array: [['SFO', 'HKO'], ['YYZ', 'SFO'], ['YUL', 'YYZ'], ['HKO', 'ORD']]
Starting: 'YUL'
Return: ['YUL', 'YYZ', 'SFO', 'HKO', 'ORD'].

Array: [['SFO', 'COM'], ['COM', 'YYZ']]
Starting airport 'COM'
Return None

Array: [['A', 'B'], ['A', 'C'], ['B', 'C'], ['C', 'A']]
Starting: 'A'
Return ['A', 'B', 'C', 'A', 'C']

Please can someone comment on readability and efficiency of the code

Thank you

function plan_journey(place, journeys, route, itinerary){
    if(journeys.length == 0){
        itinerary.push(route + place)
    }
    let stops = []
    for(i=0; i<journeys.length; i++){
        if(journeys[i][0] == place){
            stops.push(i)
        }
    }
    if(stops == []){
        return
    }

    for(var i=0; i<stops.length; i++){
        let slice_index = stops[i]
        let remaining_journeys = journeys.slice(0,slice_index).concat(journeys.slice(slice_index + 1))
        let new_route = route + place + "-"
        let next_place = journeys[stops[i]][1]
        plan_journey(next_place, remaining_journeys, new_route, itinerary)
    }
    return itinerary
}

let first_stop = "A"
let journeys = [['A', 'B'], ['A', 'C'], ['B', 'C'], ['C', 'A']]
itinerary = plan_journey(first_stop, journeys, "", [])
if(itinerary.length != 0){
    console.log(itinerary[0])
}else{
    console.log("None")
}

Your current implementation is a brute force method, that has some hefty space performance problems (you are copying large parts of the journey every step of the way). Note that both the runtime and space costs are exponential in the worst case in this form.

This problem can be solved more elegantly through some of graph theory.
First we note that every location has to be entered the same number of times as it is leaved, except the end and start points which counts as an extra leaving and entry respectively. If this isn't the case then there is no solution, and if it is the case we are guaranteed for there to be a solution.
To find such a solution we can separate the locations into places that have only one way to enter and leave, and then the rest (remember that we have already identified our endpoint based on which place had one place less to leave). For those will multiple ways through, we need to find enough cycles that passes through other locations to have it returned to being a one pass in and out, such that we can have a route enter it, go through the cycles and then leave through the remaining exit.
The algorithm then is as follows:

Calculate the order (amount of flights entering and leaving) of each location.
Determine if it is possible to solve and identify the ending location.
Start from the starting point and go through any combination of journeys until you reach the end location (there is no way to screw yourself over on this one).
While there are still journeys left untaken, connect them in separate cycles, by just starting at any one of its locations and walk until you get back to the starting location.
Combine the results from results from step 3 and 4, by injecting the cycles from step 4 as parts of other routes when those other routes reach a location in such a cycle. Repeat this until there is only one route left, that is the result.

There are different ways to implement this, mainly based on how you deal with storing the information of the journeys, routes and locations. It should be possible to implement the above algorithm in such a way that it only require \$O(E)\$ runtime and extra space, with \$E\$ being the number of journeys.


(php , mysql) Contact us form mysql injection I made this anti SQL Injection, I tested if I can still send queries, and it is looks good. could you tell me if you see any problem I missed, before I launch this to the internet? (this is contact us form, no log in)

<?php

if(isset($_POST['submit'])) {

    $name = $_POST['username'];
    $email =  $_POST['email'];
    $text =  $_POST['textt'];

    $connection = mysqli_connect('localhost', 'root', '',  'database1');

    if($name && $email && $text ) {

        echo  "thanks you for contacting us, we will respond within 24 hours.";
    }
    else {
        echo  "Please enter name, email and your message.";

        // echo  $result;
    }

    $name = mysqli_real_escape_string($connection, $name);

    $email = mysqli_real_escape_string($connection, $email);

    $text = mysqli_real_escape_string($connection, $text);

    $query = "INSERT INTO info(username,email,textt) ";
    $query .= "VALUES ('$name', '$email', '$text')";

    mysqli_query($connection, $query);

}

?>

For security, bind user supplied values when including them in a query.

Here's what that could look like using mysqli and the information provided
https://www.php.net/manual/en/mysqli-stmt.bind-param.php

// renamed `textt` to `text`
$stmt = $mysqli->prepare('INSERT INTO `info` (`username`,`email`,`text`) VALUES (?,?,?);');
$stmt->bind_param($username, $email, $text);
$stmt->execute();
$stmt->close();

Here's what that would look like using PDO, should that connection be considered.
https://www.php.net/manual/en/pdostatement.bindparam.php

Perhaps consider renaming your column textt to something more descriptive?

Also, normally, you'd want to provide some validation on user supplied input to prevent bots and help prevent user error.

e.g. for the email address:

if (filter_var($_POST['email'], FILTER_VALIDATE_EMAIL)) {
  $email = $_POST['email'];
} else {
  // often handled with a message or alert funciton
  echo 'Please enter a valid email';
  $error = true;
}

e.g. for the username:

if(!ctype_space($_POST['username'])) {
    $username = trim($_POST['username']);
} else {
    // as before often handled by a message/alert function
    echo 'Please provide a valid email address';
    $error = true;
}

It's often helpful to store the IP address of the requester. The following can be spoofed, and there are better functions out there, but it is often better than nothing.

$ip_address = $_SERVER['REMOTE_ADDR'];

There are solutions to help ensure it's not a bot completing your form, e.g. reCAPTCHA and honey pots. They're not always appropriate from a UX, UI, security standpoint, but I feel that should be mentioned as well.

I hope this was helpful!


Efficient way to get value from a dataframe and append new dataframe I have a dataframe that have about 200 million rows. the example of dataframe is like this:

date         query
29-03-2019   SELECT * FROM table WHERE ..
30-03-2019   SELECT * FROM ... JOIN ... ON ...WHERE ..
....         ....
20-05-2019   SELECT ...

I have a function to get table(s) name, attribute(s) name from dataframe above and append to new dataframe.

import sqlparse
from sqlparse.tokens import Keyword, DML
def getTableName(sql):
    def getTableKey(parsed):
        findFrom = False
        wordKey = ['FROM','JOIN', 'LEFT JOIN', 'INNER JOIN', 'RIGHT JOIN', 'OUTER JOIN', 'FULL JOIN']
        for word in parsed.tokens:
            if word.is_group:
                for f in getTableKey(word):
                    yield f
            if findFrom:
                if isSelect(word):
                    for f in getTableKey(word):
                        yield f
                elif word.ttype is Keyword:
                    findFrom = False
                    StopIteration
                else:
                    yield word
            if word.ttype is Keyword and word.value.upper() in wordKey:
                findFrom = True
    tableName = []
    query = (sqlparse.parse(sql))
    for word in query:
        if word.get_type() != 'UNKNOWN':
            stream  = getTableKey(word)
            table   = set(list(getWord(stream)))
            for item in table:
                tabl = re.sub(r'^.+?(?<=[.])','',item)
                tableName.append(tabl)
    return tableName

and the function to get attribute is just like getTableName the different is the wordKey.

function to process dataframe is like this:

import pandas as pd
def getTableAttribute(dataFrame, queryCol, date):
    tableName       = []
    attributeName   = []
    df              = pd.DataFrame()
    for row in dataFrame[queryCol]:
        table       = getTableName(row)
        tableJoin   = getJoinTable(row)
        attribute   = getAttribute(row)
        #append into list
        tableName.append(table+tableJoin)
        attributeName.append(attribute)
    df = dataFrame[[date]].copy()
    df['tableName']      = tableName
    df['attributeName']  = attributeName
    print('Done')
    return df

The result of the function is like this:

date        tableName  attributeName
29-03-2019  tableN     attributeM
30-03-2019  tableA     attributeB
....        ...        ...
20-05-2019  tableF     attributeG

But as this is my first try, I need an opinion about what I've tried, because my code runs slow with large file.

getTableKey

I'm not sure it's good style to define functions inside other functions unless you are implementing some sort of closure:

# this is much easier to read as a separate function
# and you don't incur the cost of defining it every time
# you call the encapsulating function
def getTableKey(parsed):
    findFrom = False
    wordKey = ['FROM','JOIN', 'LEFT JOIN', 'INNER JOIN', 'RIGHT JOIN', 'OUTER JOIN', 'FULL JOIN']
    for word in parsed.tokens:
        if word.is_group:
            for f in getTableKey(word):
                yield f
        if findFrom:
            if isSelect(word):
                for f in getTableKey(word):
                    yield f
            elif word.ttype is Keyword:
                findFrom = False
                StopIteration
            else:
                yield word
        if word.ttype is Keyword and word.value.upper() in wordKey:
            findFrom = True

def getTableName():
    tableName = []
    query = (sqlparse.parse(sql))
    for word in query:
        if word.get_type() != 'UNKNOWN':
            stream  = getTableKey(word)
            table   = set(list(getWord(stream)))
            for item in table:
                tabl = re.sub(r'^.+?(?<=[.])','',item)
                tableName.append(tabl)
    return tableName

yield from syntax

Furthermore, instead of using for f in getTableKey(word): yield f, later versions of python3 introduced the yield from syntax:

def getTableKey(parsed):
    findFrom = False
    wordKey = ['FROM','JOIN', 'LEFT JOIN', 'INNER JOIN', 'RIGHT JOIN', 'OUTER JOIN', 'FULL JOIN']
    for word in parsed.tokens:
        if word.is_group:
            yield from getTableKey(word)

        # combine this, since it's exactly this combination that will yield
        # f, there's no elif or else
        if findFrom and isSelect(word):
            yield from getTableKey(word)
        # rest of func

This leverages less function calls and is faster:

import dis

def f():
    for i in range(10000):
        yield i

def g()
    yield from range(10000)

dis.dis(f)
2           0 SETUP_LOOP              22 (to 24)
              2 LOAD_GLOBAL              0 (range)
              4 LOAD_CONST               1 (10000)
              6 CALL_FUNCTION            1
              8 GET_ITER
        >>   10 FOR_ITER                10 (to 22)
             12 STORE_FAST               0 (i)

  3          14 LOAD_FAST                0 (i)
             16 YIELD_VALUE
             18 POP_TOP
             20 JUMP_ABSOLUTE           10
        >>   22 POP_BLOCK
        >>   24 LOAD_CONST               0 (None)
             26 RETURN_VALUE

dis.dis(g)
2           0 LOAD_GLOBAL              0 (range)
              2 LOAD_CONST               1 (10000)
              4 CALL_FUNCTION            1
              6 GET_YIELD_FROM_ITER
              8 LOAD_CONST               0 (None)
             10 YIELD_FROM
             12 POP_TOP
             14 LOAD_CONST               0 (None)
             16 RETURN_VALUE

To show the speed gain:

python -m timeit -s 'from somefile import f, g' 'list(f())'
1000 loops, best of 3: 507 usec per loop

python -m timeit -s 'from somefile import f, g' 'list(g())'
1000 loops, best of 3: 396 usec per loop

set vs list membership tests

Checking for membership in a list over and over is slow, worst case being O(N). To fix this, make word_list a set, which yields O(1) lookup:

python -m timeit -s "x = ['FROM','JOIN', 'LEFT JOIN', 'INNER JOIN', 'RIGHT JOIN', 'OUTER JOIN', 'FULL JOIN']" "'FULL JOIN' in x"
10000000 loops, best of 3: 0.0781 usec per loop

python -m timeit -s "x = set(['FROM','JOIN', 'LEFT JOIN', 'INNER JOIN', 'RIGHT JOIN', 'OUTER JOIN', 'FULL JOIN'])" "'FULL JOIN' in x"
10000000 loops, best of 3: 0.0246 usec per loop

So create the set like:

def getTableName(...):
   ~snip~
   wordKey = set(['FROM','JOIN', 'LEFT JOIN', 'INNER JOIN', 'RIGHT JOIN', 'OUTER JOIN', 'FULL JOIN'])

Though it might be even better to move this out of the getTableKey function entirely so you aren't paying for the re-construction of this set during every iteration:

# add a positional arg for it
def getTableKey(parsed, wordKey):
    # rest of func

And define it in getTableAttribute like:

def getTableAttribute(dataFrame, queryCol, date):
    wordKey = set(['FROM','JOIN', 'LEFT JOIN', 'INNER JOIN', 'RIGHT JOIN', 'OUTER JOIN', 'FULL JOIN'])
    ~snip~
    for row in dataFrame:
        table_name = getTableName(row, wordKey)

getTableName

There's no need to enclose sqlparse.parse in parens, as it will just default to whatever the enclosed value is:

x = (5)
x
5

You are losing speed calling set(list(iterable)), since set will consume any iterable, and it looks like getWord(stream) is an iterable already:

    table   = set(getWord(stream))

re.compile

If you are going to call a regex many times, it is better to compile it once, then call compiled.sub where compiled is the output of re.compile("<expression>"):

 python -m timeit -s 'import re; x = "abc123"' 'for i in range(100000): re.match("\w\d", x)'
10 loops, best of 3: 67.5 msec per loop

python -m timeit -s 'import re; x = "abc123"; y = re.compile("\w\d")' 'for i in range(100000): y.match(x)'
10 loops, best of 3: 28.1 msec per loop

To make this work, you might consider adding an arg in getTableName to allow for a compiled regex:

# somewhere in getTableAttribute.py file
import re
def getTableAttribute(dataFrame, queryCol, date):
    tableName       = []
    attributeName   = []
    table_re        = re.compile(r'^.+?(?<=[.])')
    df              = pd.DataFrame()
    for row in dataFrame[queryCol]:
        table       = getTableName(row, table_re)
        # rest of code

def getTableName(sql, re_expr):
    ...
    for item in table:
        tabl = re_expr.sub('', item)
        # rest of code


Remove particular element from an array and return indices of other elements I'd like to get the indices of elements of an array with particular element. Is it okay or, or how can I make it even better?

//string Input = "Net Sales,COGS";

int[] numbers = { 0, 1, 2, 3, 4, 5 };
string[] Values = { "Net Sales", "COGS", "COGS%", "Gross Profit", "GP%", "Quantity" };

string[] selectedValues = Input.Split(new char[] { ',' });
selectedIndex = new int[selectedValues.Length];
selectedIndex = selectedValues.Select(n => Array.IndexOf(Values, n)).ToArray();

for (int i = 0; i < selectedIndex.Length; i++)
    numbers = numbers.Where(val => val != selectedIndex[i]).ToArray();

selectedIndex = numbers;

First of all your code doesn't compile.

Second, your code doesn't do what you state - "Remove particular element from an array".

Third, what exactly do you mean by "Remove"? You want to set these elements to null or compact your array? In second case what do you need indexes of other elements for (their indexes will change when you compact your array)?

Ok, now lets start with a declaration of a contract:

You want to get a multivalue result: filtered array and indexes of remaining elements.

public struct Result
{
    public readonly string[] FilteredArray;
    public readonly List<int> IndexesOfRemainingElements;

    public Result(string[] filteredArray, List<int> indexesOfRemainingElements)
    {
        FilteredArray = filteredArray;
        IndexesOfRemainingElements = indexesOfRemainingElements;
    }
}

Now, I'd rewrite your method in this way:

static Result FilterArray(string[] array, string[] input)
{
    var remainingElements = array.Where(i => !input.Contains(i)).ToArray();

    return new Result(remainingElements,
                      remainingElements.Select(i => Array.IndexOf(array, i)).ToList());
}

And finally you can call it:

string input = "Net Sales,COGS";
string[] values = { "Net Sales", "COGS", "COGS%", "Gross Profit", "GP%", "Quantity" };

// Split has an override that accepts params char[]
var result = FilterArray(values, input.Split(','));

Complete example here: https://dotnetfiddle.net/4wbfpC


PHP member login system I have php cookie based login system on my website using below given code. I want to know that whether this login system is secure or not, Whether it can be hacked in any way?

<?php
session_start();
if(isset($_SESSION["userid"]))
{
 header("location:index.php");
}
include( $_SERVER['DOCUMENT_ROOT'] . '/conndb.php' );

if(isset($_POST["login"]))
{
 if(!empty($_POST["member_name"]) && !empty($_POST["member_password"]))
 {
  $name = mysqli_real_escape_string($conn, $_POST["member_name"]);
  $password = mysqli_real_escape_string($conn, $_POST["member_password"]);
$stmt_check = $conn->prepare("SELECT * FROM userz WHERE email =? AND pass = ? AND active ='1'");
$stmt_check->bind_param("ss", $name, $password);
$stmt_check->execute();
$stmt_check->store_result();
$numberofrows = $stmt_check->num_rows;

if(($numberofrows) > 0){

    setcookie ("member_login",$name,time()+ (10 * 365 * 24 * 60 * 60));
    setcookie ("member_password",$password,time()+ (10 * 365 * 24 * 60 * 60));
    $_SESSION["userid"] = $name;
   header("location:index.php");

} else {

   $message = "Invalid Login OR Email not Verified";
}

$stmt_check->close();

 }
 else
 {
  $message = "Both are Required Fields";
 }
}
?>

Besides this code, below piece of code is placed on each page to check whether the user is already logged in or not.

 <?php
    session_start();
    if (empty($_SESSION['userid'])) {
        header('Location: users/login.php');
        exit;
    }
    ?>

As the comments have said, writing a login systems securely is really hard. I would strongly recommend using an existing framework like one of the ones KIKO Software has put in his comment. With that disclaimer out of the way, let's have a look at the code.

<?php
session_start();
if(isset($_SESSION["userid"]))
{
 header("location:index.php");
}

Any time you use a redirect, it should be immediately followed by an exit(). If you don't do this the rest of the code will still be executed, and if the user's browser ignores the Location header they can still view the rest of the page. Not so bad here, but can lead to nasty access control issues. HTTP headers are normally capitalised with a space after the colon like Location: /index.php but I don't think there any (common) browser that would complain about this.

if(isset($_POST["login"]))
{
 if(!empty($_POST["member_name"]) && !empty($_POST["member_password"]))

A user could submit an array here for the username or password, rather than a string, which you probably don't want, as some PHP functions do strange things with arrays. If you only want a string, you should check for it here.

 {
  $name = mysqli_real_escape_string($conn, $_POST["member_name"]);
  $password = mysqli_real_escape_string($conn, $_POST["member_password"]);

The mysql_real_escape_string() function has some issues with certain character sets, so you need to be careful with it. But since you're using prepared statements, you don't need to escape these, and in fact if you escape them and they have certain characters like single quotes, you'll end up with the wrong values.

$stmt_check = $conn->prepare("SELECT * FROM userz WHERE email =? AND pass = ? AND active ='1'");

Is active a string? I'd expect that to be an integer (or even boolean), but you have single quotes around the 1, so it's being treated as a string by MySQL.

Doing SELECT * is normally a bad sign - you should only select the columns you actually need (which doesn't look like any in this case, you could probably get away with a count).

As mentioned in the comments, storing plaintext passwords is a really bad idea. You should be using a secure hashing function like bcrypt/scrypt/PBKDF2 to store passwords.

$stmt_check->bind_param("ss", $name, $password);
$stmt_check->execute();
$stmt_check->store_result();
$numberofrows = $stmt_check->num_rows;
if(($numberofrows) > 0){

Could the number of rows returned ever by more than 1? That would mean that you have duplicate users in your database, which would be a sign something has gone badly wrong. Unless there's something very strange in the database, it'd be better to check that exactly 1 row has been returned.

setcookie ("member_login",$name,time()+ (10 * 365 * 24 * 60 * 60));
setcookie ("member_password",$password,time()+ (10 * 365 * 24 * 60 * 60));

Storing the plaintext password (or any password) in a cookie is a very bad idea, especially when it's going to be stored for a ten years. Any cookies you're creating should also be marked as HttpOnly, Secure (assuming you're using SSL, which you really should be), and optionally SameSite.

$_SESSION["userid"] = $name;

It's good to be consistent internally with your variable names. So far the same information has been called member_name, name, email and userid

header("location:index.php");

See comments on previous header() usage.

} else {
  $message = "Invalid Login OR Email not Verified";
}

$stmt_check->close();

This could have been closed earlier, immediately after you get the number of rows.

As KIKO Software mentioned in his first comment, there are also some other security features that are lacking:

Account lockout
CAPTCHA or other brute-force protection
Session hijacking

Writing authentication code is really, really difficult, and if you solve all of these issues, there are still all kinds of other complexities like timing attacks, user enumeration, etc. You're much better off letting someone else deal with that pain.


Determining if f(n) = n^2 + 3n + 5 is ever divisible by 121 Given the following problem:

  It is conjectured that for any \$n > 0\$, \$n^2 + 3n + 5\$ is never
  divisible by 121. Test this conjecture for \$n = 1,2,...,9999,10000\$.

I wrote the following solution:

(defpackage :one-twenty-one-divisible (:use :cl))
(in-package :one-twenty-one-divisible)

(defun divisible (n) (= (mod n 121) 0))
(defun f_n (n) (+ (expt n 2) (* 3 n) 5))

(let ((divisible-result (loop for n from 1 to 10000 when (divisible (f_n n)) collect n)))
  (format t "The conjecture that for 1 <= n <= 10,000 f(n) = n^2 + 3n + 5 is indivisible by 121 is ~:[false~;true~].~%" (null divisible-result))
  (unless (null divisible-result) (format t "The following items were found to have f(n) that is divisible by 121: ~a ~%" divisible-result)))

What do you think?

I don't know Lisp, but you can speed up the calculation by observing that n² + 3n + 5 is never divisible by 11 (and hence never divisible by 121), except when n mod 11 == 4.

If you check the equation for the remaining candidates (11*k+4) in the "full" module 121 (e.g. using Lisp, but I did it in Excel), you get always an remainder of 33, so the conjecture is indeed true.

I would consider this exercise as "bad style", as it encourages the "brute force" instead of the "think" approach. There are enough not-so-easy to solve conjectures (e.g. Goldbach's conjecture) where falling back to brute force actually makes sense.

[More Explanation]

Modular arithmetic is described here, but I will try to give a short introduction: Let's say we want to know the remainder of the polynom by division by 11 when n is of the form 11*k + 3. We could write

(11*k+3)² + 3*(11*k+3) + 5

But as we are only interested in remainders, not the numbers itself, we "forget" all multiples of 11 and write in modular arithmetics:

3² + 3*3 + 5 | mod 11
9 + 9 + 5 | mod 11
23 | mod 11
1 | mod 11

This says that when we have a number 11*k + 3, our polynom will always have a remainder of 1 when divided by 11. n=3 gives a polynom value of 23, which is 1 mod 11. n=256  gives 66309, which is again 1 mod 11. As 121 = 11², if a number is not divisible by 11, it isn't divisible by 121 either. So no number n = 11*k + 3 is ever divisible by 121. Testing all the cases from n = 11*k + 0 to n = 11*k + 10 I found, that only for 11*k + 4 the polynom is divisible by 11. Checking all the values 11*k + 4 in module 121 (that are 121*u + v with v = 4, 15, 26, 37, 48, 59, 70, 81, 92, 103, 114) I found that all leave a remainder of 33, proving the conjecture (let's say we take n = 121 + 15 = 136, we get a polynom value of 18909, and as expected, a remainder of 33 when dividing by 121).

So there is nothing wrong with your code (and as I said I don't know Lisp), but in my opinion the "better" solution would be to say: It can be shown directly (even with paper and pencil) that the polynom is never divisible by 121, so no test is needed.


BBCode to HTML converter using functional programming I was inspired to write a BBCode to HTML converter as I'm currently learning functional programming. I wanted achieve functional cohesion. jsFiddle

I'd like feedback on:

structuring of the code. Should the functions getHtmlTag and getHtml be contained within bbTagToHtmlTag since they are coupled through the rules variable?
any regex expressions that can reduce the code base
any changes to the data model to reduce complexity

Below is the newer version of the code. This version is slightly less coupled compared to the original version and the functions are separated to allow for more reuse. This new version doesn't involve any recursion, so it may be more efficient.

I suspect the cost of reuse is an increase in code base. Although, I'm sure there's more regex magic that can be used to slim down the code base.

/* Escapes a string for use with regular expressions */
function escapeString(input) {
    return input.replace(/([\,\!\\\^\$\{\}\[\]\(\)\.\*\+\?\|\<\>\-\&])/g,
                         function(c){return "\\" + c;});
}

/* replaces all */
function replaceAll(search, input, replacement, ignore) {
   return input.replace(
       new RegExp(escapeString(search), "g"+(ignore?"i":"")),
       replacement);
};

/* Check if an index is event */
function isIndexEven(val, index, context) {
    return (index % 2 === 0);
};

/* Sets the attributes on an HTML string given a set of attributes */
function setHtmlAttrs(htmlCode, attrs) {
    return attrs.reduce(function(htmlCode, attr, index) {
        var re = new RegExp('\\$' + (index + 1), 'g');
        return htmlCode.replace(re, attr);
    }, htmlCode);
};

/* Gets the html tag(s) for a bbCode tag */
function eleFragsToHtml(isClosing, elementFragments) {
    return '<' + elementFragments.map(function(frag) {
        return (isClosing ? '/' + frag.join(' ') : frag.join(' '));
    }).join('><') + '>';
};

/* Converts a single bbCode Tag to Html */
function bbTagToHtmlTag(rules, tag) {
    var attrs = tag.replace(/\[\w+|\]/g, '').trim().split(' ');
    var key = tag.replace(/\[\/?| .+|\]/g, '');
    var isClosing = /\//.test(tag);
    var htmlTemplate = (isClosing ? rules[key].map(function(rule) {
                                        return rule.filter(isIndexEven)
                                }).reverse()
                        : rules[key]);
    var output = eleFragsToHtml(isClosing, htmlTemplate);
    return setHtmlAttrs(output, attrs);
};

/* Converts bbCode to HTML */
function bbCodeToHtml(rules, bbCode) {
    /* Creates the Regular Expression Search */
    var regex = new RegExp( ['\\[.?[',
                             Object.keys(rules).join('|'),
                             '].*?\\]'].join(''), 'g' );
    return bbCode.match(regex)
                 .reduce(function(htmlCode, tag) {
                     return replaceAll(tag,
                                      htmlCode,
                                      bbTagToHtmlTag(rules, tag));
                 }, bbCode);
};

Below is the test setup using the same rule set as the linked post.

var rules = {
    'b': [ ['strong'] ],
    'bg': [ ['span', 'style\"background-color:$1\"'] ],
    'big': [ ['span', 'style=\"font-size: larger;\"'] ],
    'c': [ ['span', 'style=\"color:$1\"'] ],
    'f': [ ['span', 'style=\"font-family:$1\"'] ],
    'i': [ ['em'] ],
    'img': [ ['a', 'href=\"$1\"'],
             ['img', 'src=\"$1\" style=\"max-height: 200px; max-width: 200px;\"'] ],
    'p': [ ['pre'] ],
    'small': [ ['small'] ],
    's': [ ['span', 'style=\"text-decoration: line-through;\"'] ]
};
var sample = '[big][big][c red]WARNING[/c][/big][/big][s]test[/s][img test.jpg][/img]';
$('#console').text(bbCodeToHtml(rules, sample));

Overall, it's interesting, but quite hard to read. The indentation is kinda funky, and it would be easier to parse with all those ternary branches. For instance, this:

var htmlTemplate = (isClosing ? rules[key].map(function(rule) {
                                    return rule.filter(isIndexEven)
                            }).reverse()
                    : rules[key]);

Could be

var htmlTemplate = rules[key];
if (isClosing) {
  htmlTemplate = htmlTemplate.map(function (rule) {
    return rule.filter(isIndexEven);
  }).reverse();
}

Similarly, a few more local variables wouldn't hurt. E.g.

return input.replace(new RegExp(escapeString(search), "g"+(ignore?"i":"")), replacement);

Could be

var flags = "g" + (ignore ? "i" : ""),
    regex = new RegExp(escapeString(search), flags);
return input.replace(regex, replacement);

More micro-level: The pattern in escapeString can be simplified a great deal; you don't have to escape everything inside a character class, nor do you need the capture group. I get something like this

/[\\,!^${}[\]().*+?|<>&-]/g

The only things that need escaping is the backslash, the close square bracket, and the dash - unless you just put the dash at the end (if it's in the middle somewhere, it'll be interpreted as a range like 0-9).

There's a similar thing in your rules; you don't need to escape double quotes inside single-quoted strings.

I should add, though, that I haven't had the time to do a high-level review of all the regexp magic; I've looked a the individual patterns, not whether some can be combined, elided, or otherwise refactored.
Speaking of the rules: Whenever possible use class names or simply rely on HTML tag names instead of hardcoded style attributes. For instance, the <big> tag exists in HTML, just like <small>, so you don't need a styled span element. And the img tag doesn't need a style attribute; just declare the style for img tags in a CSS file. The point is, as with all things CSS, that you won't have to change your code (behavior) to change the look (presentation).
Some function names are pretty terse. E.g. eleFragsToHtml is a bit too truncated, if you ask me (especially considering the 2nd argument is spelled out as elementFragments)
I'd define the unchanging regex patterns outside the functions they're in now, since they're static - or define some more functions, to make it more descriptive. For instance, this

var attrs = tag.replace(/\[\w+|\]/g, '').trim().split(' ');
var key = tag.replace(/\[\/?| .+|\]/g, '');
var isClosing = /\//.test(tag);

Could be

var attrs     = tagAttributes(tag),
    key       = tagName(tag),
    isClosing = tagIsClosing(tag);

(yes, I've combined the var statements).
The tag* prefix on all those functions is of course a hint that they might benefit from being namespaced in an object. Or just have one parseTag function that returns an object with name, attributes, and closing properties.
Lastly, it'd be nice to stick all of this in an IIFE, like

var bbCodeToHtml = (function () {
  // shared variables, regex patterns here

  // various helper functions

  return function (string) {
    // the function that ties it all together
  }
}());

The idea is that the global namespace isn't polluted with a lot of "niche" functions; all you'll see is a function called bbCodeToHtml.

FYI, it's a toss-up whether the function should be bbCodeToHTML or stay as it is. The DOM is inconsistent in this regard (you have the innerHTML and namespaceURI properties, but you also have functions like getElementById - and I doubt they mean the Freudian "id" but rather "ID".


DRYing out a basic image slider I built a very basic image slider, but I'm still having trouble refactoring.

I caught myself reusing same code for clicking the previous/next buttons, selecting a slider dot, and the automated sliding.  The difference between these actions is how the index of images changes.  I'm not sure how to break out that piece of code "efficiently".

Here's the sample code: JSFiddle

As an example of duplicate code that I'm not sure how to refactor efficiently:

If I want to select a slider dot:

$("#dots li").click(function() {
    var select = $("#dots li").index(this);
    dotItems.removeClass('active');
    listItems.eq(i).fadeOut(transition_speed);
    i = select;
    dotItems.eq(i).addClass('active');
    listItems.eq(i).fadeIn(transition_speed);
  });

And if I want to select the previous button:

$("#prev").click(function() {
    dotItems.removeClass('active');
    listItems.eq(i).fadeOut(transition_speed);
    i = i >= 0 ? i - 1 : listLen - 1;
    dotItems.eq(i).addClass('active');
    listItems.eq(i).fadeIn(transition_speed);
  });

Hope somebody can take a look at it and at least point me in the right direction.

My background: Worked in Finance a few years, currently self-studying web development ( < 1 year).  Still new to programming, and it may be that I'm still learning to think like a programmer.

Put it in a separate function, and call it whenever you want to change it.
I'm not a fan of using an application-scope variable to store the currently selected frame. You could use something like listItems.index(listItems.filter(":visible")) to get the index of the currently selected frame. However, if you want to keep using your way, then please name it something better.

I made a few changes. Of note:

I set display: none on #slider li instead of using listItems.not(':first').hide(). This is because when I loaded it the first time, all the images were briefly shown. This will prevent them from being rendered initially.
I changed the interval into a timeout that's refreshed upon changing frames. This way, if I click on a dot 4.5 seconds after the frame changes, the slider won't change my frame after just 0.5 seconds.

Here's a fiddle.

I made a few more changes in an updated version. Specifically:

By convention, constants are named in uppercase.
By convention, jQuery variables are prefixed with $.
I got rid of the selectedFrame variable and replaced it with a getSelectedIndex() function.
I made the dots dynamically added.

Here's a fiddle.


Calculating bytes of code I am new to Perl and I wrote this Perl script to calculate bytes of code and lines of code in a directory. I want to know if there are any improvements that can be made to it.

#!/usr/bin/perl
# codestats.pl
# A perl script to calculate the number of lines of code in a project
# and what type of lines those code are, such as comments, code, or
# newlines. This script is specific to C/C++, but it should work with any
# language that has the same commenting scheme, so it should probably work
# with java too.
#
# Henry J Schmale
# April 30, 2015

use strict;
use warnings;

my @srcext = ("*.cpp", "*.c", "*.h", "*.hpp", "*.ino", "*.cxx", "*.cc");

my $commLines = 0; # Lines that start with a comment symbol
my $bothLines = 0; # Lines that have a comment and code
my $newLines  = 0; # Lines that are just whitespace
my $codeLines = 0; # Lines of code - lines that don't fit in another space
my $totLines  = 0; # Total lines of code
my $srcBytes  = 0; # Total Number of bytes in src code
my $fCount    = 0; # Number of files read

my $files;
for($a = 0; $a < scalar(@srcext); $a++){
    $files .= `find . -name "$srcext[$a]"`;
}
my @inputs = split("\n", $files);
for($a = 0; $a < scalar(@inputs); $a++){
    my $prev = $totLines;
    countLines($inputs[$a]);
    printf("Read %d ln. In %s\n", $totLines - $prev, $inputs[$a]);
    $fCount++;
}
printResults();

# Count the lines in the given file
# The first param is the file to open for counting
sub countLines{
    my ($srcfile) = @_;
    open(FILE, "<$srcfile") or die "Couldn't open file: $!\n";
    my @lines = <FILE>;
    for($b = 0; $b < scalar(@lines); $b++){
        $srcBytes += length($lines[$b]);
        $totLines++;
        if($lines[$b] =~ /^\s$/){ # is only whitespace ==> newLine
            $newLines++;
            next;
        }
        if(($lines[$b] =~ /^\s*\/\//) || # comments only lines
           ($lines[$b] =~ /^\s*\/\*/) ||
           ($lines[$b] =~ /^\s*\*/)){
            $commLines++;
            next;
        }
        # code + comments
        if(($lines[$b] =~ /\/\//) ||
           ($lines[$b] =~ /\/\*.*\*\//)){
            $bothLines++;
            next;
        }
        $codeLines++;
    }
    close FILE;
}

sub calcPercent{
    return ($_[0] / $_[1]) * 100.0;
}

sub printResults{
    # print out the results
    printf("Read %d Files\n", $fCount);
    printf("Average Lines Per File: %d\n", $totLines / $fCount);
    printf("Code    : %09d ln. %06.3f", $codeLines, calcPercent($codeLines, $totLines));
    print "%\n";
    printf("Comment : %09d ln. %06.3f", $commLines, calcPercent($commLines, $totLines));
    print "%\n";
    printf("Blank   : %09d ln. %06.3f", $newLines,  calcPercent($newLines,  $totLines));
    print "%\n";
    printf("Both    : %09d ln. %06.3f", $bothLines, calcPercent($bothLines, $totLines));
    print "%\n";
    printf("Total   : %09d ln.\n", $totLines);
    printf("CodeSize: %09d bytes\n", $srcBytes);
}

It's easy to write very ugly Perl code,
but this is pretty nicely written, so congrats.

Using barewords for file handles like this is discouraged:

open(FILE, "<$srcfile") or die "Couldn't open file: $!\n";
my @lines = <FILE>;
# ...
close FILE;

The recommended way:

open(my $fh, "<$srcfile") or die "Couldn't open file: $!\n";
my @lines = <$fh>;
# ...
close $fh;

Some simplifications are possible.
For example instead of this:

my @srcext = ("*.cpp", "*.c");

I find this way a lot easier to type:

my @srcext = qw/*.cpp *.c/;

Another, bigger simplification is in the loops.
Instead of this:

my @lines = <FILE>;
for($b = 0; $b < scalar(@lines); $b++){
    $srcBytes += length($lines[$b]);
    $totLines++;
    if($lines[$b] =~ /^\s$/){ # is only whitespace ==> newLine
        $newLines++;
        next;
    }
    if(($lines[$b] =~ /^\s*\/\//) || # comments only lines
       ($lines[$b] =~ /^\s*\/\*/) ||
       ($lines[$b] =~ /^\s*\*/)){

You can iterate using for (<$fh>) { ... },
and instead of $lines[$b], benefit from the auto-variable $_,
which can be omitted from $lines[$b] =~ /.../ statements,
like this:

for (<$fh>) {
    $srcBytes += length($_);
    $totLines++;
    if(/^\s$/){ # is only whitespace ==> newLine
        $newLines++;
        next;
    }
    if(/^\s*\/\// || # comments only lines
       /^\s*\/\*/ ||
       /^\s*\*/){


Generating non-repeating random integers in a fixed range I was working on the "Robot Name" exercise over on exercism.io (neat site, by the way), the gist of which is: Generate random, unique strings that match /[A-Za-z]{2}[0-9]{3}/, e.g. "xY123", "Gv020", and so on. So there are 2,704,000 possible names out there.

Now, the exercise's tests do not do a full-on check for uniqueness or randomness, but making the names random and collision-free is something you're encouraged to try yourself. So I wanted to try that.

One option is of course to generate each and every name sequentially and shuffle them. But that seems pretty inelegant.

So went looking for something cooler and came across this neat method for generating random, non-repeating 32 bit integers using "quadratic prime residue" (which sounds like something you'd find on a mathematician's shoes).

That method is really neat, but it makes use of C's strict 32-bit ints, and some XOR magic that I can't use for the smaller range I'm dealing with here.

So I modified it a little bit, and came up with what's below. I did a quick and dirty test, printing all the names to a file, and running checking for uniqueness thusly:

$ sort -u names.txt | wc -l

and indeed it claims that there are 2,704,000 names after sorting and removing duplicates. So yay for that. Distribution is probably skewed all sorts of ways, but it appears to work, and at least it's not sequential.

But I'm wondering if there's a simpler way that I'm just missing here. Or, failing that, if there are improvements that could be made to the thing below.

Note that the code is very much fixed to the 0..2,704,000 range. This is on purpose (hence the hard-coded prime number etc.). I'm not really interested in tweaking this particular code to make it more flexible in that regard. But if someone has an already-flexible approach that could replace it, that'd be neat.

require "singleton"

class RobotNameGenerator
  include Singleton

  # 2x fifty-two letters (A-Z and a-z), 3 digits = 2,704,000 combinations
  BOUNDARY = 2_704_000

  # Largest prime below BOUNDARY that also satisfies p % 4 == 3
  PRIME = 2_703_983

  # Another prime used as step
  STEP = 23

  # Initializes the instance, and sets a random offset (seed value)
  def initialize
    @offset = rand(0...BOUNDARY)
  end

  # Get a new pseudo-random name
  def next
    serial = next_serial_number
    serial, digits = serial.divmod(1000)
    serial, b = serial.divmod(52)
    serial, a = serial.divmod(52)
    "%s%s%03i" % [letter_for(a), letter_for(b), digits]
  end

  private

  # Get the next serial number
  def next_serial_number
    serial = permute(permute(@offset % BOUNDARY))
    @offset += STEP
    serial
  end

  # Do a one-to-one permutation of a number
  def permute(n)
    return n if n > PRIME
    residue = n**2 % PRIME
    n <= PRIME / 2 ? residue : PRIME - residue
  end

  # Encode a 0-51 number to A-Z/a-z
  def letter_for(n)
    offset = n < 26 ? 'A'.ord : 'a'.ord
    ascii = offset + (n % 26)
    ascii.chr
  end
end

Any simple generator whose output is equivalent to its internal state will generate a non-repeating sequence. In fact, most will generate a permutation of the range [0, P) where P is the length of the period. Or [1, P), if state == 0 is forbidden.

Pick any simple generator with period P ≥ 2,704,000 and Bob's your uncle.

The simplest is the Golden Weyl generator (the 'Golden' is my moniker for it):

x = (x + 0x9E3779B9u) mod 2^32

The net has a wide choice of suitable Linear Congruential Generators (LCGs) of the form

x = (a * x + c) mod 2^32

Julienne Walker has written a brilliant explanation of LCGs, and the Weyl generators are just a special case with multiplier 1.

However, regardless of which type of generator you choose, its period P is likely to be too long.

Extracting a shorter non-repeating sequence of values [0,M) from a longer sequence of non-repeating values [0,P) is not trivial. In particular, if you map the [0,P) values to the [0,M) range in some fashion - e.g. by scaling, or by computing the remainder modulo M - then you lose the property of non-repetition. Stuffing P values into a smaller number M of pigeon holes means that some values must end up in the same hole.

One strategy that works well is rejection: inspect the generator output and if you don't like the value for some reason, throw it away and call the generator again. This method is perfect for generating even the most outlandish output distributions, in the sense that it does not introduce any bias at all and it leaves properties like non-repetition intact. The downside is that you have to do more work if a lot of stuff gets thrown away.

The trick, then, is to pick a non-repeating generator whose period length is as close to M = 2,704,000 as possible, and then use rejection with that.

LCGs modulo powers of 2 have the special property that any lower k bits cycle with period 2^k. I.e., the lowest bit is strictly alternating, the low 2 bits cycle with period 4, and so on. This means that these generators can be reduced modulo any power of 2 without losing the property of non-repetition. In other words, any LCG modulo 2^x is also a generator modulo 2^k for all k that are less than x. These subsequences can be extracted by throwing away (masking off) the unwanted higher bits.

The first power of two that is not less than the modulus M = 2,704,000 is 2^22. Masking with 2^22 - 1 = 0x3FFFFF we only have to reject the 35% of outputs that are in the range [M,2^22). This means the generator will have to be cycled 1.55 times on average for each good value in the range [0,M). That's less than two MULs for a non-repeating random value with an off-beat modulus.

Simple and effective.

An example for a suitable generator of higher quality (where the lower bits do not cycle with their own lower periods) is a 22-bit Tausworthe. Think of it as a single barrel of the four-barrelled lfsr113 that powers games like Dragon Age, whose long-awaited newest chapter is due to be released in a few hours.

x = (((x ^ (x << 1)) & 0x3FFFFF) >> (22 - 17)) ^ (x << 17);

return (x & 0x3FFFFF) - 1;

This generator has period 2^22 - 1 because 0 is a forbidden (sticky) state, hence the decrement after masking the output. The rejection method is then essentially the same as above.

I wrote (22 - 17) because the 17 corresponds to parameter s in the usual descriptions of the algorithm (and 22 equals k). Other possible choices for s are here: 1, 2, 4, 5, 7, 8, 10, 11, 13, 14, 16, 19 and 20. The algorithm is usually described in a way that makes the 'live' k bits sit at the upper end of the larger x:

x = ((x ^ (x << q)) >> (k - s)) ^ ((x & m) << s);

I took the liberty of shifting the whole shebang down to the lower end, so that the live bits can be extracted with simple masking. The output sequence is the same.


Rails controller method to accept one rating per IP address via AJAX My rating controller:

  def rate
    @ip = request.remote_ip
    @konkurrencer = Rating.where(:IP => @ip, :konkurrencer_id => params[:id])
    if @konkurrencer.empty?
      params[:kon][:IP] = request.remote_ip
      params[:kon][:konkurrencer_id] = params[:id]
      @konkurrencer = Rating.new(params[:kon])
      @konkurrencer.save
      @konkurrencer.konkurrencer.rating_score += params[:kon][:ratings].to_i
      @konkurrencer.konkurrencer.ratings += 1
      @konkurrencer.save
    else
      // render error message
    end
    render :nothing => true
   end
end

My AJAX:

 jQuery.ajax({
     url: frm.attr('action'), //your server side script
     data: frm.serialize(), //our data
     type: 'POST',
     success: function (data) {
          //write here something when everything went ok

     },
     error: function (jxhr, msg, err) {
         alert(msg); //something went wrong.
     }
 });

Have do I make my controller more DRY?

Controller code looks good.. You only need to refactor code little bit so that it will look more clean & robust. otherwise rest things are ok. regarding ajax response, you need to implement success/failure response either inside controller or js file.

just to give idea on implementing ajax response inside controller

def update
  render :update do |page|
    if success
      # here replace_html is prototype helper method
      page.replace_html "DIV_ID", "PARTIAL"
      # if you are using jquery then you can execute js code like
      page << "js code goes here"
    else
      page.replace_html "ERR_DIV", "Error occured"
    end
  end
end


Fibonacci sequence array in Javascript This is my third day of learning Javascript through an online class. I learned about arrays and while/for loops today, and to practice all of that stuff the challenge I'm working on is to make a function that produces \$(n)\$ terms of the Fibonacci sequence in an array.
So I'm hoping for some feedback on this code! I think it works but I would like to know how I could make it more efficient or better in any way.
function fibonacciGenerator (n) {
    var output=[];
    if (n===1) {
      output=[0];
    } else if (n===2) {
      output=[0,1];
    } else if (n>2) {
      output=[0,1];
      for (var i=3;i<=n;i++) {
        output.push(output[i-3]+output[i-2]);
      }
    }
    return output;
}

For a beginner this code looks okay. Good job using strict equality comparisons - i.e. ===. The indentation is mostly consistent, although the first indentation level is 4 spaces while all subsequent levels appear to be 2 spaces.
The biggest thing I would change is spacing. For the sake of readability, it helps to add spacing around operators - especially binary operators like ===, >, =, etc. You could utilize a linter like ESLint, JSHint, etc. ESLint has rules like space-infix-ops.

While formatting preferences are very personal, a number of style
guides require spaces around operators, such as:
var sum = 1 + 2;

The proponents of these extra spaces believe it make
the code easier to read and can more easily highlight potential
errors, such as:
var sum = i+++2;

While this is valid JavaScript syntax, it is hard to
determine what the author intended.

1
I tried passing 0 to the function and it returned [0, 1], which is the same output if I pass 2. Theoretically it should return an empty array when 0 is passed.
The first and second else cases could be simplified to just an else, since the loop won't run unless i is less than n and for the case of 2 it would never be the case that 3 is less than 2. So it can be simplified like this:
function fibonacciGenerator (n) {
    var output = [];
    if (n === 1) {
      output = [0];
    } else {
      output = [0, 1];
      for (var i=3; i <= n; i++) {
        output.push(output[i-3] + output[i-2]);
      }
    }
    return output;
}

In more advanced JavaScript the const keyword could be used to declare output so it can't be over-written, which would require exclusively using the .push() method to add elements to the output array. Using const when re-assignment isn't needed is a good habit because it can help avoid accidental re-assignment and other bugs.
Alternatively, the function could return early in the base cases - e.g. when n is 1.