Minimize the number of workers/test rigs, with a capacity constraint, using Excel VBA Solver loop I created a code that runs 60 optimizations through solver through 4 series of loops (each with 15 'iterations'). The code works great but it is taking FOREVER to run through (over an hour). Each optimization is a simple linear model (global solution is found), just changing which month I am looking at.

Problem setup: I am trying to minimize the number of workers/test rigs that will satisfy the intake with the constraint of being under capacity.

I am not sure how I can make it go any faster, but I cannot send this to other people and expect them to use it. Does anybody have any suggestions? I have time sequenced it and each iteration takes equally as long. There is no bottleneck.

Below is my code:

Sub Optimization()

Application.ScreenUpdating = False

'Unlocks workbook to allow updating
Call Unlock_Workbook

'Makes visible and selects the tab where the optimization problem is set up
Sheets("Optimization").Visible = True
Sheets("Optimization").Select

'Clear variable ranges that solver will change
Range("Worker_All[[1]:[15]]").Clear
Range("TestRig_All[[1]:[15]]").Clear
Range("Worker_787[[1]:[15]]").Clear
Range("TestRig_787[[1]:[15]]").Clear

'Install the Add-in for users who have no done this already
AddIns("Solver Add-in").Installed = True

'Optimized All Workers

For i = 1 To 15

'Cell address for objective
Min = Cells(3, 2 + i).Address
'Cell adress for variable
Variable = Range("Worker_All[" & i & "]").Address
'Cell address for constraint range
ConstraintRange = Range("IntakeHours_NonKeyWO[" & i & "]").Address
'Cell address for constrants
Constraint = Range("IntakeHours_NonKeyWOC[" & i & "]").Address

SolverReset

SolverOk SetCell:=Min, MaxMinVal:=2, ValueOf:=0, ByChange:=Variable, _
    Engine:=2, EngineDesc:="Simplex LP"
SolverAdd CellRef:=ConstraintRange, Relation:=1, FormulaText:=Constraint
SolverSolve True

Next i

'Optimized All Test Rigs

For i = 1 To 15

Min = Cells(4, 2 + i).Address
Variable = Range("TestRig_All[" & i & "]").Address
ConstraintRange = Range("IntakeHours_NonKeyMO[" & i & "]").Address
Constraint = Range("IntakeHours_NonKeyMOC[" & i & "]").Address

SolverReset

SolverOk SetCell:=Min, MaxMinVal:=2, ValueOf:=0, ByChange:=Variable, _
    Engine:=2, EngineDesc:="Simplex LP"
SolverAdd CellRef:=ConstraintRange, Relation:=1, FormulaText:=Constraint
SolverSolve True

Next i

'Optimized 787 Workers

For i = 1 To 15

Min = Cells(5, 2 + i).Address
Variable = Range("Worker_787[" & i & "]").Address
ConstraintRange = Range("IntakeHours_Key787WO[" & i & "]").Address
Constraint = Range("IntakeHours_Key787WOC[" & i & "]").Address

SolverReset

SolverOk SetCell:=Min, MaxMinVal:=2, ValueOf:=0, ByChange:=Variable, _
Engine:=2, EngineDesc:="Simplex LP"
SolverAdd CellRef:=ConstraintRange, Relation:=1, FormulaText:=Constraint
SolverSolve True

Next i

'Optimized 787 Test Rigs

For i = 1 To 15

Min = Cells(6, 2 + i).Address
Variable = Range("TestRig_787[" & i & "]").Address
ConstraintRange = Range("IntakeHours_Key787MO[" & i & "]").Address
Constraint = Range("IntakeHours_Key787MOC[" & i & "]").Address

SolverReset

SolverOk SetCell:=Min, MaxMinVal:=2, ValueOf:=0, ByChange:=Variable, _
    Engine:=2, EngineDesc:="Simplex LP"
SolverAdd CellRef:=ConstraintRange, Relation:=1, FormulaText:=Constraint
SolverSolve True

Next i

Sheets("Cell Summary").Select
Sheets("Optimization").Visible = False

Call Lock_Workbook

Application.ScreenUpdating = True

End Sub

Let's talk about simple things that can be fixed by tools and then let's talk about performance:

I ran your code through Rubberduck's Online Analyzer [1] and got a lot of results. Most of them were related to the missing definitons for SolverAdd, SolverReset, SolverSolve and SolverOk but there were some more results:

You're not using Option Explicit. This means you could mistype variable (and function/sub) names and only realize at runtime. This is especially dangerous with variables, since VBA just "on-the-fly" declares that mistyped variable as a Variant.
You're using implicit references to ActiveSheet and ActiveWorkbook in your calls to Range and Sheets
Sheets may contain elements you don't expect. Generally it's more correct to use WorkSheets instead
Optimization is "implicitly public". While this is probably exactly what you need, it's usually a better idea to be upfront about this and explicitly declare it as public.

Additionally the code inside the Sub is not indented by one level, which makes it hard to distinguish the Sub from surrounding procedures. Furthermore Rubberduck should've flagged the use of Call, which is obsolete.

Since we've cleared about everything that can be easily addressed by more or less intelligent tools, let's get into the meat of the code and start optimizing:

First things first, we stop working with ActiveSheet and Select, because they are slow and can result in really funny undesired behaviour:

Dim optimizationSheet As Worksheet
'still references ActiveWorkbook, but I don't know what your workbook is called :/
Set optimizationSheet = Worksheets("Optimization")

Then we can make the clearing of "working ranges" a bit cleaner and faster:

With optimizationSheet
    .Range("Worker_All[[1]:[15]]").Clear
    .Range("TestRig_All[[1]:[15]]").Clear
    .Range("Worker_787[[1]:[15]]").Clear
    .Range("TestRig_787[[1]:[15]]").Clear
End With

This is minimally faster, because VBA knows it can reuse the optimizationSheet reference. In addition to that it doesn't access the ActiveSheet anymore, which is really good.

The interesting part of optimization comes now. Unfortunately you did not provide the code for your Solver, which is most probably the actual bottleneck of this, but even so we can get some minor speed out of this by declaring the variables we use with their proper types.

The true improvement will lie within the fact that you're abusing the EXCEL-Sheet as a holder for your variables. Instead of doing that (which is degrading performance more than you might imagine) the Solver should be rewritten to perform it's task on Arrays instead of taking the Address of a Range on the sheet and then use the cells of the Worksheet.
A general rule is to avoid using the Worksheet whenever you can, because it's so mindbogglingly slow.

I strongly recommend that you post the code of the solver after refactoring it to not use the Worksheet as data storage for a proper review ;)

  [1] I'm involved in Rubberduck as Translator and occasional Dev


Reading a sequence of characters and dynamically splitting into multiple arrays I am trying to read a sequence of random characters, integers and symbols from the standard inputstream, split it into 3 separate arrays (uppercase, lowercase and numeric) and print them in that order using standard Java API and JDK7.

Is there any other(better) way of dong the same?

BufferedReader bufferedReader = new BufferedReader(new InputStreamReader(System.in));
String characters = bufferedReader.readLine();

char[] chars = characters.toCharArray();
List<Character> characterList = new ArrayList<>(chars.length);

for (char c : chars) {
    if (Character.isUpperCase(c)) {
        characterList.add(c);
    } else if (Character.isLowerCase(c)) {
        characterList.add(c);
    } else if (Character.isDigit(c)) {
        characterList.add(c);
    }
}

StringBuilder uppercaseStringBuilder = new StringBuilder();
StringBuilder lowercaseStringBuilder = new StringBuilder();
StringBuilder numericStringBuilder = new StringBuilder();

for (Character character : characterList) {
    if (Character.isUpperCase(character)) {
        uppercaseStringBuilder.append(character);
    } else if (Character.isLowerCase(character)) {
        lowercaseStringBuilder.append(character);
    } else if (Character.isDigit(character)) {
        numericStringBuilder.append(character);
    }
}

char[] uppercaseChars = uppercaseStringBuilder.toString().toCharArray();
char[] lowercaseChars = lowercaseStringBuilder.toString().toCharArray();
char[] numericChars = numericStringBuilder.toString().toCharArray();

for (char uc : uppercaseChars) {
    System.out.print(uc);
}

System.out.println();

for (char lc : lowercaseChars) {
    System.out.print(lc);
}

System.out.println();

for (char nos : numericChars) {
    System.out.print(nos);
}
}

First obvious thing: Why do you do the almost same thing twice?

for (char c : chars) {
    if (Character.isUpperCase(c)) {
        characterList.add(c);
    } else if (Character.isLowerCase(c)) {
        characterList.add(c);
    } else if (Character.isDigit(c)) {
        characterList.add(c);
    }
}

...and...

for (Character character : characterList) {
    if (Character.isUpperCase(character)) {
        uppercaseStringBuilder.append(character);
    } else if (Character.isLowerCase(character)) {
        lowercaseStringBuilder.append(character);
    } else if (Character.isDigit(character)) {
        numericStringBuilder.append(character);
    }
}

You are going through an array, check if you want the character and add it to a list. Then you go through that list and make the same check again to put the character in different lists / StringBuilders. You can simply remove the last of these loops and add the characters directly from the array to the stringbuilders.

Also your output would be pretty much the same if you simple wrote, for example...

System.out.println(uppercaseStringBuilder.toString());

I guess, with Java8, your problem could be solved nicely (or not so nicely, since char primitives are a little bit tricky there) via Streams.


Memory management for fetching a list of drive letters on Windows I'm using the WinAPI GetLogicalDriveStrings() function that requires a LPWSTR and am wondering if there is a safer way to do this to ensure there is no memory leak.

Currently I construct an initial pointer to a buffer buf using:

auto buf = GetLogicalDriveStrings(0, nullptr);

Then I create the LPWSTR to be used in lieu of the null pointer in my actual call using:

auto driveStrings = static_cast<LPWSTR>(malloc((buf + 1) * sizeof(WCHAR)));

Next I create a pointer to driveStrings to free it later. After checking if driveStrings is a null pointer or if the buffer (buf) is NULL (in case memory couldn't be allocated), I call GetLogicalDriveStrings() using driveStrings.

After I get the result I manually free() the LPWSTR using the pointer I made after allocating it.

How can I use a smart pointer for LPWSTR instead so I don't have to use malloc() and free(), but so that it will still work with the GetLogicalDriveStrings() function?

Mininum working example:

auto buf = GetLogicalDriveStrings(0, nullptr);

auto driveStrings = static_cast<LPWSTR>(malloc((buf + 1) * sizeof(WCHAR)));
auto pDriveStrings = driveStrings;

if (driveStrings == nullptr || buf == NULL)
{
    std::stringstream msg;
    msg << "Can't allocate memory for drive list: ";
    msg << GetLastError();
    throw std::runtime_error(msg.str());
}

// get drive strings
if (GetLogicalDriveStrings(buf, driveStrings) == NULL)
{
    std::stringstream msg;
    msg << "GetLogicalDriveStrings error: ";
    msg << GetLastError();
    throw std::runtime_error(msg.str());
}

// iterate over results
while (*driveStrings)
{
    // GetDriveType() requires a LPCWSTR
    if (GetDriveType(driveStrings) == DRIVE_FIXED || GetDriveType(driveStrings) == DRIVE_REMOVABLE)
    {
        std::wcout << driveStrings << std::endl;
    }
    driveStrings += lstrlen(driveStrings) + 1;
}

free(pDriveStrings);

If I use a std::wstring, I can't figure out how to iterate over each string in the driveStrings buffer. If I use a std::vector<WCHAR>, I can't figure out how to cast each element to an LPCWSTR for GetDriveType().

This works fine as is, but is there a better/safer way to do this? I'm open to any and all improvements.

Cross-posted on StackOverflow, but it has close votes because it is a better fit here.

Your code is leaking memory. If you throw an exception, free no longer gets called. Using a std::vector makes sure the memory is freed.

Iterating through the array returned is quite straight-forward, considering that each drive letter has a null-terminator and than the whole vector has an extra null-terminator (requiredSize = writtenChars + 1)

auto requiredSize = GetLogicalDriveStringsW(0, nullptr);

std::vector<WCHAR> buffer(requiredSize + 1);
auto writtenChars = GetLogicalDriveStringsW(requiredSize, buffer.data());

if (writtenChars > requiredSize)
{
    //MSDN: If the buffer is not large enough, the return value is greater than nBufferLength.
}

if (writtenChars == 0)
{
    //handle error
}

const WCHAR* currentString = buffer.data();

while (*currentString != 0)
{
    std::wcout << currentString << L'\n';
    //pass currentString to other methods
    currentString += wcslen(currentString) + 1;
}


CSS Good Practice on a JavaScript Plugin I've realized a jQuery Plugin recently and made a basic default CSS for it.

As I'm mainly a server-side guy, I'm not too familiar with CSS and would like to have insight about what could be improved.

The main issues I've targeted are :

Are the CSS selectors appropriate, knowing that this CSS is meant to be on a plugin and to be side-effect-less as much as possible.
Are the property usages appropriate or are there better way of using some of the properties I have used.

Any suggestion of code style is also highly appreciated!

Here is a working fiddle

Here is the css :

div.paginateContainer {
    display : block;
    text-align: center;
}

div.paginateContainer > ul.paginateList{
    display: inline-block;
}

div.paginateContainer > ul > li{
    display: inherit;
}

div.paginateContainer > ul > li > a.paginateAnchor{
    font-family: "Helvetica Neue",Helvetica,Arial,sans-serif;
    text-decoration: none;
    cursor: pointer;

    line-height: 20px;
    padding: 4px 12px;
    background-color: #ddd;
    color: #08c;
    font-size : 16px;
    min-width: 16px;
    float: left;

    border-top: 1px solid #ccc;
    border-bottom: 1px solid #ccc;

    /* Select None */
    -webkit-touch-callout: none;
    -webkit-user-select: none;
    -khtml-user-select: none;
    -moz-user-select: none;
    -ms-user-select: none;
    user-select: none;
}

div.paginateContainer > ul > li > a.paginateAnchor:hover {
    color : #fff;
    background-color: #2ae;
    border-top: 1px solid #2ae;
    border-bottom: 1px solid #2ae;
}

div.paginateContainer > ul:first-child > li.paginatePrevious > a{
    border-top-left-radius: 10px;
    border-bottom-left-radius: 10px;
    border: 1px solid #ccc;
    font-size : 18px;
    font-weight: bold;
}

div.paginateContainer > ul:first-child > li.paginatePrevious > a:hover {
    border: 1px solid #2ae;
}

div.paginateContainer > ul:first-child > li.paginateNext > a {
    border-top-right-radius: 10px;
    border-bottom-right-radius: 10px;
    border: 1px solid #ccc;
    font-size : 18px;
    font-weight: bold;
}

div.paginateContainer > ul:first-child > li.paginateNext > a:hover {
    border: 1px solid #2ae;
}

div.paginateContainer > ul:last-child > li > a {
    border-radius: 10px;
    font-size: 18px;
    font-weight: bold;
}

div.paginateContainer ul.paginateList:last-child {
    float: right;
}

div.paginateContainer > ul > li.paginateActive > a,
div.paginateContainer > ul > li.paginateActive > a:hover {
    color : #fff;
    cursor: default;
    background-color: #08c;
    border-top: 1px solid #08c;
    border-bottom: 1px solid #08c;
}

div.paginateContainer > ul:first-child > li.paginateDisabled > a,
div.paginateContainer > ul:first-child > li.paginateDisabled > a:hover {
    color : #bbb;
    background-color: #eee;
    cursor: default;
    border: 1px solid #ddd;
}

Note that I've posted a question to review the JavaScript also, so this post is only about CSS. The JavaScript having no real dependency with the CSS in this case.

You styles are pretty solid, it's mainly your selectors that have issues I feel. Here are some notes:

It's normally best to not include the type of element if it has a class. This is for maintainability and extensibility purposes, you could switch in
 for
 with less work for example. Inversely, if all
's have the same style, they don't even need the same class.
You never want to go too deep with you CSS selectors, more specific is less flexible.

/* Bad */
div.paginateContainer > ul > li.paginateActive > a

/* Good*/
.paginateContainer .paginateActive a

Avoid the use of the direct descendant > selector where ever possible, again it just locks your CSS down and makes it less flexible. I recommend only ever using this if you need it to add styles to a legacy application or have a situation where you have a hierarchy something like this that doesn't make sense to add different classes.

These target different elements

.menu > li { /* ... */}
.menu li { }

See in this situation putting a .submenu class on the inner
 won't make a different, .menu li will still target both.
Try to keep your selectors consistent, for example you refer to both ul.paginateList and ul to mean the same thing.
I would add a different class to your first
 so you don't need to refer to it as ul:first-child, you may want to change the order in the future for example. Maybe only include .paginateList on the first
.
Due to your use of specific pixel font sizes your plugin may require modification in order to be used on someone's site. Generally these days we use 'ems' which represent the size of the character 'M'. Currently your font-size is fixed at 18px and it will not change whatever site you put it on, using font-size:1.2em for example would scale to be 20% larger than the font-size of the element containing .paginateContainer.

It also allows for better mobile support, taking advantage of the default font size set by the user. This does require a bit of work though as you should apply it to padding, border-radius, line-height, font-size and min-width (everything except border-size).

And here is your modified CSS

.paginateContainer {
    display : block;
    text-align: center;
}

.paginateContainer ul {
    display: inline-block;
}

.paginateContainer li {
    /* Better to state the display explicitly */
    /*display: inherit;*/
    display:inline-block;
}

.paginateContainer a {
    font-family: "Helvetica Neue",Helvetica,Arial,sans-serif;
    text-decoration: none;

    /* Put the cursor rule on :hover */
    /*cursor: pointer;*/

    line-height: 20px;
    padding: 4px 12px;
    background-color: #ddd;
    color: #08c;
    font-size : 16px;
    min-width: 16px;
    float: left;

    border-top: 1px solid #ccc;
    border-bottom: 1px solid #ccc;

    /* Select None */
    -webkit-touch-callout: none;
    -webkit-user-select: none;
    -khtml-user-select: none;
    -moz-user-select: none;
    -ms-user-select: none;
    user-select: none;
}

.paginateContainer a:hover {
    color : #fff;
    background-color: #2ae;
    border-top: 1px solid #2ae;
    border-bottom: 1px solid #2ae;
    cursor:pointer;
}

.paginateContainer ul:first-child .paginatePrevious a {
    border-top-left-radius: 10px;
    border-bottom-left-radius: 10px;
    border: 1px solid #ccc;
    font-size : 18px;
    font-weight: bold;
}

.paginateContainer ul:first-child .paginatePrevious a:hover {
    border: 1px solid #2ae;
}

.paginateContainer ul:first-child .paginateNext a {
    border-top-right-radius: 10px;
    border-bottom-right-radius: 10px;
    border: 1px solid #ccc;
    font-size : 18px;
    font-weight: bold;
}

.paginateContainer ul:first-child .paginateNext > a:hover {
    border: 1px solid #2ae;
}

.paginateContainer ul:last-child a {
    border-radius: 10px;
    font-size: 18px;
    font-weight: bold;
}

.paginateContainer ul:last-child {
    float: right;
}

.paginateContainer .paginateActive a,
.paginateContainer .paginateActive a:hover {
    color : #fff;
    cursor: default;
    background-color: #08c;
    border-top: 1px solid #08c;
    border-bottom: 1px solid #08c;
}

.paginateContainer ul:first-child .paginateDisabled a,
.paginateContainer ul:first-child .paginateDisabled a:hover {
    color : #bbb;
    background-color: #eee;
    cursor: default;
    border: 1px solid #ddd;
}

EDIT: As for you question about why I've used  tags instead of the class. It's a similar situation to why I've referred to your lists as ul and not .paginateList, the class literally provides no additional information because every single  tag has the class present. Changing it to use a instead of .paginateAnchor will make both your HTML and CSS more lightweight.

I also noticed just now that you're using inline styles applied with your JavaScript.

This would be an ideal situation to use classes, the elements that are inline-block can have no class (which will use div.paginateContainer > ul > li { display:inherit; }) and the hidden elements can have a class called .hidden. The reasoning for this is separation of concerns, your JavaScript should not need to do any styling, instead it should offload that detail to your CSS so everything is in one place.


Email subscription queries for a PHP website Is this code resistant against injection? I already used 'real-escape-string' but I don't know if this good enough. As you see I replaced my personal info. And the code does work. I'm building this site for a family member so it needs to be very protected against 'hackers'. Thank you in advance.

HTML:

<input type="text" name="email" id="email">

 Subscribe

 Unsubscribe

PHP:

$conn = new mysqli('#', '#', '#', '#');

if ($conn->connect_error) {
       die("Connection failed: " . $conn->connect_error);
}

$email = $_POST['email'];

$sql = sprintf("SELECT * FROM ... WHERE email = '%s'", mysql_real_escape_string($email));

$result = $conn->query($sql);

    if ($result->num_rows > 0) {

        die ("Already subscribed");

    } else {

        $sql2 = sprintf("INSERT INTO ... (email) VALUES ('%s')", mysql_real_escape_string($email));
        $result2 = $conn->query($sql2);
        echo "You subscribed!";

    }

If you are using the mysqli functions, then do not call mysql_real_escape_string(), which is part of the deprecated mysql suite of functions.  At the least, you should change it to call mysqli_real_escape_string() so that you are using the same suite of functions.

That said, using sprintf() to compose the query and escaping the string parameter yourself is not the best practice.  A more foolproof way would be to use prepared statements, such that the library takes care of any necessary escaping for you.

$select_query = $conn->prepare("SELECT * FROM … WHERE email = ?");
$select_query->bind_param('s', $email);
if (! $select_query->execute()) {
    die("Database error");
}
if ($select_query->num_rows > 0) {
    die("Already subscribed");
}

I wouldn't use a SELECT query at all to detect whether an e-mail address is already subscribed.  Rather, I would place a UNIQUE constraint on the email column, and catch any attempt to insert a duplicate.

Reasons for doing it that way are:

It's generally good practice to enforce such uniqueness constraints in the database schema.  It makes the schema more foolproof and self-documenting.  It also automatically creates an index on the email column, which could improve search performance.
If you enforce uniqueness in the PHP code rather than in the database, then you are vulnerable to a race condition.  If two requests come in at the same time, it's possible that one of them could manage to sneakily perform an insert in between the other request's select and insert.
Eliminating a SELECT query would make it more efficient.  You should be checking the result of the INSERT anyway — which you don't.  (Your code ignores $result2.)


Copy two properties from one array item to matching index in another array I have two IEnumerable objects called items and newItems.  I need to update the Did and KeyDid property of each element in items with the matching Did and KeyDid from newItems.  I can't just call items = newItems; since other things have a reference to items.  Here's my code:

T[] arrItems = items as T[] ?? items.ToArray();
if (arrItems.Length != newItems.Count())
    throw new Exception("Item counts do not match.");

//Copy Did and KeyDid from old items to newItems
int i = 0;
foreach (T newItem in newItems)
{
    T oldItem = arrItems[i++];
    oldItem.Did = newItem.Did;
    oldItem.KeyDid = newItem.KeyDid;
}

I'm interested in a more concise way to do this, perhaps using a single LINQ expression.  Any ideas how to improve this code in terms of readability, clearness, conciseness, or overall coolness?

if (arrItems.Length != newItems.Count())
        // --> BAD throw new Exception("Item counts do not match.");
        // Do not throw the Exception base here look for the real exception for example ArgumentException(if the arr come from arguments) or your custom exception:
        throw new ArgumentException();

      //Copy Did and KeyDid from old items to newItems
      for (var index = 0; index < newItems.Length; index++)
      {
        arrItems[index].Did = newItems[index].Did;
        arrItems[index].KeyDid = newItems[index].KeyDid;
      }

I do not think LINQ will give you a better solution than what you have above; you can use group by or select to create a new element in the linq query and than put it back to object! "For" in this case is very fast and it solve the problem.


Read strings from a standard input, and find the most duplicated word Write a program to read strings from standard input looking for duplicated words.
The program should find places in the input where one word is followed immediately by itself.
Keep track of the largest number of times a single repetition occurs and which word is repeated.
Print the maximum number of duplicates, or else print a message saying that no word was repeated.

For example, if the input is

how now now now brown cow cow

the output should indicate that the word now occurred three times.

#include <iostream>
#include <string>

int main()
{
    using std::cin; using std::cout; using std::endl;
    using std::string;

    // running median
    string most_duplicated_str;
    int most_duplicated_count = 0;

    // loop variables
    string prev_str;
    string new_str;
    int new_duplicate_count = 1;

    while(cin >> new_str)
    {
        if (new_str != prev_str)
        {
            // check and update median
            if (new_duplicate_count > most_duplicated_count)
            {
                most_duplicated_str = prev_str;
                most_duplicated_count = new_duplicate_count;
            }

            // update prev_str
            new_duplicate_count = 1;
            prev_str = new_str;
            continue;
        }

        ++new_duplicate_count;
        prev_str = new_str;
    }

    // check and update median for last word
    if (new_duplicate_count > most_duplicated_count)
    {
        most_duplicated_str = prev_str;
        most_duplicated_count = new_duplicate_count;
    }

    // print result
    if(most_duplicated_count == 0)
        cout << "# No Duplicated Words #\n";
    else
        cout << "Most duplicated word: " << most_duplicated_str << ", " << most_duplicated_count << " times.\n";
}

Nothing awful here.
If this was production code then putting things in named functions would be nice to help with self documenting code.

Don't particularly like this.
    using std::cin; using std::cout; using std::endl;
    using std::string;

But I am not going to complain about it very much.

Using snake case is unusual but not unheard of in C++.
    string most_duplicated_str;
    int most_duplicated_count = 0;

Like the long descriptive names and the grouping of you variables into logical combinations.

I would note that:
      cin >> new_str

Reads a white space separated word. For college and learning purposes this will be fine. But in real life you would have to consider punctuation and other non alphabetical content and what to do with it.

Not a very useful comment.
I would either make it more descriptive (more words) or remove it.
    // loop variables

and
            // update prev_str

These three lines can be simplified:
            new_duplicate_count = 1;
            prev_str = new_str;
            continue;

to:
            new_duplicate_count = 0;

It will have the same effect and the loop will feel more logical as the same action happens every loop. But that's just my opinion on readability. If you did not change it I would not oppose it being merged into mainline.

If any words are read: Can the value ever be zero?
    // print result
    if(most_duplicated_count == 0)
        cout << "# No Duplicated Words #\n";
    else

If you only have a duplicate count of 1 were there any duplicates?
What happens if there are ties in the number of duplicate counts?
These are good questions to ask your customer! :-)


Dynamic programming solution for cross river algorithm (part 2) I'm working on the cross river problem (previous post here). Any advice on performance improvement in terms of algorithm time complexity, code bugs or code style advice is appreciated.

More specifically, my idea is:

build a DP set to represent if we can reach a location by a certain speed;
when checking whether we are reach a certain location by a certain speed, we check if we can reach (1) location - speed with speed, (2) location - speed with speed + 1, (3) location - speed with speed - 1, if any (1), (2) or (3) is true, it means we can reach a certain location with such specific speed;
For the last location, (1) if it is not land, we will check if it could be reached by any speed, return True, (2) if it is not land, we will check if we can reach end position + 1 with any speed.

Problem

  Given a string representation of the rocks in the river e.g. "*****  *
  * * *  *  *", determine whether the river is crossable. Where,

* = landable
  = cannot land there

  Suppose Initial location: 0 and Initial speed: 0

in each step:
    1. choose a new speed from {speed, speed + 1, speed - 1} (but not negative)
    2. move speed spaces: loc += speed
    3. if I land on water, I fail
    4. if I land past end of river, I win
    5. otherwise, keep going

  For example, below river could be crossed by the first method. So, it
  should return True.

X~~~~~~~~~~~~~~~~~~~~~_______($)
*****  *   * * *  *  *
011 2  3   4   4  3  3   4 (crossed)
01 2 2 (failed)
01111 2 (failed)

def can_cross_river(river):
    # a set of tuple (position, speed), means we can reach
    # a specific position by a specific speed
    dp = set()
    dp.add((0,0))
    for i,v in enumerate(river):
        if i == 0 or v != '*':
            continue
        for speed in range(0,i+1):
            if (i-speed, speed) in dp or (i-speed, speed-1) in dp or (i-speed, speed+1) in dp:
                dp.add((i, speed))
                if i == len(river) - 1:
                    return True
    if river[-1] != '*':
        for speed in range(0,len(river)+1):
            if (len(river)-speed, speed) in dp or (len(river)-speed, speed-1) in dp or (len(river)-speed, speed+1) in dp:
                return True
    return False

if __name__ == "__main__":
    river = "*****  *   * * *  *  *" # should return True
    river = "* *" # should return False
    river = "******  " # should return True
    print can_cross_river(river)

You still have the remnant of the same bug.

Consider the river **.*..*...*....** The last slot exists but is not reachable. Yet you can jump from the penultimate slot to cross the river.

Or consider **.*..*...*....*. The slot just past the end is not reachable but something further to its right is.

This can be dealt with by modifying your code thus:

def can_cross_river(river):
    dp = set()
    dp.add((0,0))
    for i,v in enumerate(river):
        if v != '*':
            continue
        for speed in range(0,i+1):
            if (i-speed, speed) in dp or (i-speed, speed-1) in dp or (i-speed, speed+1) in dp:
                dp.add((i, speed))
                if i+speed+1 >= len(river):
                    return True
    return False


jQuery code to create web alert I wrote the following code using jQuery to create a WebAlert instead of using the alert() function from jQuery:

(function($){
    $.fn.WebAlert = function(msg) {
        // If WebAlert is Availble -> First Remove  this-> callback = Yes -> Now Create New WebAlert
        if ($('.WebAlert_Area').length) {
            $(this).Remove_WebAlert(msg , 'Yes')
            return false;
        //If WebAlert Not Availble
        } else {
            $(this).Create_WebAlert(msg)
            return false;
        }
    }

    //Function For Create WebAlert
    $.fn.Create_WebAlert = function(msg) {
        $('body').append('

');
        $('.WebAlert_Area .Text_Area').html(msg)
    };

    //Function For Remove WebAlert
    $.fn.Remove_WebAlert = function(msg , callback) {
        $('.WebAlert_Area').hide(1000 , function() {
            $(this).remove();
            if (callback == 'Yes') {
                $(this).Create_WebAlert(msg)
            }
        })
    };
})(jQuery);

$(document).ready(function() {
    $('body').WebAlert('Welcome To My Web')
}};

But I think that it's too complex and non-standard, is that true?

Consistency and correct syntax would be a start:  The last function call
is improperly closed, so technically that's broken.  Also my JavaScript
mode marks all missing semicolons, so I inserted them too.  The naming
convention is unusual in the sense that it's mixing camel case and
underscores, doesn't loook the greatest.  The comments mostly state what
the code already says - I'd leave them out unless they add significant
insights.

Now to the actual code.  WebAlert can be simplified since it always
returns false.

The name callback implies the parameter being a function, which it's
not, a better name could be inCallback or so?

The whole adding/removing nodes just for an alert seems messy.  I'd say
that you should always have the div for WebAlert_Area in the
website, then toggle its visibility with CSS unless there's a pressing
reason to do it this way.

In any case, removing the div just to recreate it immediately
doesn't strike me as a good way.  Can't it just stay there and be filled
with new content (.html(msg)) instead of
Remove_WebAlert(); Create_WebAlert?

Of course it is untested, but I think splitting it like below makes more
sense and is less convoluted:

(function($){
    $.fn.WebAlert = function(msg) {
        if (!$('.WebAlert_Area').length) {
            $(this).Create_WebAlert();
        }
        $(this).Set_WebAlert(msg);
        return false;
    };

    $.fn.Create_WebAlert = function(msg) {
        $('body').append('

');
    };

    $.fn.Set_WebAlert = function(msg) {
        $('.WebAlert_Area .Text_Area').html(msg);
    };

    $.fn.Remove_WebAlert = function() {
        $('.WebAlert_Area').hide(1000, function() {
            $(this).remove();
        });
    };
})(jQuery);

$(document).ready(function() {
    $('body').WebAlert('Welcome To My Web');
});


Oracle PL/SQL decryption function breaking index? I have a package called pkg_crypto.

My select looks like this:

SELECT "SEQ", pkg_crypto.decrypt("NAME", 'secret') name, pkg_crypto.decrypt("TEL", 'secret') tel
FROM MYTABLE
ORDER BY "SEQ" DESC

This takes about 3 seconds for a few thousand records. (Average of 10 runs is 3.032s.)

Now, if I change it to either of the following, the query completes in about 0.2 seconds (average of 10 runs is 0.203s):

SELECT "SEQ", "NAME", "TEL"
FROM MYTABLE
ORDER BY "SEQ" DESC

SELECT "SEQ", pkg_crypto.decrypt("NAME", 'secret') name, pkg_crypto.decrypt("TEL", 'secret') tel
FROM MYTABLE

What seems to be happening is that if I use the decrypt and order by on the same command, it will re-sort the records and ignore the index that exists on "SEQ".

I suspect this may be a problem with the implementation of the pkg_crypto package that I am using.  Its code is as follows:

CREATE OR REPLACE PACKAGE PKG_CRYPTO AS

    FUNCTION encrypt (
        input_string        IN  VARCHAR2 ,
        key_data IN VARCHAR2 := 'DEFDB'
    ) RETURN RAW;

    FUNCTION decrypt (
        input_string        IN  VARCHAR2 ,
        key_data IN VARCHAR2 := 'DEFDB'
    ) RETURN VARCHAR2;

END PKG_CRYPTO;PACKAGE BODY pkg_crypto
IS
-- .. .... error code . message . .. .. .. ...
    SQLERRMSG   VARCHAR2(255);
    SQLERRCDE   NUMBER;

-- ... .. .. key_data . .... .. .. default . 12345678 . ....
    FUNCTION encrypt (input_string IN VARCHAR2 , key_data IN VARCHAR2 := 'DEFDB')
     RETURN RAW
    IS

        key_data_raw        RAW(2000);
        converted_raw       RAW(2000);
        encrypted_raw       RAW(2000);

    BEGIN

-- ... data . .... .. RAW . .....
        converted_raw := UTL_I18N.STRING_TO_RAW(input_string, 'AL32UTF8');
        key_data_raw     := UTL_I18N.STRING_TO_RAW(key_data, 'AL32UTF8');

-- DBMS_CRYPTO.ENCRYPT . ... .. encrypted_raw . ...
        encrypted_raw :=
             DBMS_CRYPTO.ENCRYPT(
                 src => converted_raw ,
-- typ ... .... ... ..... ... . ...
--., key value bype . . ... .... ...
                 typ => DBMS_CRYPTO.DES_CBC_PKCS5 ,
                 key => key_data_raw ,
                 iv =>  NULL);

        RETURN encrypted_raw;
    END encrypt;

    FUNCTION decrypt (input_string IN VARCHAR2 , key_data IN VARCHAR2 := 'DEFDB')
     RETURN VARCHAR2
    IS
        converted_string    VARCHAR2(2000);
        key_data_raw        RAW(2000);
        decrypted_raw    VARCHAR2(2000);

    BEGIN

        key_data_raw     := UTL_I18N.STRING_TO_RAW(key_data, 'AL32UTF8');

        decrypted_raw :=
            DBMS_CRYPTO.DECRYPT(
                 src => input_string ,
                 typ => DBMS_CRYPTO.DES_CBC_PKCS5 ,
                 key => key_data_raw ,
                 iv =>  NULL);

-- DBMS_CRYPTO.DECRYPT .. .. .. .... raw data . varchar2 . .... .!
        converted_string := UTL_I18N.RAW_TO_CHAR(decrypted_raw, 'AL32UTF8');

        RETURN converted_string;
    END decrypt ;
END pkg_crypto;

I've also attempted changing the query around a bit, but it hasn't been fruitful: the following take the same 3 seconds:

SELECT "SEQ", pkg_crypto.decrypt("NAME", 'secret') name, pkg_crypto.decrypt("TEL", 'secret') tel
FROM
(
    SELECT "SEQ", "NAME", "TEL" FROM MYTABLE ORDER BY "SEQ" DESC
)

and

SELECT "SEQ", pkg_crypto.decrypt("NAME", 'secret') name, pkg_crypto.decrypt("TEL", 'secret') tel
FROM
(
    SELECT "SEQ", "NAME", "TEL" FROM MYTABLE
)
ORDER BY "SEQ" DESC

I've found that the reason that decryption xor sorting is fast, but decryption and sorting is slow:

Decryption will only grab the first 100 or so records, decrypt them, and display them.  Fast.
Sorting will grab the first 100 or so records (utilizing the pk index) and display them.  Fast.
Decryption and sorting will force a table scan, resulting in decrypting several thousand records, and then sorting them all.

In other words, the "fast" queries are only fast because they're doing very little work.


Pytest fixture for testing a vertex-parsing function I have just started using pytest and I am still getting used to how they do things. It seems like fixtures are at the core of the library, and that they can be used for making small pieces of dummy data that will get reused. I see that there are other methods for handling large dummy data. I have the following test code which tests a module I wrote called generate_kml.

import pytest
import generate_kml as gk
import pandas

@pytest.fixture
def line_record():
    return pandas.Series({gk.DB_VERTICES: "LINESTRING(1.1 1.1,2.2 2.2)"})

def test_convert_wkt_to_coords(line_record):
    expected = pandas.Series({gk.DB_VERTICES: [("1.1", "1.1"), ("2.2", "2.2")]})
    assert gk.convert_wkt_vertices_to_coords(line_record).equals(expected)

I am wondering if this is the way fixtures are meant to be used; to set up small reused data. (I plan to use the line_record multiple times in the test file). Additionally, I am wondering about the readability or redundancy of assigning the expected value to expected. If I directly compared the two Series, the line would exceed PEP8's recommended line length, so I would break it into two lines anyway. If it adds readability here, then would it be good practice to always assign the expected value to a variable called expected (assuming you are comparing values that are expected to be equal)?
Here is the function being tested from generate_kml:

def convert_wkt_vertices_to_coords(vertices_as_wkt):
    def parse_coords(wkt):
        wkt = wkt[wkt.find("(") + 1:wkt.find(")")]
        coords = wkt.split(",")
        coords = [tuple(x.split(" ")) for x in coords]
        return coords
    return vertices_as_wkt.apply(parse_coords)

One more thing here; in convert_wkt_vertices_to_coords I have a nested function. I don't plan on reusing it, but in the past I haven't had a need for nested functions, so it feels a bit off to me. Should I leave it as a nested function or break it out as its own function in the module?

This is one of the way you can use fixtures. Fixtures are a form of dependency injection. For example, I use pytest to execute integration testing for an embedded target over ssh. So I want to reuse the ssh connection between tests.

The pytest fixture documentation has a similar example with SMTP:
https://docs.pytest.org/en/latest/fixture.html

Note that if you want to reuse the same exact object between tests, you need to specify the scope @pytest.fixture(scope='module')

Another reason to use fixtures is that they are the basis for writing plugins. Here's the pytest documentation on turning fixtures into plugins:
https://docs.pytest.org/en/latest/writing_plugins.html

If you want to learn more about fixtures I would recommend checking out the source for some of the 3rd party plugins they have one their website:
https://pytest.readthedocs.io/en/2.7.3/plugins_index/index.html

So that should give you an idea that fixtures are really flexible and people have implemented a lot of really cool ones.

Pytest fixtures let you do things like examine the values of existing command line options and add new command line options.
https://docs.pytest.org/en/latest/example/simple.html#pass-different-values-to-a-test-function-depending-on-command-line-options


Dynamic SQL query for count data I'm working on an application that counts the number of employees in each department.

Here's my code:

SELECT
    count(case when (JS_TITLE = 'Accounting')then 1 end)as Accounting,
    count(case when (JS_TITLE = 'Management')then 1 end)as Management,
    count(case when(JS_TITLE = 'Marketing')then 1 end )as Marketing,
    count(case when(JS_TITLE = 'HR')then 1 end)as HR
FROM EMP
LEFT JOIN JOB_STATUS ON EMP.JS_REF = JOB_STATUS.JS_REF

but the thing is, the user may add a new department.

Is there a better way to make this static code more dynamic?

You can use a GROUP BY clause to group by JS_TITLE, then you must transpose the record into a column matrix:

SELECT
    JS_TITLE,
    count(EMP.JS_REF)
FROM EMP
LEFT JOIN JOB_STATUS ON EMP.JS_REF = JOB_STATUS.JS_REF
GROUP BY JS_TITLE


Calculate GS1 / SSCC / UPC check digit I try to write a function which calculates the check digit for shipping label based on the algorithm provided by GS1
http://www.gs1.org/how-calculate-check-digit-manually

Version 1: my first try

public static int GetGTINCheckDigitV1(string code)
{
    int sum = 0;
    for (int i = 0; i < code.Length; i++)
    {
        int n = int.Parse(code.Substring(code.Length - 1 - i, 1));
        sum += i % 2 == 0 ? n * 3 : n;
    }

    return sum % 10 == 0 ? 0 : 10 - sum % 10;
}

Version 2: using Reverse()

public static int GetGTINCheckDigitV2(string code)
{
    int sum = 0;
    var list = code.Reverse().ToArray();

    for (int i = 0; i < list.Length; i++)
    {
        int n = (int)Char.GetNumericValue(list[i]);
        sum += i % 2 == 0 ? n * 3 : n;
    }

    return sum % 10 == 0 ? 0 : 10 - sum % 10;
}

Version 3: using lambda

public static int GetGTINCheckDigitV3(string code)
{
    var sum = code.Reverse().Select((c, i) => (int)char.GetNumericValue(c) * (i % 2 == 0 ? 3 : 1)).Sum();
    return (10 - sum % 10) % 10;
}

Is there anything I can improve this function or any concern?

It looks good!  I only have some minor suggestions.  You might want to not have the LINQ expression all on one line.  Right now I have to scroll to see it all.

An alternative is to use Enumerable.Range() to create a range of indexes:

public static int GetGTINCheckDigitUsingRange(string code)
{
    var reversed = code.Reverse().ToArray();
    var sum = Enumerable
        .Range(0, reversed.Count())
        .Sum(i => (int)char.GetNumericValue(reversed[i]) * (i % 2 == 0 ? 3 : 1));
    return (10 - sum % 10) % 10;
}

Another alternative is to use query syntax:

public static int GetGTINCheckDigitUsingQuery(string code)
{
    var reversed = code.Reverse().ToArray();
    var sum =
       (from i in Enumerable.Range(0, reversed.Count())
        let digit = (int)char.GetNumericValue(reversed[i])
        select digit * (i % 2 == 0 ? 3 : 1)).Sum();
    return (10 - sum % 10) % 10;
}

I think your version three is still better than my alternatives, but you might prefer one of them.  Enumerable.Range() is nice for replacing for loops, and query syntax is nice for declaring intermediate values (like digit) and replacing nested loops.  But the Select() method that takes a lambda that includes the index as a parameter is very nice for this situation.  With enough practice you can get to the point where your code looks like version three right away.  I rarely write loops anymore.  I actually find them tougher to write because they make you focus more on "how" than "what".


Comparing if-else and case condition I have a script used for installing few rpms depending on operating system type. Here in this example, the script will install packages on two system "centos" and "suse". The system names are listed in nodes.txt and read as an input while executing the script.

For example:

~]# cat systems.txt

centos
suse

~]# ./myscript.sh systems.txt

There are two scripts written which does the same purpose. One is written using "if-else" and other using "case".

When I use "if-else" condition script, the execution (i.e installation of rpms) completes much faster than using the "case" condition script. Both scripts does the same job, except that the control structure is different. I will have many hundreds of installation criteria listed in the script, and will add/remove new system types within systems.txt file.

Hence I am not sure which condition is better to choose, and considering my use case. Can someone help me know why does case script taking more time to execute when compared if-else? Any help/recommendations?

Below the case script which took 10 secs to complete.

#! /bin/bash

while read -d $'\n' -r node
do

# Each node has a unique id assigned and used along with install script
id=$(./name.sh -getid)

case $node in
   centos)
echo $(./install.sh --resourceId=$id --name=httpd)
echo $(./install.sh --resourceId=$id --name=gcc)
      ;;

    suse)
echo $(./install.sh --resourceId=$id --name=compact)
echo $(./install.sh --resourceId=$id --name=glib)
      ;;

   *)
        echo "#No resources found for the node"
    ;;
esac
done < "$1"

Below the if-else script which took 5 secs to complete:

#!/bin/bash
while read -d $'\n' -r node
do

# Each node has a unique id assigned and used along with install script
id=$(./name.sh -getid)

if [ $node == centos ]
then

echo $(./install.sh --resourceId=$id --name=httpd)
echo $(./install.sh --resourceId=$id --name=gcc)

elif [ $node == suse ]
then

echo $(./install.sh --resourceId=$id --name=compact)
echo $(./install.sh --resourceId=$id --name=glib)

else

echo "#No resources found for the node"

fi
done < "$1"

This problem has me perplexed. Frankly, I don't believe you ;-) !!!

There is no logical reason for this code to be so different from each other. There must be something else going on.... For a start, the bash process itself is probably taking almost no time at all, and all the time will be spent running the 'install' sub-processes. I imagine that you are running the install processes twice as many times for some reason.... or that, subsequent installs are faster because you don't need to access the disk/net, or something.

Regardless, I don't believe the difference in performance is because of the case vs. if

As a side note, do you need to execute the installs in a sub-shell and capture the results like you do and then echo them? The install lines should all look similar to:

# Relace this: echo $(./install.sh --resourceId=$id --name=httpd)
# with:
./install.sh --resourceId=$id --name=httpd


Finding the occurrences of all words in movie scripts I was wondering if someone could tell me things I could improve in this code. This is one of my first Python projects. This program gets the script of a movie (in this case Interstellar) and then finds the occurrences of all words and prints out the 20 most common words in the script.

from bs4 import BeautifulSoup
import requests
from collections import Counter
import operator
import re

class MovieScript(object):
    def __init__(self, movie):
        self.movie = movie
        self.url = "http://www.springfieldspringfield.co.uk/movie_script.php?movie={0}".format(movie.replace(" ", "-"))

    def get_movie(self):
        return self.movie

    def get_text(self):
        r = requests.get(self.url)
        soup = BeautifulSoup(r.text)
        return soup

    def parse_text(self):
        text = self.get_text().findAll('div', {"class":"scrolling-script-container"})[0].text.lower()
        text = re.sub(r'\W+', ' ', text) #remove puncation from the text
        return text.split()

    def find_all_occurences(self):
        return Counter(self.parse_text())

    def get_all_occurences(self):
        sorted_occurences = sorted(self.find_all_occurences().items(), key=operator.itemgetter(1))
        return sorted_occurences

def Main():
    db = MovieScript("interstellar")
    print "The Movie: " + db.get_movie()
    all_occurences = db.get_all_occurences()
    for i in range(-1, -21, -1):
        print str(-i) + ". " + str(all_occurences[i][0]) + " -> " + str(all_occurences[i][1])

if __name__ == "__main__":
    Main()

A few things I can think of are:

parse_text can be marked as a private method. def _parse_text(self):. I understand this won't actually be hidden, but its a convention followed in python to treat methods starting with an underscore as private.
In MovieScript __init__ method maybe have the url as a kwarg (defaulting to http://www.springfieldspringfield.co.uk, but can be overwritten and point to another site in case you ever have to scrape another site)
In def get_text: You can cache the result from the request.get you're running and check the cache before making the request
Finally consider moving the logic of calculating the top keywords inside the MovieScript class itself (call it something like def get_common_words(self, count=20):) and allow the count to change in case you ever want more than the default.

I'm sure there are more and people will have there own preferences, but this is just what I could suggest in a quick glimpse :)


Two duration widgets for a break timer, with ± buttons I'm sure I can optimise this code, but I can't find how in order to be generic. I'm not sure that .parent and .child combination is good in the long term here.

$(document).ready(function() {
  var lengthInit = $("#lengthSession").text();
  $("#timer").text(lengthInit);
  $("#dimBreak").click(function() {
    dimLength();
  });
  $("#augBreak").click(function() {
    augLength();
  });
  $("#dimSession").click(function() {
    dimLengthSession();
  });
  $("#augSession").click(function() {
    augLengthSession();
  });
});

function dimLength() {
  var length = parseInt($("#lengthBreak").text());
  length = length - 1;
  $("#lengthBreak").text(length);
}

function augLength() {
  var length = parseInt($("#lengthBreak").text());
  length += 1;
  $("#lengthBreak").text(length);
}

function dimLengthSession() {
  var length = parseInt($("#lengthSession").text());
  length = length - 1;
  $("#lengthSession").text(length);
}

function augLengthSession() {
  var length = parseInt($("#lengthSession").text());
  length += 1;
  $("#lengthSession").text(length);
}

Break length

      <button class="boutons" id="dimBreak">-</button>
      5
      <button class="boutons" id="augBreak">+</button>

Session length

      <button class="boutons" id="dimSession">-</button>
      25
      <button class="boutons" id="augSession">+</button>

Extract common logic to helper method

These functions have duplicate logic, but different data:

function augLength() {
  var length = parseInt($("#lengthBreak").text());
  length += 1;
  $("#lengthBreak").text(length);
}

function augLengthSession() {
  var length = parseInt($("#lengthSession").text());
  length += 1;
  $("#lengthSession").text(length);
}

A simple improvement is to extract the common logic to a helper method:

function inc(selector) {
  var length = parseInt($(selector).text());
  length += 1;
  $(selector).text(length);
}

function augLength() {
  inc("#lengthBreak");
}

function augLengthSession() {
  inc("#lengthSession");
}

Parsing an int

Instead of:

  var length = parseInt($(selector).text());

Always when using parseInt, you should specify a radix parameter to indicate the base, for example for base-10:

  var length = parseInt($(selector).text(), 10);

And, what if the text cannot be parsed? It's good to make it default to something, say, 0, which you can achieve simply with:

  var length = parseInt($(selector).text(), 10) || 0;

DOM lookups are expensive

Instead of:

function inc(selector) {
  var length = parseInt($(selector).text(), 10) || 0;
  length += 1;
  $(selector).text(length);
}

It's better to do the DOM lookup once and reuse:

function inc(selector) {
  var dom = $(selector);
  var length = parseInt(dom.text(), 10) || 0;
  length += 1;
  dom.text(length);
}

Extracting more common logic

The inc helper can be extended to take care of the decrements too, by adding a second parameter:

function inc(selector, increment) {
  var dom = $(selector);
  var length = parseInt(dom.text(), 10) || 0;
  length += increment;
  dom.text(length);
}

function augLength() {
  inc("#lengthBreak", 1);
}

function augLengthSession() {
  inc("#lengthSession", 1);
}

function dimLength() {
  inc("#lengthBreak", -1);
}

function dimLengthSession() {
  inc("#lengthSession", -1);
}

Functions as parameters

You can do without the augLength, dimLength, augLengthSession, dimLengthSession functions, if you like, by using another helper that returns a function:

function makeIncrementer(selector, increment) {
  return function () { inc(selector, increment); };
}

Which then you can call directly from the ready block:

$(document).ready(function() {
  var lengthInit = $("#lengthSession").text();
  $("#timer").text(lengthInit);
  $("#dimBreak").click(makeIncrementer('#lengthBreak', -1));
  $("#augBreak").click(makeIncrementer('#lengthBreak', 1));
  $("#dimSession").click(makeIncrementer('#lengthSession', -1));
  $("#augSession").click(makeIncrementer('#lengthSession', 1));
});


Leetcode longest substring with no duplicates class Solution(object):
    def lengthOfLongestSubstring(self, s):
        """
        :type s: str
        :rtype: int
        """
        longest = ''
        max_len = 0
        for key,value in enumerate(s):
            if value in longest:
                longest = longest[longest.index(value)+1:] + value
            else:
                longest+=value
            max_len = max(max_len,len(longest))
        return max_len

Here is my solution for leetcode's "longest-substring-without-repeating-characters question". I don't quite understand why my time complexity is so efficient (99% out of all python submissions). Since strings are immutable in python, assuming a strength of length n, won't my algorithm be $$ 1+2+3+...n-1+n = O(n^2)$$ time? But the optimal solution is on \$O(n)\$ yet the runtime from the testcases says my solution is just as good as the optimal one provided?Am I missing something. Also I am having a hard time proving the space complexity for my solution as well.

The linearity of your approach is due to the fact that your longest could never be longer than the size of the alphabet (otherwise it'd have a duplicate for sure). It means that your code runs at \$O(N * A)\$, where \$N\$ is a length of the string, and \$A\$ is a size of alphabet. Since the latter is a constant, you may safely take it out of the big-O, yielding \$O(N)\$.

The asymptotic constant is still a little bit too large (I don't know what alphabet is used in the test cases, but it is safe to assume at least 26), so there is a room for improvement. You correctly noticed that the immutability of the strings may hurt performance; try to get rid of them. A most obvious approach is to preallocate a list (of the alphabet size), and use it as a circular buffer.

In fact, even that is suboptimal. Try to get rid of the explicit buffer at all (hint: two indices into the original string).

It also may be beneficial to keep a dictionary indexed by letters currently present in the buffer with the values being their corresponding indices. This way you can do a constant time lookup rather than linear search in the buffer. Keep in mind however that such a dictionary may itself be costly; I expect it to improve performance if the alphabet is quite large. Try and profile it.

As for the code, it is simple and clean. Not much to say.


Shopping cart simulator I have tried to simulate a cart (only partially completed). I just wanted to see how the code upto this point can be enhanced. I am trying to learn about the usage of directives.

Here I have used 2 directives:

msgpallette- to show the messages when something is added to the cart
item-container - to show each item in menu

A demo of the same is provided at plunkr

Do tell me whether I have used directives properly. If not, do suggest code changes with explanations.

HTML

    <msgpallete msg="message" ng-show='show'>{{message}}
    </msgpallete>

    <item-container
        startcounter = 1
        resetter     = 'reset'
        name         = {{item.name}} >
        {{item.name}}
    </item-container>

    <order-container >
        {{order.name}}
    </order-container>

JS

angular.module('myApp',[])
.controller('MainCtrl', function($scope, $timeout) {
  $scope.items = [
  {'name':'kabab'},
  {'name':'noodles'},
  {'name':'chicken'},
  {'name':'egg'}
  ]
  $scope.resettrigger = function () {
  $scope.reset = true;
  $timeout(function() {
  $scope.reset = false;
},100)
}
  $scope.show=0;
  $scope.addMsg = function (qty,item) {
  $timeout.cancel($scope.promise);
  $scope.show = true;
  msg = "Added "+qty+" "+item+" to the cart";
  $scope.message = msg;
  $scope.promise = $timeout(function() {
  $scope.show = false;
},3000)

}})
.directive('msgpallete',function(){
 return{

restrict:"E",
transclude:true,
scope:{},
template:"

"
}
})

.directive('itemContainer',function(){
return {
controller: function() {return {}},
    restrict:'E',
    scope:{

  resetter:"="
    },
transclude:true,
    link:function(scope,elem,attr){

        scope.qty = attr.startcounter
        scope.add = function(){

            scope.qty++;
        }
        scope.remove = function(){
            scope.qty--;
        }
        scope.reset = function(){
    console.log("attr.item:"+attr.name);
     scope.$parent.addOrder(attr.name)
    scope.$parent.addMsg(scope.qty,attr.name)
    console.log("value when submitted:" + scope.qty + "name:"+ attr.name);
            scope.qty = attr.startcounter;
            scope.$parent.resettrigger();
        }

        scope.$watch(function(attr){
            return attr.resetter
        },
        function(newValue){
            if(newValue === true){
                scope.qty = attr.startcounter;
            }
        });

    },
    template:"<button ng-click='reset();'>Add to cart</button>&nbsp&nbsp"+
         "<button ng-click='remove();' >-</button>&nbsp"+
                 "{{qty}}&nbsp" +
                 "<button ng-click='add();'>+</button>&nbsp&nbsp"+
         " "

    }
});

Don't have time for a full review but hopefully this will give you some food for thought.

Why add an extra variable show? Angular is more than capable of binding to an expression:

ng-show='message != ""'

Then you have one less thing to juggle in your controller:

$scope.addMsg = function (qty, item) {
    $scope.message = "Added " + qty + " " + item + " to the cart";
    // console.log($scope.message); // if you want to debug
    $timeout(function() {
      $scope.message = "";
    },3000);
}

Some notes about code clarity:

Don't forget your semicolons
Add some spacing between operators e.g. a + b + c + d is easier to read than a+b+c+d
addCartMessage is probably more descriptive than addMsg

As for your question about directives:

msgpallete should be killed with fire
itemContainer seems like a good candidate but execution isn't great (will come back to this if I get time)

By removing the msgpallete directive you can change your view to:

{{message}}

(Your controller will be unchanged).


Reading fixed-width data fields in .NET I'm working on a communications layer for a system that reads data from a TCPIP client that is formatted as fixed-width ASCII (yeah, old school). I was quite surprised that there seemed to be no built in way to do this, and ended up using the following simple StreamReader subclass:

/// <summary>
/// A Stream reader that reads values as fixed width fields from a stream
/// </summary>
class FixedWidthFieldStreamReader : StreamReader
{
    #region Private/Protected fields
    private char[] buffer; // Local buffer used to copy data before conversion
    #endregion

    #region Methods
    /// <summary>
    /// Instantiates a new FixedWidthFieldStreamReader for a stream
    /// </summary>
    /// <param name="stream">Stream to read from</param>
    /// <param name="initialBufferSize">Initial size of the buffer used to copy data before formatting</param>
    /// <param name="encoding">Encoding to use when reading from the stream</param>
    public FixedWidthFieldStreamReader(Stream stream, int initialBufferSize, Encoding encoding)
        : base(stream, encoding)
    {
        buffer = new char[initialBufferSize];
    }

    /// <summary>
    /// Checks if the buffer exists and is large enough,
    /// and allocates or grows it if necessary.
    /// </summary>
    /// <param name="length">The required buffer length</param>
    private void EnsureBufferLength(int length)
    {
        if (null == buffer ||
            buffer.Length < length)
        {
            buffer = new char[length];
        }
    }

    /// <summary>
    /// Reads a number of bytes into the buffer
    /// </summary>
    /// <param name="length">The number of bytes to read</param>
    /// <returns>True if the required number of bytes was read, false otherwise</returns>
    private bool ReadToBuffer(int length)
    {
        EnsureBufferLength(length);

        // Read from the stream
        int read = Read(buffer, 0, length);
        return read == length;
    }

    /// <summary>
    /// Reads a specified number of bytes from the stream and
    /// converts and returns the read value.
    /// </summary>
    /// <typeparam name="T">Type of the object to read and return</typeparam>
    /// <param name="length">Number of bytes in the field to read from the stream.</param>
    /// <returns>The read object if successful, or the default value for the type otherwise.</returns>
    public T Read<T>(int length) where T : IConvertible
    {
        if (ReadToBuffer(length))
        {
            return (T)Convert.ChangeType(new string(buffer, 0, length), typeof(T));
        }

        return default(T);
    }

    /// <summary>
    /// Skips a specified number of bytes in the stream
    /// </summary>
    /// <param name="length">The number of bytes to skip</param>
    public void Skip(int length)
    {
        // Ideally we should be able to just seek on the current stream,
        //  but that seems to seek to an incorrect location?
        //this.BaseStream.Seek(length, SeekOrigin.Current);
        ReadToBuffer(length);
    }
    #endregion
}

This would be used something like this:

using (MemoryStream stream = new MemoryStream(buffer))
{
    stream.Seek((int)FieldOffsets.DATA, SeekOrigin.Begin);
    using (FixedWidthFieldStreamReader reader = new FixedWidthFieldStreamReader(stream, 15, Encoding.ASCII))
    {
        intVal = reader.Read<int>(3);
        stringVal = reader.Read<string>(15);
        floatVal= reader.Read<float>(5);
    }
}

I have two questions based on this:

Am I just missing some completely obvious existing utility to do this? It really does seem like a common problem that would have been solved by the framework team ages ago.
Aside from the obvious optimization of having some type specific versions of Read that don't do the conversion, are there any suggestions to improve this approach?

I don't know if something does that already, but you could mimic StructLayout if you need to read multiple types of 'objects' or need reusability.

Basically just set starting point/offset and length for each property and read them.

public class Item
{
     // 3 chars starting from 0
    [Layout(0, 3)]
    public int Number { get; set; }

     // 15 chars starting from 3
    [Layout(3, 15)]
    public string Text { get; set; }

    // 5 chars starting from 18
    [Layout(18, 5)]
    public float Number2 { get; set; }
}

reader.Read<Item>()


Function to represent an integer in a base This function represents an integral type and represents it in a base.

#include <type_traits>
#include <limits>
#include <array>
#include <iterator>

namespace detail {

template <class Integral>
char n_to_digit(Integral n) {
    static const char * digits = "0123456789abcdefghijklmnopqrstuvwxyz";
    return digits[n];
}

/// This implementation is called if Integral is unsigned
template <unsigned Base, class Integral, class OutputIterator>
std::size_t integral_to_base_impl(Integral n, OutputIterator dst_it, std::true_type) {
    /// Length of str is the minimum number of bits to store Integral which is
    /// the number of chars required to store the maximum value of in base 2
    /// representation
    std::array<char, (sizeof(Integral) << 3)> str;
    auto it = str.begin();

    do {
        *it++ = n_to_digit(n % Base);
        n /= Base;
    } while (n != 0);

    while (it != str.begin()) {
        *dst_it++ = *(--it);
    }

    return std::distance(str.begin(), it);
}

/// This implementation is called if Integral is signed
template <unsigned Base, class Integral, class OutputIterator>
std::size_t integral_to_base_impl(Integral n, OutputIterator dst_it, std::false_type) {
    std::size_t length = 0;

    if (n < 0) {
        *dst_it++ = '-';
        ++length;
    }

    length += integral_to_base_impl<Base>(std::abs(n), dst_it, std::true_type());
    return length;
}

}

/// Represents an integral in a certain base
///
/// @tparam Base The base that the integral will be represented as
///
/// @param n The number that will be represented
/// @param dst_it A char OutputIterator that the representation will be written to
///
/// @return The length of the representation
template <unsigned Base, class Integral, class OutputIterator>
std::size_t integral_to_base(Integral n, OutputIterator dst_it) {
    static_assert(2 <= Base && Base <= 36, "Base must be >= 2 and <= 36");
    static_assert(std::is_integral<Integral>::value, "n must be an integral type");

    return detail::integral_to_base_impl<Base>(n, dst_it,
        std::integral_constant<bool, std::is_unsigned<Integral>::value>());
}

My implementation is like the C's non standard function itoa however it takes the base is taken as a template parameter which allow the compiler to make optimizations with operations involving the base such as modulo and divide however it must be known at compile time. This allows integrals to represented as binary and hexadecimal with the Base template parameter being 2 and 16 respectively.

The function integral_to_base has implementation specializiations for unsigned and signed integral types and has static assertions to check that the base is valid at compile time for safety.

Example usage:

int main() {
    std::string test;

    integral_to_base<10>(123343, std::back_inserter(test));
    std::cout << test << std::endl; //prints 123343
}

n_to_digit doesn't really need to be a template, since its argument n can only ever be in the range 0 to 35. I'd just make it

char n_to_digit(int n) {
    static const char digits[] = "0123456789abcdefghijklmnopqrstuvwxyz";
    return digits[n];
}

Notice also the replacement of *digits with digits[]; the optimizer will likely generate the same code either way, but without the optimizer's help, making digits the name of the array itself instead of the name of a pointer to the array will remove one level of pointer dereferencing.

YMMV, but I would find

std::array<char, (sizeof(Integral) << 3)> str;

easier to read if it were just

char str[sizeof(Integral) * 8];

Of course then you'd have to use std::begin(str) instead of str.begin(). But you could make up for it by using it - str instead of std::distance(str.begin(), it).

Actually, hang on —

while (it != str.begin()) {
    *dst_it++ = *(--it);
}

return std::distance(str.begin(), it);

The loop never exits unless it == str.begin(), which means std::distance(str.begin(), it) == 0 by definition! This is certainly a bug.

Since you're trying to make this fully generic: note that *dst_it++ is going to be inefficient for a lot of iterator types. If you were writing this for a popular library, you'd write

*dst_it = '-';
++dst_it;

If you consistently avoid postfix operators, your users might even thank you, because your code will work even with their incompletely belled-and-whistled iterator types.

In the signed version, your use of std::abs(n) is a red flag. abs is never appropriate in generic code. For one thing, if Integral n is some clever fixed-point type, std::abs(n) won't compile; what you meant is more like

using std::abs;
... abs(n) ...

to pick up the correct abs through ADL. (But this opens the whole customization-point can of worms, which is still a mess even in C++17.)

No, the real reason abs is a red flag is because std::abs(-1 - 2147483647) invokes undefined behavior (source). You should never call abs on untrusted input.

The usual hack in this domain is to say

if (n < 0) {
    *dst_it = '-'; ++dst_it;
} else {
    n = -n;
}

and then print the negative of n.

std::integral_constant<bool, std::is_unsigned<Integral>::value>()

could conceivably be written

typename std::is_unsigned<Integral>::type{}

but honestly, only the most evil and sadistic library implementation in the universe would make is_unsigned not derived from boolean_constant already. So you can just write

std::is_unsigned<Integral>{}

and be assured that your tag-dispatch code will work at least as well as your neighbor's. :)


Temperature unit converter in Python I am starting to learn python and this has been my first project but I am not entirely happy, I think it could be done better. Could you help me? The strings are in Spanish but I think it does not harm understanding.
    elegir_magnitud = int(input(
"""1 = Temperatura
2 = Longitud
3 = Masa
4 = Volumen
5 = Tiempo
6 = Divisa
Entra número deseado: """))
if elegir_magnitud == 1 :
    unidad_temperatura = int(input(
        """

------------------Elige la unidad de temperatura a ser convertida:---------------
1 = Celsius
2 = Farhengei o como se escriba
3 = Kelvin
Entra número deseado: """))
    cantidad_temperatura = float(input(
        """

Entra la cántidad de esa unidad deseada: """))
#ELEGIR UNIDAD DE TEMPERATURA A CONVERTIR
    unidad_a_convertir = int(input(
        """

--------------------Elige la unidad de temperatura a convertir:-----------------
1 = Celsius
2 = Farhengei o como se escriba
3 = Kelvin
Entra número deseado: """))
    if unidad_temperatura == unidad_a_convertir:
        print("Por qué quieres convertir a la misma unidad?")
    if unidad_temperatura == 1 and unidad_a_convertir == 3:
        print("""El resultado de la conversion es: \n""",float(cantidad_temperatura - 273),"ºK")
    if unidad_temperatura == 1 and unidad_a_convertir == 2:
        print("""El resultado de la conversion es: \n""",float(cantidad_temperatura * 9%5 +32),"ºF")
    if unidad_temperatura == 3 and unidad_a_convertir == 1:
        print("""El resultado de la conversion es: \n""",float(cantidad_temperatura + 273),"ºC")
    if unidad_temperatura == 3 and unidad_a_convertir == 2:
        print("""El resultado de la conversion es: \n""",float((cantidad_temperatura -273) * 9 % 5 + 32),"ºF")
    if unidad_temperatura == 2 and unidad_a_convertir == 3:
        print("""El resultado de la conversion es: \n""",float((cantidad_temperatura - 32) % 9 * 5 - 273),"ºK")
    if unidad_temperatura == 2 and unidad_a_convertir == 1:
        print("""El resultado de la conversion es: \n""",float(cantidad_temperatura % 9*5 -32),"ºC")

Localisation

The strings are in Spanish

Great! However, that's not the only thing in Spanish: your variables (elegir_magnitud, unidad_temperatura, etc.) are as well.
This is non-ideal for at least a couple of reasons:

Python, being a language whose syntax is written in English, conflicts with your code that is then a mixture of English and Spanish. This is inconsistent.
For better or worse, the de-facto language of software development is English. Particularly for international collaboration or online open-source development, it is important to use a language that is accessible to more people.

It's a wonderful thing to apply internationalization (i18n) to your strings, but it should be to your strings only, and not the code.
Factor out common code
Consider refactoring your code to:

Have only one method that asks for units, called twice (once for source unit and once for destination unit)
Make a tuple of coefficient-offset pairs. These represent the linear function from the given unit to Kelvin and back. This will only need two unit conversion formulae, rather than (worst-case) 2^n with the current strategy.

Bug?
Are you sure that this works?
cantidad_temperatura % 9*5 -32

I doubt that modulus is the right thing to do here. You should be using division /.
Example code
Notes:

This does a rough i18n, with some strings from Google Translate (so my apologies for the likely poor Spanish).
Only English and Spanish are implemented but any other language can be added.
This assumes that your operating system is set to the correct locale
For these purposes, only the ISO639-1 language without country code is considered
Enums are used for stricter type assurance
Type hints are used for better static analysis and documentation
I have not bothered to add the complexity of input validation, so if input is invalid, there will be an exception thrown
Note the use of locale methods rather than direct int and float construction and formatting

strings.py
from locale import getlocale, getdefaultlocale

# For testing only
# from locale import setlocale, LC_ALL
# setlocale(LC_ALL, 'es_ES.UTF8')

def get_lang() -> str:
    lang, _ = getlocale()
    def_lang, _ = getdefaultlocale()
    return (lang or def_lang)[:2]

LANG = get_lang()

if LANG == 'en':
    DIMENSIONS = (
      'Temperature',
      'Length',
      'Mass',
      'Volume',
      'Time',
      'Currency',
    )
    TEMP_UNITS = (
      'Kelvin',
      'Celsius',
      'Fahrenheit',
    )
    ENTER_NUMBER = 'Enter the desired number: '
    CHOOSE_TEMP_SOURCE = 'Choose the source temperature unit to convert: '
    CHOOSE_TEMP_DEST = 'Choose the destination temperature unit to convert: '
    PROVIDE_TEMP = 'Provide the source temperature: '
    RESULT = 'The result of the conversion is:'

elif LANG == 'es':
    DIMENSIONS = (
      'Temperatura',
      'Longitud',
      'Masa',
      'Volumen',
      'Tiempo',
      'Divisa'
    )
    TEMP_UNITS = (
      'Kelvin',
      'Centígrada',
      'Fahrenheit'
    )
    ENTER_NUMBER = 'Entra número deseado: '
    CHOOSE_TEMP_SOURCE = 'Elija la unidad de temperatura fuente para convertir: '
    CHOOSE_TEMP_DEST = 'Elija la unidad de temperatura de destino para convertir: '
    PROVIDE_TEMP = 'Proporcionar la temperatura de la fuente: '
    RESULT = 'El resultado de la conversión es:'

convert.py
from enum import Enum
from locale import format_string, atof, atoi
from typing import TypeVar, Type, Iterable

from strings import *

class Dimension(Enum):
    TEMPERATURE = 1
    LONGITUDE = 2
    MASS = 3
    VOLUME = 4
    TIME = 5
    CURRENCY = 6

class TempUnit(Enum):
    KELVIN = 1
    CELSIUS = 2
    FAHRENHEIT = 3

# To get from this unit to Kelvin:
# kelvin = m * this_unit + b
TEMP_PARAMETERS = {
    # This unit       m  b
    TempUnit.KELVIN: (1, 0),
    TempUnit.CELSIUS: (1, 273.16),
    TempUnit.FAHRENHEIT: (5/9, 273.16 - 5/9*32),
}

InputEnum = TypeVar('InputEnum', bound=Enum)

def enum_from_input(t_enum: Type[InputEnum], labels: Iterable[str]) -> InputEnum:
    print(
        '\n'.join(
            format_string('%d = %s', (enum.value, label))
            for enum, label in zip(t_enum, labels)
        )
    )
    result = t_enum(atoi(input(ENTER_NUMBER)))
    print()
    return result

def main():
    dimension = enum_from_input(Dimension, DIMENSIONS)
    if dimension != Dimension.TEMPERATURE:
        raise NotImplementedError()

    print(CHOOSE_TEMP_SOURCE)
    source = enum_from_input(TempUnit, TEMP_UNITS)
    print(CHOOSE_TEMP_DEST)
    dest = enum_from_input(TempUnit, TEMP_UNITS)

    temp1 = atof(input(PROVIDE_TEMP))

    # Use a simple linear transformation where y = mx + b; first convert to
    # the base unit (Kelvin)
    m, b = TEMP_PARAMETERS[source]
    kelvin = m*temp1 + b

    # Now calculate inverse to the destination unit
    m, b = TEMP_PARAMETERS[dest]
    temp2 = (kelvin - b)/m

    print(format_string('%s %.1f', (RESULT, temp2)))

if __name__ == '__main__':
    main()

Example output
1 = Temperatura
2 = Longitud
3 = Masa
4 = Volumen
5 = Tiempo
6 = Divisa
Entra número deseado: 1

Elija la unidad de temperatura fuente para convertir:
1 = Kelvin
2 = Centígrada
3 = Fahrenheit
Entra número deseado: 3

Elija la unidad de temperatura de destino para convertir:
1 = Kelvin
2 = Centígrada
3 = Fahrenheit
Entra número deseado: 2

Proporcionar la temperatura de la fuente: 81
El resultado de la conversión es: 27,2


One element only subarray max length Task description

  You are given a list and an item, find the length of the longest consecutive subsequence of the list containing only the item.

Testcases

[1, 1, 2, 3] 1 -> 2
[1, 0, 0, 2, 3, 0, 4, 0, 0, 0, 6] 0 -> 3

This code is pretty readable in my opinion but I fear that the \$O(N^2)\$ time complexity is sub-optimal.

def subsequences(arr)
  ((0...arr.length).to_a)
    .repeated_permutation(2)
    .select {|start, finish| finish >= start}
    .collect {|start, finish| arr[start..finish] }
end

def longest_item_only_subsequence_len(arr, item)
   subsequences(arr)
     .select {|seq| seq.all? {|i| i == item} }
     .max_by(&:length)
     .length
end

p subsequences([1, 2, 3])
p longest_item_only_subsequence_len([1, 0, 0, 2, 3, 0, 4, 0, 0, 0, 6], 0)

Yes, chunk is the way to go. I'd write it differently, though, using a list-comphehension approach (in Ruby: map+if+compact. Or with a custom method.) and taking in account the edge case (no elements item in the array):

def longest_item_only_subsequence_len(xs, item)
  xs.chunk(&:itself).map { |y, ys| ys.size if y == item }.compact.max || 0
end


Printing the last two digits of an exponential calculation I am trying to create a program where a user inputs two numbers - a base and an exponent, while exponent is a binary number. Then prints out the last two digits of the result. My code is working well, but according to the time execution it is not the fastest way to do it. I read a little about using Horner's method, is there a way to use it here? If so, how?

base = int(input())
exponent = int(input())

def binaryToDec(binary):
    decimal, i = 0, 0
    while(binary != 0):
        dec = binary % 10
        decimal = decimal + dec * pow(2, i)
        binary = binary//10
        i += 1
    return decimal
exponent = binaryToDec(exponent)
result = base ** exponent
result = abs(result)%100
print(result)

For example, the output of 3 and 1010 should be 49.

Another example:

Input:

111111111111111111111111111111111111111111111111111111111111111111
111111111111111111111111111111111111111111111111111111111111111111

Output:

31

I expect the program to work faster in doing so, how can I do this?

document your code. In the code.
Python got it right with docstrings:
The laziest copying of a function includes its docstring, ready for introspection.
document what everything is there for wherever not blatantly obvious.
This would include the problem description - if it was in the source code, one might notice that the statements use an abs() where the description never mentions it.
let a tool help you to follow the Style Guide for Python Code.
your naming is OK (with the exception of result - there should be digits of power or modulus of power)
your binaryToDec() is too weird to mention - just try to describe what it does accomplish, and how.
read the requirement specification carefully. If possible, fix in writing how to check requirements are met.
You mention that your implementation is not the fastest way to do [modular exponentiation]: is there an upper limit on execution time? Rank among course-mates?
The way the exponent is specified looks constructed to head you to use one particular way to implement exponentiation.
know your python library. Mast commented the existence of a built-in - for conversion of strings to numbers using a non-default base, more likely than not.

How not to do more than hardly avoidable (/be "fast"):
For starters, only the last d digits of a number have any influence on the last d digits of its integral powers represented using the same base b.
Moreover, those last d digits become cyclic with b**d an integral multiple of cycle length - just watch out for things like 2**100.


Anything wrong with my typical module pattern? The javascript I typically need to write seems to be a good fit for the module pattern, e.g., repeatable widgets such as photo galleries, forms, etc. that have their own internal states, but may need to be accessible or editable by other parts of the website.

The particular flavor of the module pattern I've been using I inherited from a senior dev, but with that person out of the picture I'm taking a look at how I write my code so I can really understand it and ensure it's optimal.

I'll usually have one global object to contain my code and a selector library on hand (such as Zepto or jQuery). That global object would return specified submodules that I can call when I need them. Below is what a sample photo gallery module in the app might look like in skeleton form.

I'd love any input. I don't have a formal background in computer science or programming, particularly not OOP, so please point out any errors I've made in terminology as well.

var App = (function($) {

    "use strict";

    // images is an array of image paths
    // container is a DOM node where we append the gallery
    var Gallery = function(images, container) {

        // private variables go here

        // create a Zepto object from the DOM node that was passed in
        // it makes DOM operations easier and less verbose
        var $container = $(container);

        // these are prev/next buttons that will have event handlers on them
        var $next;
        var $prev;

        // an anonymous function expression executes initializers and event bindings
        (function() {

            // init renders the DOM we want
            init();

            // bind events on elements we just rendered
            bindEvents();

        })();

        function init() {

            // create DOM nodes from images, an array of image paths
            for (var i = 0; i < images.length; i++) {
                $container.append(render(images[i]));
            }

            // create the nav elements
            $container.append('');

            var $nav = $container.find('nav');
            $nav.append('
Next
');
            $nav.append('
Prev
');

            $next = $nav.find('.next');
            $prev = $nav.find('.prev');

        }

        function bindEvents() {

            $prev.click(function() {
                // show previous image
            });

            $next.click(function() {
                // show next image
            });

        }

        // this is a method of Gallery that would allow us to add more images to the gallery after it's already instantiated
        function addImage(path) {
            $container.append(render(path));
        }

        function render(path) {
            return '';
        }

        // return only the parts of the module we need exposed publicly
        return {
            addImage: addImage
        };

    };

    function init() {

        // an init function for the global App

    }

    // return the public properties of App
    return {
        Gallery: Gallery,
        init: init
    };

})(Zepto);

(function($) {

    App.init();

    var imagepaths = ["image1.jpg", "image2.jpg", "image3.jpg"];
    var galleryNode = document.getElementById('#gallery');

    $(document).ready(function() {
        new App.Gallery(imagepaths, galleryNode);
    });

})(Zepto);

I would suggest using a constructor and prototypes (as well as the module pattern) if you want to learn OOP in JavaScript. This is typically how libraries like jQuery and Zepto are built:

var App = (function($) {

  'use strict';

  // Gallery

  var Gallery = (function() {

    // Constructor
    function Gallery(images, container) {
      this.images = images;
      this.$container = $(container);

      // Create elements we need first,
      // and cache them in variables to use them
      // in prototype methods
      this.$nav = $('');
      this.$next = $('
Next
');
      this.$prev = $('
Prev
');

      // Initialization
      this._init();
      this._events();
    }

    // Methods that start with underscore are
    // meant for internal use, even though they are still public
    // it is a well known convention
    Gallery.prototype._init = function() {
      this.addImages(this.images);
      this.$nav.append(this.$next, this.$prev);
      this.$container.append(this.$nav);
    };

    Gallery.prototype._events = function() {
      this.$prev.click(function() {
        //...
      });
      this.$next.click(function() {
        //...
      });
    };

    // Public method
    Gallery.prototype.addImages = function(imgs) {
      // Append all images at once for best performance
      this.$container.append(imgs.map(render).join(''));
    };

    // Helpers

    // This function is available in the code above
    // because function declarations get hoisted to the top
    // of the scope, as opposed to function expressions, ie:
    // "var render = function(){}"
    function render(path) {
      return '';
    }

    // Return a wrapper that will let us call "Gallery"
    // without using the "new" keyword
    return function(x, y) {
      return new Gallery(x, y);
    };
  }());

  // App

  function init() {
    // app init
  }

  return {
    Gallery: Gallery,
    init: init,
  };

})(Zepto);

(function($) {

  App.init();

  var imgs = ['image1.jpg', 'image2.jpg', 'image3.jpg'];

  // short for document.ready(fn)
  $(function() {
    // We get back an instance of "Gallery"
    // without using the "new" keyword
    var gallery = App.Gallery(imgs, '#gallery'); // you can pass a selector

    // Add more images using public method
    gallery.addImages(['a.jpg', 'b.png']);
  });

})(Zepto);

Once you feel more comfortable with prototypes and inheritance in JavaScript, I would probably consider making a jQuery/Zepto plugin for your gallery, so you'd be able to call it on your elements like $('#gallery').myGalleryPlugin(images).


How can I make my filter search code more efficient? Here is App.js

const listItems=document.querySelector(".student-list").children;
const studentList=document.querySelector('.student-list');
const numberOfItems=10;
const page=document.querySelector(".page");

displayPage=(list,pageNumber)=> {
   const SI=(pageNumber*numberOfItems)-numberOfItems;
   const EI=pageNumber*numberOfItems;
   Array.from(list).forEach((item,index)=> {
      if (index>= SI && index<EI) {
         item.style.display="block";
      } else {
         item.style.display="none";
      }
   })
}

addPaginationLinks=(list)=> {
   const pages=Math.floor(list.length/10)
   let html=``;
   for (let i=0; i<pages; i++) {
      if (i===0) {
         html+=`

            ${i+1}

`;
      } else {
         html+=`

            ${i+1}

`;
      }

   }
   const ul=document.createElement("ul");
   ul.innerHTML=html;
   const div=document.createElement("div");
   div.classList.add("pagination");
   div.appendChild(ul);
   page.appendChild(div);
}

displayPage(listItems,1);
addPaginationLinks(listItems);

addEventListener=()=> {
   const a=document.querySelectorAll("a");
   a.forEach(item=> {
      item.addEventListener('click', (e)=> {
         a.forEach(item=> {
            if (item.classList.contains("active")) {
               item.classList.remove("active")
               e.target.classList.add("active");
            }
         })
         const pageNumber=parseInt(e.target.textContent);
         displayPage(listItems,pageNumber);
      })
   })
}

addEventListener();

addSearchComponent=()=> {
   const pageHeader=document.querySelector(".page-header.cf")
   let html=`

      <input placeholder="Search for students...">
      <button>Search</button>

`;
   pageHeader.insertAdjacentHTML('beforeend', html);
}

addSearchComponent()

const search=document.querySelector("input");
const studentDetails=document.getElementsByClassName("student-details");

noResultsItem=()=> {
   const item=`

No Results Shown Found

`;
   studentList.insertAdjacentHTML('beforeend', item);
}

noResultsItem()

search.addEventListener('keyup', (e)=> {
   const noResults=document.querySelector(".no-results");
   const array=Array.from(studentDetails).filter(student=>student.children[1].textContent.includes(e.target.value))
   if (array.length==0) {
      Array.from(studentDetails).forEach(student=>{
         student.parentNode.style.display="none";
      });
      noResults.style.display="block"
   } else if (array.length>0) {
      noResults.style.display="none"
      Array.from(studentDetails).forEach(student=>{
         if (student.children[1].textContent.includes(e.target.value)) {
            student.parentNode.style.display="block";
         } else {
            student.parentNode.style.display="none";
         }
      });
   }
});

I'm trying to refactor the Javascript code at the bottom of my app.js.

There's a keyup event in the search input, and when no results are found, I want to display the no results li item and also make it disappear immediately in case a user types in a character that creates a search match. However, I feel like I could have written this much better. I made the li's display none by default, but it feels tedious to re-alter it to display: none and display: block in both the if conditions.

Does anyone have any suggestions to a beginner programmer on how to make the code more efficient?

1) Avoid unnecessary document.querySelector and document.querySelectorAll. Store its results if you sure there won't be any changes.

Element searching in the DOM is relatively hard operation (it is extremely optimized, but still). So you should avoid it if you can.

You've already done a lot for that, but I still can see unsaved document.querySelector(".page-header.cf")

2) Use event delegation pattern.

You don't have to add event listeners to each element on the page. Especially if they are in the same container.

const parent = document.querySelector('#parent');

parent.addEventListener('click', (e) => {
  const target = e.target.closest('a');

  if (!target) return;

  if (target.matches('.active')) {
    target.classList.remove('active');

    resetPage();
  } else {
    target.classList.add('active');

    const page = target.textContent;

    displayPage(page);
  }
});

  1
  1

As a bonus, you don't have to add event listeners on newly created links or remove it from removed links.

3) Do not modify elements content as HTML string if you can avoid it.

In your case you can move li.no-results outside of studentList and show/hide it with display: none/block property. Or at least remove/append this element as a node, not as HTML-string. It is much faster.

4) Use debounce.

You don't need to trigger the search on each keyup. You need to trigger it only when user stops typing. So when you have event series with, let's say, 200 ms interval, you need to trigger search only for the last one. You can use lodash/debounce for that or write your own implementation

5) Learn how to use Chrome Profiling Tool
It helps to find bottlenecks of your app and fix them.