Run 'N' Number of PHP Scripts on shell forever in a loop My objective is to run some 'N' number of PHP Scripts in cron.

Since these are web scrapers coded in PHP and hits database on my server, each script runs for like 1 hour and they take a lot of CPU and memory. That's why I want to run one script at a time.

My first approach was to run each script at specific hour of the day but then there are only 24 hours in a day, so I can't really go with that way.

I have prepared a Unix script which runs in an infinite loop and runs the entire script. Assuming the shell will execute one script, wait for some time and then execute the next script. This is my script 1, say.

Then there is another script, Script 2 which runs every 3 hours and checks if Script 1 is running. If Script 1 is not running it will execute Script 1.

My concerns:

Is it feasible to run like 100 PHP scripts from one Unix script? If one fails, will it fail the whole script?
Is there any better approach?

Script 1 (MasterCronCarlos.sh):

#!/bin/bash
# Description = Script to check cpu usage and if above threshold issue a linux based command.
# This Script only checks "user cpu time (us - userspace (doing userspace stuff))
#

SUBJECT="CRON Status | Jai Carlos"
TO="myemail@gmail.com"

while true
do

echo -en "Starting fresh loop of cron" | mail -s "$SUBJECT" "$TO"

    php /home/site1/public_html/scrapperM1.php
    sleep 300
    php /home/site1/public_html/scrapperM2.php
    sleep 300
    php /home/site1/public_html/scrapperM3.php
    sleep 300

    echo -en "Script completed for site1.com" | mail -s "$SUBJECT" "$TO"

    php /home/site2/public_html/scrapperM1.php
    sleep 300
    php /home/site2/public_html/scrapperM2.php
    sleep 300
    php /home/site2/public_html/scrapperM3.php
    sleep 300

    echo -en "Script completed for site2.tv" | mail -s "$SUBJECT" "$TO"

    php /home/site3/public_html/scrapperM1.php
    sleep 300
    php /home/site3/public_html/scrapperM2.php
    sleep 300
    php /home/site3/public_html/scrapperM3.php
    sleep 300

    echo -en "Script completed for site3" | mail -s "$SUBJECT" "$TO"

    php /home/site4/public_html/scrapperM1.php
    sleep 300
    php /home/site4/public_html/scrapperM2.php
    sleep 300
    php /home/site4/public_html/scrapperM3.php
    sleep 300

    echo -en "Script completed for site4" | mail -s "$SUBJECT" "$TO"

    php /home/site5/public_html/scrapperM1.php
    sleep 300
    php /home/site5/public_html/scrapperM2.php
    sleep 300
    php /home/site5/public_html/scrapperM3.php
    sleep 300

    echo -en "Script completed for site5" | mail -s "$SUBJECT" "$TO"

    php /home/site6/public_html/scrapperM1.php
    sleep 300
    php /home/site6/public_html/scrapperM2.php
    sleep 300
    php /home/site6/public_html/scrapperM3.php
    sleep 300

    echo -en "Script completed for site6" | mail -s "$SUBJECT" "$TO"

    php /home/site7/public_html/scrapperM1.php
    sleep 300
    php /home/site7/public_html/scrapperM2.php
    sleep 300
    php /home/site7/public_html/scrapperM3.php
    sleep 300

    echo -en "Script completed for site7" | mail -s "$SUBJECT" "$TO"

done

Script 2 (mastercron.sh):

#!/bin/sh

RESULT=`ps axf | grep "MasterCronCarlos.sh" | grep -v grep | awk '{print  $1}'`

if [ $RESULT -ge 0 ]; then

echo "Running"

else

echo "Not Running"

sh /home/backups/MasterCronCarlos.sh

fi;

I think more than even a code review, what you are really needing to think about is your architecture.  In the modern age of event-based workflows and parallel computing should you really be managing and executing a complex set of scripts in this manner (i.e. via shell automation)?

My advice is to look for better tooling that will allow you to orchestrate your pieces of logic via an automated workflow of some sort.  Even consider things like map reduce tools (Hadoop, Elastic Map Reduce, etc.) which are ideal for parallel workload execution with aggregation of results.


Alternative to using strncpy() as 'safe' version of strcpy() I am thinking of using this as part of a header file to give me a useful string-manipulation tool. Elements is size of dest to prevent overflow:

int copystring(char *dest,char *source,int elements)
{
    int run,omitted=0;
    for(run=0;run<elements-1;run++)//last reserved for'\0'
    {
       dest[run]=source [run];
       if (dest[run]=='\0')
       {
           break;
           //doesn't increment, so a below will be '\0'
       }
    }
    //now see if characters missed out, starting from first non-copied character
    char a=source[run];
    while (a!='\0')
    {
       run++;//move on to next char in source
       omitted++;// missed out one more char
       a=source[run];
    }
    dest[elements-1]='\0';//could add an if condition if omitted is 0
    return omitted;///tells parent function how many letters weren't copied
}

It may be worth my while adding a macro defining maximum amount of omitted chars to check for, and if exceeded to return -1.

Advantages: terminates in null charcter like strcpy while avoiding overflow like strncpy; returns the amount of characters, if any, that were omitted, so saves writing bit of code for handling this if needed.

Just a few comments.

The standard type for specifying something like the maximum length of a string is size_t. You should probably use that instead of int.
traversing the remainder of the string on the off chance that somebody might want to know how much was skipped seems like poor design. Unless you know you're going to use that (frequently), I'd advise against it. If somebody wants that information, strlen(s) - size is pretty trivial to type, so it's not like you're saving them something terribly arduous.
I think most people accustomed to C would probably use pointer notation throughout.

char *copystring(char *restrict dest, char const *restrict source, size_t elements) {
    char *d;
    for (d = dest; d + 1 < dest + elements; d++, source++)
        *d = *source;
    *d = '\0';
    return d;
}

This returns a pointer to the end of the new string, which frequently is quite useful. For example, if you're building up a string with data filled in at appropriate points, it's often useful to limit the size of each field you insert to a specific maximum, then get a pointer to where you're going to add the next item (though, in fairness, something like snprintf is often more convenient still).


Merging hashes, summing values that have the same key I'm trying to group by keys in an Array of Hashes in the following form and add their totals. Maybe with #map and #reduce/#inject. I know there is a more compact way to do this.

Given I have

codes_and_totals = [{'1001' => 153, '212' => 153}, {'212' => 1}]

When I run

process_codes_and_totals(codes_and_totals)

Then the return value should be

=> {'1001' => 153, '212' => 154}

Currently #process_codes_and_totals looks like this

def process_codes_and_totals(codes_and_totals)
    totals_across_locations = {}
    codes_and_totals.each do |location|
      location.each do |class_code, total|
        totals_across_locations[class_code] ||= 0
        totals_across_locations[class_code] += total
      end
    end

    totals_across_locations
end

One way might be to use #reduce (as you suggest) and Hash#merge with a block:

totals = codes_and_totals.reduce({}) do |sums, location|
  sums.merge(location) { |_, a, b| a + b }
end

The merge-block is only invoked if a key exists in both the hashes being merged, so we don't need to guard against nil or start with a zero value. If the block runs, it's because there are two number values that need to be merged (i.e. added, in this case).


Bitwise operation to get the weekday name The function days_name receive a val (string) corresponding to weekdays in hexadecimal.

def days_name(val)
    value = val.to_i(16)
    week = Array.new

    if value.eql? 127
      week.push 'All'
    elsif value.eql? 0
      week.push 'No'
    else
      week.push 'mon' if (value & 1) > 0
      week.push 'tue' if (value & 2) > 0
      week.push 'wed' if (value & 4) > 0
      week.push 'thu' if (value & 8) > 0
      week.push 'fri' if (value & 16) > 0
      week.push 'sar' if (value & 32) > 0
      week.push 'sun' if (value & 64) > 0
    end
    week.join(' ')
end

days_name? Shouldn't that rather be day_names? Actually, given its function it should probably be weekdays
Shouldn't "sar" be "sat" for Saturday? Never seen it abbreviated "sar"
The opposite of "all" is "none" - not "no"
Day names are capitalized in English, so "Mon", "Tue", "Wed", etc.
You rarely need to create an array and then push items to it. It's easier to simply filter a complete array. But if you do need to create an empty array, just use [] - no need for Array.new

Sticking close to your current code, I'd do this

def weekdays(hex)
  value = hex.to_i(16)
  case value
  when 127
    'All'
  when 0
    'None'
  else
    days = %w(Mon Tue Wed Thu Fri Sat Sun)
    days.each_with_index.select { |_, i| value & 2**i > 0 }.map(&:first).join(" ")
  end
end

However, I'd probably prefer a method that simply returns an array of day names, rather than a string. Then let other code check and join the array as needed.

Something like this

def weekdays(hex)
  value = hex.to_i(16)
  days = %w(Mon Tue Wed Thu Fri Sat Sun)
  days.each_with_index.select { |_, i| value & 2**i > 0 }.map(&:first)
end

Update: As tokland points out in the comments, it'd be cleaner to use map.with_index plus compact, rather than each_with_index.select plus map. I.e.

def weekdays(hex)
  value = hex.to_i(16)
  days = %w(Mon Tue Wed Thu Fri Sat Sun)
  days.map.with_index { |day, i| day if value & 2**i > 0 }.compact
end

or, for the first code block

def weekdays(hex)
  value = hex.to_i(16)
  case value
  when 127
    'All'
  when 0
    'None'
  else
    days = %w(Mon Tue Wed Thu Fri Sat Sun)
    days.map.with_index { |day, i| day if value & 2**i > 0 }.compact.join(" ")
  end
end

As mentioned in the comments, I happened to make a gem for weekday bitmasks a while back, which you might find useful. The major difference is that the gem expects Sunday to be zero, Monday to be 1 and so forth (like Date#wday and most other date APIs).


Returning Key Values from Stored Procedures I wonder which is better practice when I need to return the primary key value of a newly inserted record from a SQL stored procedure. Consider the following implementations:

As Return Value

CREATE PROCEDURE [CreateRecord] ( @value NVARCHAR(128) )
AS
BEGIN
    INSERT [Records] ( [Value] ) VALUES ( @value );
    RETURN SCOPE_IDENTITY();
END

Obviously this only works for INT keys, but it takes advantage of pre-existing functionality. Also, only works when inserting a single record. I have seen this done in few places, but I generally don't like it.

As Output Parameter

CREATE PROCEDURE [CreateRecord] ( @value NVARCHAR(128), @id INT = NULL OUTPUT )
AS
BEGIN
    INSERT [Records] ( [Value] ) VALUES ( @value );
    SET @id = SCOPE_IDENTITY();
END

Works for any type of key, including multi-column keys, but still only works when inserting a single record. This is my preferred method unless I absolutely have to insert more than one record at a time.

As XML Output Parameter

CREATE PROCEDURE [CreateRecord] ( @value NVARCHAR(128), @xmldata XML = NULL OUTPUT )
AS
BEGIN
    DECLARE @inserted TABLE ( Id INT );

    INSERT [Records] ( [Value] )
    OUTPUT [inserted].[Id]
    INTO @inserted ( [Id] )
    VALUES ( @value );

    SET @xmldata = (SELECT [Id] FROM @inserted FOR XML AUTO);
END

This is similar to the previous solution, but allows multiple records to be returned. Trying to parse the output would be a huge pain, but I suppose this would be suitable for some applications. For example, if a stored procedure can insert into several different tables, a single output parameter can describe all the inserted records.

As Result Set

CREATE PROCEDURE [CreateRecord] ( @value NVARCHAR(128) )
AS
BEGIN
    DECLARE @inserted TABLE ( Id INT );

    INSERT [Records] ( [Value] )
    OUTPUT [inserted].[Id]
    INTO @inserted ( [Id] )
    VALUES ( @value );

    SELECT * FROM @inserted;
END

Superficially, this may be appealing, especially if your calling this SP from ADO, since getting the result set from the command is very straight forward. To use this SP from SQL, however, you have to write an INSERT-EXEC script, which is somewhat inelegant, in my opinion (but still a lot better than parsing XML).

Perhaps there are other methods that I'm not even aware of. What is the best method of returning the newly inserted key value from this kind of stored procedure? Are there any guidelines for deciding which is the best method?

I think you've already answered your own question to some degree: there are multiple options for returning data from a stored procedure and they all behave differently so there is no single answer. But there are definitely some general guidelines and common practices.

RETURN is a control-of-flow statement: its primary use is to exit a procedure, not to return data. But of course return codes are a very common feature of many programming languages, and the documentation makes that usage pattern explicit:

  A stored procedure can return an integer value called a return code to
  indicate the execution status of a procedure

So you should use this not to return data, but to return metadata about the execution of the procedure. Usually this means did it succeed or fail, and if it failed what was the error status. Whether or not you actually want to use this feature is entirely up to you, of course.

Output parameters are the standard way of returning scalar values, and are especially useful when the procedure is called by another piece of TSQL code, because as you mentioned sharing data between stored procedures is not always easy, especially if you want to share a result set. The only obvious reason not to use them for scalar values is if your client code - perhaps an ORM or DAL - and/or coding practices make it easier to consume a result set (more on that below).

I'm not going to comment on your XML solution because I'm not an XML guy, but it seems very awkward and unless your client code could only consume XML then I'm not sure what the point would be.

A result set is more flexible than an output parameter because it can return multiple rows (obviously), so if you need a result set then it's the only choice anyway.

(Strictly speaking that isn't true, because you can return a cursor as an output parameter, but avoiding cursors whenever possible is a good rule in TSQL and I personally don't see any advantage over returning a result set. Other than it being easier for calling code in TSQL to consume the cursor.)

That all means that the 'correct' answer to your question is to use an output parameter when returning one key value, or a result set when returning more than one. But, that will add some complexity to your calling code because it needs to know which procedures return output parameters and which ones return result sets (or both).

For that reason it's not uncommon for organizations to make explicit coding rules that say that stored procedures must always return data in result sets (usually one and only one per procedure). Then developers don't need to worry about it, they just always write code to consume result sets, even if some result sets always have just one row. This is an important consideration, because developers' time is usually by far the most expensive component of any application, so anything you can do to make development simpler, faster and less error-prone is very worthwhile.

Perhaps there's a performance difference between using output parameters and result sets, but if you're worried about that then set up a test and gather some performance data (it's almost always better to test and measure performance yourself before asking someone else's opinion, since that person's general answer may turn out to be wrong for your specific case). And even if there is some performance difference, if it isn't big enough to impact your application then you can ignore it and use whichever mechanism you prefer.


Return row count from mysqli prepared statement I want to know if it is okay/safe to use a prepared statement and mysqli_num_rows like this:
public function getUnreadNumber() {
    $userLoggedIn = $this->user_obj->getUsername();

    // get_result for * and bind for select columns
    // bind_result Doesn't work with SQL query that use *
    $query = $this->con->prepare('SELECT * FROM notifications WHERE viewed="0" AND user_to = ? ');
    $query->bind_param("s", $userLoggedIn);
    $query->execute();
    $query_result = $query->get_result();

    return mysqli_num_rows($query_result);
}

Or should I do this?
$query = $this->con->prepare('SELECT * FROM notifications WHERE viewed="0" AND user_to = ? ');
$query->bind_param("s", $userLoggedIn);
$query->execute();
$query_result = $query->get_result();
$numRows = $query_result->num_rows;

return $numRows;

You should definitely not be mixing procedural and object-oriented syntax.
Although it works with un-mixed syntax, the process is working harder than it needs to and delivering more result set data than you intend to use.
I would add COUNT(1) or COUNT(*) to the sql like this:
$sql = "SELECT COUNT(1) FROM notifications WHERE viewed='0' AND user_to = ?";
$query = $this->con->prepare($sql);
$query->bind_param("s", $userLoggedIn);
$query->execute();
$query->bind_result($count);
$query->fetch();
return $count;

Assuming the sql is not broken due to syntax errors, this will always return a number.


Get length of the longest sequence of numbers with the same sign I need to get length of the longest sequence of numbers with the same sign. Assumes that zero is positive. For example:

{10, 1, 4, 0, -7, 2, -8, 4, -2, 0} → 4
{0, 1, 2, 3, -2, -4, 0}  → 4
{1, -2, 0, -1} → 1

I wrote a function:

unsigned getLongestSameSignSequenceLength(std::vector<int> const& a)
{
    unsigned maxlen = 1;

/* Assumes that zero is positive. */
#define SIGN(a) (a >= 0)

    for (size_t i = 1, len = 1; i < a.size(); i++, len++) {
        if (SIGN(a[i]) != SIGN(a[i - 1])) {
            maxlen = std::max(maxlen, len);
            len = 0;
        } else {
            if (i == a.size() - 1)
                return std::max(maxlen, len + 1);
        }
    }

#undef SIGN

    return maxlen;
}

Can you please give me tips to improve my code?

A small portability bug: std::size_t is in the std namespace, assuming it's declared by including <cstddef> (recommended).

No unit tests are included, but I'd expect one that tests that the result is zero when the input collection is empty.  We need to initialize maxlen to zero for that test to pass.

When comparing consecutive elements of a collection, always consider using std::adjacent_find().  With a suitable predicate function, we can find changes from negative to non-negative and vice versa without needing to code our own loop or do any indexing.

(More advanced) Consider making your algorithm generic, templated on an iterator type, so that it can be applied to any collection (or even to an input stream directly).

Here's a version that applies all of these suggestions (and some from other answers that I've not repeated above):

#include <algorithm>
#include <cmath>
#include <cstddef>
#include <iterator>

template<typename ForwardIt>
std::size_t getLongestSameSignSequenceLength(ForwardIt first, ForwardIt last)
{
    auto const signdiff =
        [](auto a, auto b){ return std::signbit(a) != std::signbit(b); };

    std::size_t maxlen = 0;

    while (first != last) {
        ForwardIt change = std::adjacent_find(first, last, signdiff);
        if (change != last) { ++change; }

        std::size_t len = std::distance(first, change);
        if (len > maxlen) { maxlen = len; }

        first = change;
    }

    return maxlen;
}

// tests:

#include <vector>

int main()
{
    struct testcase { std::size_t expected; std::vector<int> inputs; };
    std::vector<testcase> tests
        {
         {0, {}},
         {1, {1}},
         {1, {1, -2}},
         {1, {1, -2,  3}},
         {1, {-1, 2, -3}},
         {2, {1,  2}},
         {2, {1,  2, -3}},
         {2, {-1, -2, 3}},
         {2, {-1, 2,  3}},
         {2, {-1, 2,  3, -4}},
        };

    int failures = 0;
    for (auto const& [e, v]: tests) {
        failures += getLongestSameSignSequenceLength(v.begin(), v.end()) != e;
    }

    return failures;
}


Parsing and formatting a time two ways (perfectionistic approach) I'm still not satisfied by the readability of the code. The wanted behavior which would be self descriptive would sound like: "I've two fields; one is for the date and one for the hour. This two fields should be in a UserControl of which I have two instances, one is for the event time and one is for the event end time."

My personal opinion is all the extra code is something which is specific to C# syntax, and not to my purpose. Handling null, exceptions, conversions between dates and strings makes me to write that extra code and a developer could get confused and wonder on what was the purpose of that solution. The only thing which is at a functional level in this code is an assignment.

The String.Format is an example of what I mean... it's a really mean solution. Now that every approach tries to separate Model, View and Controllers, I'm mixing the solution I found to to fill a gap of conversion and the code wrote to make the behavior I needed.

Which solution would you use?

First formulation

public override void OnEventTimeChanged()
{
    DateTime eventDateWithTime;
    TimeSpan eventTime;

    if (TimeSpan.TryParse(EventTime, out eventTime))
    {
        eventDateWithTime = (EventDate.Date + eventTime);
        CultureInfo cultureInfo = Thread.CurrentThread.CurrentUICulture;
        EndTime = String.Format(cultureInfo, "{0:HH:mm}", eventDateWithTime);
        EndDate = eventDateWithTime;
    }
}

Second formulation

public override void OnEventTimeChanged()
{
    CultureInfo cultureInfo = Thread.CurrentThread.CurrentUICulture;

    TimeSpan eventTime;
    if (!TimeSpan.TryParse(EventTime, out eventTime)) return;

    DateTime eventDateWithTime = (EventDate.Date + eventTime);
    string eventTimeString = String.Format(cultureInfo, "{0:HH:mm}", eventDateWithTime);

    EndTime = eventTimeString;
    EndDate = eventDateWithTime;
}

In the 2nd version, the if acts as a guard: it prevents invalid states to get through it. Many people prefer it this way, for example in this related post:
Should I return from a function early or use an if statement?

Which way you choose, is a matter of taste. There's nothing wrong with either one.

If you do go with the 2nd one, it can be improved a little bit:

public override void OnEventTimeChanged()
{
    TimeSpan eventTime;
    if (!TimeSpan.TryParse(EventTime, out eventTime))
    {
        return;
    }

    DateTime eventDateWithTime = EventDate.Date + eventTime;
    CultureInfo cultureInfo = Thread.CurrentThread.CurrentUICulture;
    string eventTimeString = String.Format(cultureInfo, "{0:HH:mm}", eventDateWithTime);

    EndTime = eventTimeString;
    EndDate = eventDateWithTime;
}

That is:

cultureInfo is not needed in the if, so declare and initialize it later. Even more precisely, declare and initialize it right before you really need it.
It's recommended to use braces with if statements always, even if only one line will go inside the block. Remember this bug: https://www.imperialviolet.org/2014/02/22/applebug.html


Java Pong Game Collision between bat and ball I have a very simple Pong game that I've built in Java. The code is quite long so I've decided to focus this question on the collision that occurs with the ball and the bat and also the effects in the game. I'm using ACM Graphics package to learn Java so most of the methods are from that package. I want to know how I can improve this checking process and if the way I revert the speed and direction is efficient. I'm also open to any suggestions for the game.

//Setting up variables
static final int WAIT = 50;
static final int MV_AMT = 20;
static final int BATWIDTH = 120;
static final int BATHEIGHT = 20;
static final int WINDOWX = 400;
static final int WINDOWY = 400;
static final int BALLRADIUS = 10;
private int batX = 150, batY = 400; //Starting positions
private int ballX = 160, ballY = 370;
private int ballSpeedX = 2; //the ball speed on the X axis
private int ballSpeedY = -9; //the ball speed on the Y axis

public void run(){
    //... Stuff that runs before the game, ie. draw the sceen etc.

    int currentTime = 0;
    //Do all our stuff here
    while(continueGame){
        //Pause dat loop
        pause(WAIT);
        currentTime = currentTime + WAIT;

        //Up the speed every 5 seconds
        if (currentTime % 5000 == 0) {
            if(ballSpeedY>0)
                ballSpeedY += 2;
            else
                ballSpeedY -= 2;

            if(ballSpeedX>0)
                ballSpeedX += 2;
            else
                ballSpeedX -= 2;
        }

        //Move the ball
        ballX=ballX+ballSpeedX;
        ballY=ballY+ballSpeedY;

        ball.setLocation(ballX, ballY);

        //Check
        checkCollisions();
    }

    //... Stuff that gets done after game over
}

public void checkCollisions(){
    //This method is quite long so I won't be posting it all
    //Just the part the calls the collision method

    //Get the bounds
    GRectangle batBounds = bat.getBounds();
    GRectangle ballBounds = ball.getBounds();

    //Where is the ball?
    ballX = (int)ball.getX();
    ballY = (int)ball.getY();

    //Where is the bat?
    batX = (int)bat.getX();
    batY = (int)bat.getY();

    //Did the bat touch the ball?
    if(batBounds.intersects(ballBounds)){
        batCollision();
    }
}

public void batCollision(){
    if( ballX+BALLRADIUS > batX+(BATWIDTH/2) ){ //Which side of the bat?
        //Which direction is the ball traveling when it hits?
        if(ballSpeedX >> 31 !=0){
            ballSpeedX = ballSpeedX * -1;
        } else {
            ballSpeedX = ballSpeedX;
        }
    } else {
        if(ballSpeedX >> 31 !=0){
            ballSpeedX = ballSpeedX;
        } else {
            ballSpeedX = -ballSpeedX;
        }
    }

    ballSpeedY = -ballSpeedY; //Adjust Y speed
}

In the original Pong® brand video game, there were two bats which were confined to move vertically.  A collision would be detected if ball circuit was triggered at the same time as one of the bat circuits, and if the ball was not already moving in the proper direction for that bat.  The ball's vertical speed would be set to an odd number in the range -15 to +15 based upon the number of scan lines of bat that were displayed before the collision was detected (basically the difference between the ball's Y position and the bat's position).  What exact bounce behavior are you looking for in your game?


Golang keep installation status for multiple installations process Im fairly new to Golang and I appreciate any feedback about how to make the code cleaner.
The code is called by go-routine and should update status (fail/success etc) of several installation process.
package avss

import (
    "context"
    "sync"

    "helm.sh/helm/v3/pkg/release"

    "sigs.k8s.io/controller-runtime/pkg/client"

    avmv1alpha1 "github.vmp/avs/api/v1alpha1"
)

func GetUpdateStatus(ctx context.Context, log logr.Logger, cr client.Object, status *avmv1alpha1.ComponentStatusWithAggregation, statusClient client.StatusClient) func(chartStatus *avmv1alpha1.InstallStatus) error {
    var conditionMutex sync.Mutex

    return func(chartStatus *avmv1alpha1.InstallStatus) error {

        // prevent status update by subroutines at the same time, lock the access to prevent concurrent modification
        conditionMutex.Lock()
        defer conditionMutex.Unlock()
        status.ChartStatus = updateStatus(chartStatus, status.ChartStatus)
        status.Status = overallStatus(status.ChartStatus)
        err := statusClient.Status().Update(ctx, cr)
        return err
    }
}

func overallStatus(chartsStatus []avmv1alpha1.InstallStatus) string {
    overallStatus := ""
    for _, status := range chartsStatus {
        switch status.ReleaseStatus {
        case release.StatusFailed.String():
            overallStatus = "error"
        case release.StatusDeployed.String():
            if overallStatus == "" {
                overallStatus = "installed"
            }
        default:
            if overallStatus != release.StatusFailed.String() {
                overallStatus = "reconcile"
            }
        }
    }
    return overallStatus
}

func updateStatus(newStatus *avmv1alpha1.ChartStatus, cStatus []avmv1alpha1.InstallStatus) []avmv1alpha1.ChartStatus {
    found := false
    for i, status := range cStatus {
        if status.Name == newStatus.Name {
            found = true
            cStatus[i] = *newStatus
        }
    }
    if !found {
        cStatus = append(cStatus, *newStatus)
    }
    return cStatus
}

https://play.golang.org/p/9C8xGV2116O
The code is working! however, I guess it can be improved as this is my first Go program which should be run in production :-)
If the code is perfect,   please let me know

Overall this looks like a pretty sensible file. Some minor points you might want to consider follow.
Imports
logr is not imported in the file. I presume this is https://github.com/go-logr/logr. It's passed into the first function but not used. Perhaps remove it from the signature?
Discrete strings
overallStatus returns one of four strings. You could define these strings as constants like Siva mentioned. A step further is to introduce a type that groups these strings together. This

ensures the function returns one of the strings defined, and
makes it clear what strings can be returned from the function by looking at the signature.

type status string

const (
    statusUnknown   status = ""
    statusError     status = "error"
    statusInstalled status = "installed"
    statusReconcile status = "reconcile"
)


Python implementation of stack to return minimum in O(1) time I have written a stack in Python that is required to push, pop and return the minimum of the stack in \$O(1)\$ time.

#!python3
class Stack():
    def __init__(self):
        self.items = []
        self.min = None

    def push(self, item):
        self.items.append(item)
        self.minimum()

    def pop(self):
        if self.isEmpty():
            raise ValueError('Empty stack')
        else:
            return self.items.pop()

    def minimum(self):
        if self.min is None:
            self.min = self.peek()
        else:
            if self.peek() < self.min:
                self.min = self.peek()

    def getMinimum(self):
        return self.min

    def peek(self):
        try:
            return self.items[-1]
        except IndexError as e:
            print(e)

    def size(self):
        return len(self.items)

    def isEmpty(self):
        return self.size() == 0

stack = Stack()

nums = [6,4,8,9,1,5,2,3]
for i in nums:
    stack.push(i)

print(stack.getMinimum())

What I have tried to do is, compare the current minimum with each new addition to the stack so that the

self.min

attribute constantly updates.

Eventually, to get the minimum, I simply call the

self.getMinimum()

function to return the minimum. My question is, does this actually occur in \$O(1)\$ time?

The minimum gets updated after a push but not after a pop, causing getMinimum to return the wrong value:

>>> stack = Stack()
>>> stack.push(2)
>>> stack.push(1)
>>> stack.getMinimum()
1 # correct
>>> stack.pop()
1
>>> stack.getMinimum()
1 # wrong: the only item in the stack now is the number 2


Recursive binary search in Python I have implemented a recursive binary search in Python and tried to implement some verification to my code. Other than that, is there any optimization I am missing?

def isSorted(ary):
    for i in range(1, len(ary)):
        if (ary[i] < ary[i - 1]):
            return False
    return True

def binSearch(ary, elem, first, last):
    if not isSorted(ary):
        raise AssertionError("Elements are not in sorted order!")
    mid = (first + last) / 2
    if (ary[mid] < elem):
        return binSearch(ary, elem, mid + 1, last)
    elif (ary[mid] > elem):
        return binSearch(ary, elem, first, mid - 1)
    else:
        return mid

ary = (1, 2, 3, 4, 5, 6, 7)
print binSearch(ary, 5, 0, len(ary) -1)

Specify Requirements Up Front

The whole point of binary search is that it's O(lg N). If you're going to check that the array is sorted first, you may as well do a linear search. Not to mention that you're doing even worse here since you're checking that the array is sorted on every recursive call, so now we have an O(N lg N) search. Binary search should assume a sorted list. If the user doesn't provide one, it's garbage in garbage out.

Use appropriate defaults

Usage-wise, it makes more sense to just write:

idx = binSearch(ary, 5)

So let's do:

def binSearch(ary, elem):
    def recurse(first, last):
        ...

    return recurse(0, len(ary)-1)

What if it's not found?

You should have an error case, if first > last, you should return None. Otherwise, you have infinite recursion.

Full solution:

def binSearch(ary, elem):
    def recurse(first, last):
        mid = (first + last) / 2
        if first > last:
            return None
        elif (ary[mid] < elem):
            return recurse(mid + 1, last)
        elif (ary[mid] > elem):
            return recurse(first, mid - 1)
        else:
            return mid

    return recurse(0, len(ary)-1)


Calculating the number of coins in money change I wrote a program that calculates the minimum number of coins required to give a user change.

One concern I have is: When do we initialize the value of a float to be negative or positive? I recently saw

float userInput = -1.0;

but what about

float userInput = 1.0;

Are there differences between the two, and when would one be used over the other?

#include <stdio.h>
#include <cs50.h>
#include <math.h>

    int main(void)
{
    float input = -1;
    int z = 0;
    int counter = 0;

    do
    {
        printf("The amount of changed owed(in dollars) is ");
        input = get_float();
    }
    while (input < 0);
    input = input * 100;
    input = round(input);
    z = input;

    while (z >= 25)
    {
        z = z - 25;
        counter++;
    }

    while (z >= 10)
    {
        z = z - 10;
        counter++;
    }

    while (z >= 5)
    {
        z = z - 5;
        counter++;
    }

    while (z >= 1)
    {
        z = z-1;
        counter++;
    }

    printf("The number of minimum coins needed is %d\n", counter);
}

In general we want to initialize a variable directly to the value it needs, rather than to a temporary "invalid" value. This reduces complexity, and helps to prevent the accidental use of the "invalid" value as if it were a real input.

We should also put variable declarations as close to the point of use as practical. (e.g. z could be declared at the point of assignment from input). It's best for variables to only exist in the scope in which they are needed.

Note that the input variable is effectively reassigned 3 times, and used to represent 3 different things in the program. If we split the program into separate stages, this becomes clearer:

int main(void)
{
    float input = -1;

    // here "input" is invalid - it's just a placeholder.

    // get user input (dollars):
    {
        do
        {
            printf("The amount of changed owed(in dollars) is ");
            input = get_float();
        }
        while (input < 0);
    }

    // here "input" is the amount of dollars as a float

    // convert input to cents:
    {
        input = input * 100;
        input = round(input);
    }

    // here "input" is the number of cents, as a float

    // calculate number of coins for change:
    {
        int z = input; // note we actually want the number of cents as an int...
        int counter = 0;

        ...

        printf("The number of minimum coins needed is %d\n", counter);
    }
}

It's best to avoid reusing variables like this. Any name given to such a variable is inaccurate or very general (e.g. "input"). Also, when changing some part of the program, we have to understand and modify a much larger amount of code than would otherwise be necessary.

Here, we can avoid reusing the variable by splitting the program up using functions, e.g.:

float get_dollar_input()
{
    while (true)
    {
        printf("The amount of change owed (in dollars) is: ");

        const float dollars = get_float(); // variable initialized to actual value :)

        if (dollars < 0)
        {
            printf("Input must not be negative.");
            continue;
        }

        return dollars;
    }
}

int convert_to_cents(float dollars)
{
    return (int)round(dollars * 100);
}

int calculate_minimal_coins(int cents)
{
    int counter = 0;

    // ...

    return counter;
}

int main(void)
{
    const float dollars = get_dollar_input();
    const int cents = convert_to_cents(dollars);
    const int coins = calculate_minimal_coins(cents);

    printf("The minimum number of coins needed is %d\n", coins);
}

When calculating the change, we do a lot of subtraction in a loop. For a large input (e.g. $200,457,298.46), this could take a looooooong time. We can use division to find the count of each coin, and the remainder (modulus) operator to apply the subtraction:

int calculate_minimal_coins(int cents)
{
    const int quarters = cents / 25;
    cents %= 25;

    const int dimes = cents / 10;
    cents %= 10;

    const int nickels = cents / 5;
    cents %= 5;

    const int pennies = cents; /* unnecessary, but explanatory */

    return quarters + dimes + nickels + pennies;
}

One last thing: We should never use floating point variables to represent exact monetary values. It would be better to ask the user for the number of cents as an integer (or perhaps to ask for two integers: one for dollars, and one for cents).


Faster function iteration on Pandas df with axis=1 I have the following function that allows me to calculate the marketing spend on my advertising campaigns.

The problem is that it's now taking too long (over 15 minutes) to apply the function to said dataframe, as it now has more than 8 months of historical data.

I have no idea if there's a way to speed up the process. Any help would be appreciated if you know a faster way to iter through rows.

def actual_spend_atlas(row):

    filterdatemediaplanid_day = (atlas_df.Date == row.Date) & (atlas_df.MediaPlanIndex == row.MediaPlanIndex)
    filterdatemediaplanid_total = (atlas_df.MediaPlanIndex == row.MediaPlanIndex)

    total_impressions_day = atlas_df.ix[filterdatemediaplanid_day, 'Impressions'].sum()
    total_impressions = atlas_df.ix[filterdatemediaplanid_total, 'Impressions'].sum()

    if row.Amnet == "Amnet":
        spend = atlas_df.AmnetCost * (row.Impressions / filterdatemediaplanid_day)

    if row.CostMethod == 'FLF' or row.CostMethod == 'CPH':

        if total_impressions < 500:
            spend = 0
        else:
            rate = row.BookedRate
            spend = rate * (row.Impressions/ total_impressions)

    elif row.CostMethod == 'CPC':
        spend = row.BookedRate * row.Clicks

    elif row.CostMethod == 'CPM':
        cpm_impressions = row.Impressions / 1000.0
        rate = row.BookedRate
        spend = rate * cpm_impressions

    else:
        spend = 0

    return spend

atlas_df['Spend'] = atlas_df.apply(actual_spend_atlas, axis=1)

The easiest way to get a small speed-up  is swap these two lines:

filterdatemediaplanid_day = (atlas_df.Date == row.Date) & (atlas_df.MediaPlanIndex == row.MediaPlanIndex)
filterdatemediaplanid_total = (atlas_df.MediaPlanIndex == row.MediaPlanIndex)

to:

filterdatemediaplanid_total = (atlas_df.MediaPlanIndex == row.MediaPlanIndex)
filterdatemediaplanid_day = (atlas_df.Date == row.Date) & filterdatemediaplanid_total

This part of the code does not serve any purpose:

if row.Amnet == "Amnet":
    spend = atlas_df.AmnetCost * (row.Impressions / filterdatemediaplanid_day)

Since you don't return right away and afterwards comes an if/elif/else block that is guaranteed to modify spend, anything you calculate here will be overwritten.

While some people adhere to the single exit policy, I prefer early returns:

if row.CostMethod == 'FLF' or row.CostMethod == 'CPH':
    total_impressions = atlas_df.ix[filterdatemediaplanid_total, 'Impressions'].sum()
    if total_impressions >= 500:
        return row.BookedRate * (row.Impressions/ total_impressions)
elif row.CostMethod == 'CPC':
    return row.BookedRate * row.Clicks
elif row.CostMethod == 'CPM':
    return row.BookedRate * row.Impressions / 1000.0
return 0

total_impressions_day is not used anywhere and total_impressions should only be calculated if needed.


Comparing equivalent Scala and C# string-manipulation code - with Scala running much slower I am very early in my journey of learning Scala, and I was doing challenges on hackerrank.com to get used to the syntax. As I finished one (very trivial) challenge, I noticed that the run times seem to be unusually slow.

Thinking that it might be environment-dependent (as the code runs on the HackerRank servers), I decided to quickly reimplement the same code in C#. Interestingly, the code runs 5x faster...

I know that the environment could affect a lot of things, but surely a 5x speed gain for such a trivial operation is strange? They are using Mono for compiling C# code, so both implementations run on Linux. You can get detailed info on their environment here.

The only other idea I have is that, as I am very new to Scala, it is my code which is the problem. So, please take a look at my two implementations below (first in Scala, second in C#) for this problem.

Scala implementation:

object Solution {
    def main(args: Array[String]) {
        println(if (readLine.toLowerCase.filter(x => x >= 'a' && x <= 'z').distinct.length == 26) "pangram" else "not pangram")
    }
}

C# implementation:

using System;
using System.Collections.Generic;
using System.IO;
using System.Linq;
class Solution {
    static void Main(String[] args) {
        string x = System.Console.ReadLine();
        System.Console.WriteLine((x.ToLowerInvariant().ToCharArray().Where(z => z >= 'a' && z <= 'z').Distinct().Count() == 26) ? "pangram" : "not pangram");
    }
}

and let me know how it is possible that the Scala implementation runs that much slower...

I can post screenshots of the run results if needed, but to give an idea - most C# test cases finish in 0.1s and Scala cases take 0.5s.

Can it be the startup time of the runtime? Or do I surrender to the idea that Scala is really that much slower by nature?

The timings were returned by HackerRank's environment (so I realize, of course, that this is nowhere near a benchmark - but nevertheless 5x is crazy).

Scala results:

C# results:

There is no reason for Scala code to be slower than C#.
The different is probably the time that it take to the JVM to load.
To verify that I suggest you to write simple code in C#, Scala & Java which just print "Hello World" and see the different.  My guess is that the Java & Scala will take the same time while the C# will be faster.
Then just do something which do long calculations and see the different again.


Lots of click handlers that hide one element and show another I've a lot of code blocks that hide/show some block.

$('#Back').on('click', function () {
    $('#FormRow').hide();
    $('#TableRow').show();
});

$('#AddLanguageShow').on('click', function () {
    $('#FormRow').show();
    $("#TableRow").hide();
});

$('table').on('click', 'button.edit', function (evt) {
    $('#FormRow').show();
    $('#TableRow').hide();
    languageManager.load(getRowId($(this)));
});

Is it possible to split logic of changing blocks visibility ?

Extract the duplicated code to a function:

function showOneHideOther(elToShow, elToHide) {
    $(elToShow).show();
    $(elToHide).hide();
}

You can reuse the function anyplace now:

$('#Back').on('click', function() {
    showOneHideOther('#TableRow', '#FormRow')
});
$('#AddLanguageShow').on('click', function() {
    showOneHideOther('#FormRow', '#TableRow')
});

$('table').on('click', 'button.edit', function (evt) {
    showOneHideOther('#FormRow', '#TableRow')
    languageManager.load(getRowId($(this)));
});


Ceasar Cipher (Rot 13) I've created a basic cipher program that will turn your string into a Rot13 cipher. I'm pretty sure there are much easier ways to do this, so a little input on what I've done would be great:

What did I do well?
What can be done better?
Are there easier ways to do this?

using System;

namespace Rot13
{
    class CeasarCipher
    {
        static string Encrypt(string toEncrypt, int shift)
        {
            char[] buffer = toEncrypt.ToCharArray();

            for (int i = 0; i < buffer.Length; i++)
            {
                char letter = buffer[i];
                letter = (char)(letter + shift);

                if (letter > 'z')
                {
                    letter = (char)(letter - 26);
                }
                else if (letter < 'a')
                {
                    letter = (char)(letter + 26);
                }
                buffer[i] = letter;
            }
            return new string(buffer);
        }

        static void Main(string[] args)
        {
            Console.Write("Enter string to cipher: ");
            var toCipher = Console.ReadLine();

            Console.WriteLine(Encrypt(toCipher, 13));
        }
    }
}

You could use the modulo operator instead of conditions:

static string Encrypt(string toEncrypt, int shift)
{
    const int alphabetLength = 'z' - 'a' + 1;
    char[] buffer = toEncrypt.ToCharArray();

    for (int i = 0; i < buffer.Length; i++)
    {
        char letter = (char)(buffer[i] - 'a'); // Let the `letter` be an offset from 'a'
        letter = (char)((letter + shift) % alphabetLength);
        buffer[i] = (char)(letter + 'a');
    }
    return new string(buffer);
}

Or almost one-liner:

static string Encrypt(string toEncrypt, int shift)
{
    const int alphabetLength = 'z' - 'a' + 1;
    return new string(Array.ConvertAll(toEncrypt.ToCharArray(),
        c => (char)('a' + (c - 'a' + shift) % alphabetLength)));
}


Detect if linked list contains a loop The following is a code to detect a loop in a linked list. This question is prepared strictly for interview purposes. This code has been modeled as per Linkedlist.java

How should I decide if a function like hasLoop() should be included as an instance method or should be a static method similar to sort in Collections.java?
Ideally I would inherit Linkedlist and add an extra function mergeSort to the subclass, but the interviewer would not like it, thus I should keep my solution without inheriting linkedlist for interview sake.  Do you agree?

    public class DetectLoopExistance<E> {

    private Node<E> first;
    private Node<E> last;

    public void add(E element) {
        final Node<E> l = last;
        final Node<E> newNode = new Node<E>(element, null);
        last = newNode;
        if (first == null) {
            first = newNode;
        } else {
            l.next = newNode;
        }
    }

    private static class Node<E> {
        E element;
        Node<E> next;

        Node(E element, Node<E> next) {
            this.element = element;
            this.next = next;
        }
    }

    public boolean hasLoop() {
        Node<E> fast = first;
        Node<E> slow = first;

        while (fast != null && fast.next != null) {
            fast = fast.next.next;
            slow = slow.next;
            if (slow == fast) {
                return true;
            }
        }
        return false;
    }
   }

To answer your first question, I would create the method as a static method, because similar to sorting, detecting loops is basic functionality, that is applicable to Lists in general, not only a specific List object.

I would not recommend inheriting from the original List implementation in order to create the merge sort functionality. If you do so you will end up with List objects that can be sorted using merge sort and List objects, that cannot. But there is not really a difference between the two in structure, the only difference is that one is you inherited version and one is the LinkedList implementation. This complicates things unnecessarily. Again I would create a static method to merge sort any List, which would add the ability to merge sort any LinkedList, any ArrayList and any other custom List implementation, that implements the Java List interface.


Returning Function Calls from an Array within a JavaScript Function I'd like to use JavaScript to generate HTML.

I've made a JavaScript function object that has two parameters; a string and generic object. The string is used to specify the type of HTML element you want to generate and the generic object is a container for key value pairs specifying the element's attributes -- or a string for simple elements like paragraphs containing only text that needs no id or class specification.

I'm drafted some functions that generate some HTML elements. I then contained the functions in an array that is a local variable of a function called html.

When html is called with appropriate arguments (e.g. html("element", {id:"myElement"})), a string of HTML text is returned.

I wonder what the drawbacks of the approach are. More specifically, is there a better method to achieve the same results?

var html = function (type, spec={}) {
  html_element = [];

  html_element["table"] = function() {
    markup = "
";

    if (spec.cols)
    {
      markup += "
";

      for (value in spec.cols)
        markup += "" + spec.cols[value] + "	";

      markup += "
";
    }

    while (spec.rows.length)
    {
      markup += "
";

      for (data in (row = spec.rows.shift()))
        markup += "" + row[data] + "	";

      markup += "
";
    }

    return markup += "
";
  };

  html_element["button"] = function() {
    if (spec.hasOwnProperty("value")) value = spec.value;
    else                              value = "";

    return "<button id=\"" + id + "\">" + value + "</button>";
  };

  html_element["p"] = function() {
    if (typeof spec === Object) text = spec.text;
    else                        text = spec;

    return "
" + text + "
";
  };

  ...

  return html_element[type]();

}

Normally, is a good practice create sub functions instead of add all on an array.

var button = function() {

};

Always use var in front of the variables to scope to the context. (good practice).

A good choice is take advantage of the evaluation of data types on javascript, for variables.

var variable = paramValue || "defaultValue";

this evaluates paramaValue, if is true, is assigned, if not, assign "defaultValue"

Also, if is possible, considere use a template engine like:

http://handlebarsjs.com
http://mustache.github.io

Final Code


How to optimize the pagination query? This is completely optimization question, I have a pagination query like this:

$this->paginate    = array(
        'fields'    =>  array(
                        'DISTINCT Contact.contact_id',
                        'Contact.first_name',
                        'Contact.last_name',
                        'Contact.email',
                        'Contact.created',
                        'ContactGroup.name',
                      ),
        'conditions' => array(
                        $this->conditions,
                        'ContactsContactGroup.contact_group_id'=>$viewList,
                        isset($keywordQuery)?$keywordQuery:"",
                      ),
        'limit'      => 5,
         'group'     => array('Contact.contact_id')
                      );
$data = $this->paginate('ContactsContactGroup');
$data = $this->paginate('ContactsContactGroup');

This query is called in every if and else statement, I have four conditions of if and else, and in all conditions the above piece of code is written.

I want to optimize, and avoid the big line of code in every condition. How can I optimize it? Any answer will be appreciated.

Perhaps you could wrap it all in a function:

function pagination($this,$viewList,$keywordQuery)
{
    $this->paginate    = array(
        'fields'    =>  array(
                        'DISTINCT Contact.contact_id',
                        'Contact.first_name',
                        'Contact.last_name',
                        'Contact.email',
                        'Contact.created',
                        'ContactGroup.name',
                      ),
        'conditions' => array(
                        $this->conditions,
                        'ContactsContactGroup.contact_group_id'=>$viewList,
                        isset($keywordQuery)?$keywordQuery:"",
                      ),
        'limit'      => 5,
         'group'     => array('Contact.contact_id')
                      );

   return $this->paginate('ContactsContactGroup');

}

if(something())
{
    //something
    $data=pagination($this,$viewList,$keywordQuery);
}
else
{
    //other
    $data=pagination($that,$viewList,$keywordQuery);
}

etc..


Scrabble word scoring I have a Scrabble game program which takes two players' words and calculates who has the winning score.
Whichever player has the highest score is the winner; there's a tie if the scores are the same.
It all seems to work how I have written it; however I am sure it can be done better. Just wondering if anyone could give me some ideas on a more efficient way to write this?
#include <ctype.h>
#include <cs50.h>
#include <stdio.h>
#include <string.h>

// Points assigned to each letter of the alphabet
int points[] = {1, 3, 3, 2, 1, 4, 2, 4, 1, 8, 5, 1, 3, 1, 1, 3, 10, 1, 1, 1, 1, 4, 4, 8, 4, 10};

int compute_score(string word);

int main(void)
{

    // Get input words from both players
    string word1 = get_string("Player 1: ");
    string word2 = get_string("Player 2: ");

    // Score both words
    int score1 = compute_score(word1);
    int score2 = compute_score(word2);

    // TODO: Print the winner
    if (score1 < score2)
    {
        printf("Player 2 wins!...");
    }
    else if (score1 == score2)
    {
        printf("Tie!");
    }
    else
    {
        printf("Player 1 wins!...");
    }
}

int compute_score(string word)
{
    // TODO: Compute and return score for string
char letters[26];
int asciiletter = 97;
int score = 0;
int i = 0;
int k = 0;

    for (i = 0; i < 26; i++)
    {
        letters[i] = asciiletter + i;

    }

    //Get the chars in the string of word and compare to all letters in alphabet letters[26]
    for(int j= 0, n = strlen(word); j < n; j++)
    {
        //if entered values uppercase, convert to lower as points are the same for either case
        word[j] = tolower(word[j]);
        for(k = 0; k < 26; k++ )
        {
                if (word[j] == letters[k])
                {
                    score = score + points[k];
                }
        }

    }
    return score;
}

and welcome to C programming! Also, to Code Review.
Style
There are a couple of "style" issues with your code. The most obvious one is that your indentation is not consistent. I don't know if that's due to pasting it into the browser, or if it appears that way in your code. But computescore needs to be cleaned up.
Next is an issue of "comprehensibility". You have this array:
// Points assigned to each letter of the alphabet
int points[] = {1, 3, 3, 2, 1, 4, 2, 4, 1, 8, 5, 1, 3, 1, 1, 3, 10, 1, 1, 1, 1, 4, 4, 8, 4, 10};

Quick, how many points for 'S'?
This array might possibly be correct, but it's not comprehensible and it's not modifiable. It would be much better to provide some other mechanism to initialize the array, or initialize some other structure, such that the next guy to maintain the code has a chance of seeing bugs and/or being able to make changes.
For example:

You might create an enum, with point values for each letter, then initialize the array using the enum labels:
enum {
    A_POINTS = 1,
    B_POINTS = 3,
    C_POINTS = 3,
    :
};

int points[] = {
    A_POINTS,
    B_POINTS,
    C_POINTS,
    :
};

You might create a clever designated initializer macro that makes things more clear:
#define TILE(CH) [CH - 'a']

int points[] = {
    TILE('a') = 1,
    TILE('b') = 3,
    TILE('c') = 3,
    :
};

You might consider building a string and parsing it at runtime, like:
const char * points =
     "A = 1; B = 3; C = 3; ...;"
     "N = ..."
     ;

Be aware: the point of this is purely to make it easy to understand the mapping between letters and points. Ideally, you want to make it easy enough to understand that someone could look at the code and catch a mistake before you compiled it and went to testing.
Magic
Your scoring function searches through a list of characters in order to obtain the index of a particular character. Never do that! You know the index, it's just hidden behind a thin layer of mathematics.
In ASCII, the upper and lowercase letters (also, digits) are runs of encoding values assigned in consecutive, increasing order. Once you know where the start of the run is, you can get to any other character by simple addition. Likewise, you can reverse that trick using simple subtraction:
    score_index = ch - 'a';
    score += points[score_index];


Range of all variables in List in Prolog I have a list in Prolog like this:

Puzzle = [ A1, A2, A3,
           B1, B2, B3,
           C1, C2, C3 ].

And I want every variable in the list to be a number in the range of 1 to 9.
Currently I'm doing this like so:

between(1,9, A1),
between(1,9, A2),
% ...
between(1,9, C3).

Even though this works, it's already pretty ugly even for the only 9 numbers I have so far. However I will have to expand my list to contain at least 81 numbers at first (A1 till I9), and even 405 in the end.

I would rather not 'write' 405 lines of code just to limit the range of the numbers in the list, but since I'm new to Prolog, I don't know how I can improve this.

Once you will have to hardcode your list to include all the variables from A1 to I9. Then you can iterate through it:

foreach(X, Your_list) do between(1, 9, X).


Find max number you can create from an array of numbers Was asked this in an interview yesterday:

You are given an array of numbers (not digits, but numbers: e.g. 9, 23, 184, 102, etc.) - you need to construct the largest number from it. For example: you get 21, 2, 10 - the largest number is 22110.

This is my solution, I wonder if you can improve this?

def maximum_number(a):
    work_dict = {
        0: 0,
        1: 0,
        2: 0,
        3: 0,
        4: 0,
        5: 0,
        6: 0,
        7: 0,
        8: 0,
        9: 0
    }

    for ai in a:
        # if ai is a single digit number, just add it
        if ai in work_dict:
            work_dict[ai] += 1
            continue

        # otherwise, decompose it to it's digits
        while ai > 0:
            number = ai % 10
            work_dict[number] = work_dict[number] + 1
            ai = int(ai/10)

    max_num = int('9'*work_dict[9] + '8'*work_dict[8] + '7'*work_dict[7] + '6'*work_dict[6] +
                    '5'*work_dict[5] + '4'*work_dict[4] + '3'*work_dict[3] + '2'*work_dict[2] +
                    '1'* work_dict[1] + '0'*work_dict[0])

    return max_num

Your code

Your code as such seems to be functional, but not really elegant or concise.

First, the variable names don't speak for themselves. Nobody would be hurt if the function input was named numbers instead of a and number instead of ai. work_dict is also  not a particularly good name since it's very generic. How about digit_histogram?

Handling single digit numbers separately seems unnecessary. The algorithm you implemented can handle them without special treatment.

When constructing max_num, there is a lot of repeated code. You could simplify this using a list comprehension and join (more on that soon).

How I would have tackled this

Since we have the luxury that combination of these numbers should be maximized in base 10, we can get their digits simply by looking at their str representation (which coincidentally happens to be in base 10 ;-) )1.

If you include the other recommendations from above you end up with:

def maximum_number_str(arr):
    digit_histogram = {
        "0": 0, "1": 0, "2": 0, "3": 0, "4": 0,
        "5": 0, "6": 0, "7": 0, "8": 0, "9": 0
    }
    # or: digit_histogram = {str(i): 0 for i in range(10)}

    for number in arr:
        for digit in str(number):
            digit_histogram[digit] += 1

    max_num = "".join(str(i)*digit_histogram[str(i)] for i in reversed(range(10)))

    return int(max_num)

Depending on how familiar you are with Python and if other modules are allowed, you could come up with a solution using collections.Counter, or at least skip the dict initialization all-together if you use .get(...) instead of [...] when accessing the dictionary as presented by Pål GD in is answer.

Just for reference, this is how it could look like using a Counter:

from collections import Counter

def maximum_number_counter(arr):
    digit_histogram = Counter()

    for number in arr:
        digit_histogram.update(str(number))

    max_num = "".join(
        str(i) * digit_histogram[str(i)] for i in reversed(range(10)))

    return int(max_num)

Edit: The other way to think about that task

There seems to be a vivid discussion here if you understood the task correctly. If you follow the arguments that speak against your and my former interpretation, this actually leads to another interesting problem.

I came up with the solution below, though I highly doubt that I could have come up with this in an interview situation.

from functools import cmp_to_key

def maximize_joint_number(number1, number2):
    joined12 = int(str(number1)+str(number2))
    joined21 = int(str(number2)+str(number1))
    return joined21 - joined12

def maximum_number(numbers):
    """
    Generate the largest possible number that can be generated rearanging the
    *numbers*, not the digits of the input sequence
    """
    return int("".join(str(i) for i in sorted(numbers, key=cmp_to_key(maximize_joint_number))))

The idea to this is actually from this blog post that was given in a comment by Eric Duminil. The cmp_to_key trickery is needed because the cmp keyword was removed from sort in Python 3. You could also use cmp_to_key as a decorator, which makes it a little bit nicer:

from functools import cmp_to_key

@cmp_to_key
def maximize_joint_number(number1, number2):
    ...

def maximum_number(numbers):
    return int("".join(str(i) for i in sorted(numbers, key=maximize_joint_number)))

A quick test seems to fulfill all the presented example outputs:

if __name__ == "__main__":
    assert maximum_number([0, 12]) == 120
    assert maximum_number([2, 21, 10]) == 22110
    assert maximum_number([9, 2, 5, 51]) == 95512
    assert maximum_number([20, 210, 32]) == 3221020
    assert maximum_number([1, 19, 93, 44, 2885, 83, 379, 3928]) == 93834439283792885191

The second and third test case break implementations that would try to use something like sorted(numbers, key=str, reverse=True) (lexicographical sort) directly.

1 Thanks to Peter Cordes for pointing out the inaccurate wording here in earlier revisions.


Get argument as unicode string from argparse in Python 2 and 3 Code from my last project, that has to work on Python 2.7 and Python 3.
I want to get a path as a unicode string from argparse:

parser = argparse.ArgumentParser(
    description='Organize your movie collection using symbolic links')
parser.add_argument('media_src', metavar='FILE|DIR',
                    help='Media file or directory')
args = vars(parser.parse_args(args=argv[1:]))
try:
    args['media_src'] = unicode(args['media_src'], "utf-8",
                                errors="ignore")
except NameError:
    pass  # py3

Can that be done more elegantly without relying on duck typing?

You can define unicode for Python3 when you get a NameError.
You should make a small note that explains unicode is not defined in Python3, rather than having the user guess, what #py3 means.

try:
    unicode
except NameError:
    # Define `unicode` for Python3
    def unicode(s, *_):
        return s

You should use the type parameter to add_argument, to change the type returned.
And so if you don't want to have unicode in the global scope in your Python3 environment you can bind it to that functions scope.

def build_to_unicode():
    try:
        unicode
    except NameError:
        def unicode(s, *_):
            return s

    def inner(s):
        return unicode(s, "utf-8")

    return inner

to_unicode = build_to_unicode()

Also your lack of a definitive style to pass variables to the parser made it harder for me to read your code.

try:
    unicode
except NameError:
    # Define `unicode` for Python3
    def unicode(s, *_):
        return s

def to_unicode(s):
    return unicode(s, "utf-8")

parser = argparse.ArgumentParser(
    description='Organize your movie collection using symbolic links')
parser.add_argument(
    'media_src',
    metavar='FILE|DIR',
    help='Media file or directory',
    type=to_unicode)

args = parser.parse_args(args=argv[1:])
# If you really want to.
args = vars(args)


Pick a folder and consolidate workbooks I'm trying to create a simple way for current & future (non VBA literate) employees to accomplish some of the tasks I've struggled with. The first being Parsing cells containing Line Feed Characters.

When reports are produced from Workday, sometimes they have to be produced for each item separately, and you end up with a folder like this -

Within each book is a sheet of the same name, but cut off at 31 characters (the maximum for sheet names).

I'd like to enable the user to simply select the folder containing the books and consolidate, rename and alphabetize the sheets.

All in the same module (the CONST) used later, main sub:

Option Explicit
Public Const COMPUTER As String = "C:\"

Public Sub CombineWorkbooks()

    Dim myPath As String
    myPath = UserInputDirectory(COMPUTER)
    If myPath = vbNullString Then
        MsgBox "You cancelled the operation", vbCritical
        Exit Sub
    End If

    Application.DisplayAlerts = False
    Application.EnableEvents = False
    Application.ScreenUpdating = False

    Dim destinationWorkbook As Workbook
    Set destinationWorkbook = Workbooks.Add
    destinationWorkbook.Sheets("Sheet2").Delete
    destinationWorkbook.Sheets("Sheet3").Delete

    Dim sourceWorkbook As Workbook
    Dim sourceWorksheet As Worksheet

    Dim strFilename As String
    strFilename = Dir(myPath & "\*.xls", vbNormal)
    If Len(strFilename) = 0 Then Exit Sub

    Do Until strFilename = ""
        Set sourceWorkbook = Workbooks.Open(Filename:=myPath & "\" & strFilename)
        Set sourceWorksheet = sourceWorkbook.Worksheets(1)
        RenameSheet sourceWorksheet
        sourceWorksheet.Copy After:=destinationWorkbook.Worksheets(destinationWorkbook.Worksheets.Count)
        sourceWorkbook.Close False
        strFilename = Dir()
    Loop
    destinationWorkbook.Sheets("Sheet1").Delete

    Application.DisplayAlerts = True
    Application.EnableEvents = True

    AlphebetizeSheets destinationWorkbook
End Sub

And the supporting cast

Private Sub AlphebetizeSheets(ByVal myBook As Workbook)
    Dim i As Long
    Dim j As Long

    For i = 1 To myBook.Worksheets.Count
        For j = i To myBook.Worksheets.Count
            If UCase(myBook.Sheets(j).Name) < UCase(myBook.Sheets(i).Name) Then
                  myBook.Sheets(j).Move before:=Sheets(i)
             End If
        Next
    Next
    Application.ScreenUpdating = True
    Application.GetSaveAsFilename
End Sub

Private Sub RenameSheet(ByVal targetSheet As Worksheet)
    Dim myParser As Long
    Dim newName As String
    myParser = InStr(1, targetSheet.Name, "(")
    If myParser > 0 Then
        newName = Trim(Left(targetSheet.Name, myParser - 1))
        targetSheet.Name = newName
    End If
End Sub

Private Function UserInputDirectory(ByVal baseDirectory As String) As String
    Dim userFolder As FileDialog
    Dim directoryName As String
    MsgBox "Please select the folder the workbooks are in"
    Set userFolder = Application.FileDialog(msoFileDialogFolderPicker)
    With userFolder
        .Title = "Select a Folder"
        .AllowMultiSelect = False
        .InitialFileName = baseDirectory
        If .Show <> -1 Then GoTo FolderPicked
        directoryName = .SelectedItems(1)
    End With
FolderPicked:
    UserInputDirectory = directoryName
    Set userFolder = Nothing
End Function

I'm sort of worried that there's too much going on in the CombineWorkbooks() sub and that I've missed some errors that may need to be handled.

You've got a spelling mistake; it's "Alphabetize".

Private Sub AlphebetizeSheets(ByVal myBook As Workbook)
    Dim i As Long
    Dim j As Long

    For i = 1 To myBook.Worksheets.Count
        For j = i To myBook.Worksheets.Count
            If UCase(myBook.Sheets(j).Name) < UCase(myBook.Sheets(i).Name) Then
                  myBook.Sheets(j).Move before:=Sheets(i)
             End If
        Next
    Next
    Application.ScreenUpdating = True
    Application.GetSaveAsFilename
End Sub

I thought this is the sort of thing that has been made before...

https://msdn.microsoft.com/en-us/library/office/ff195025.aspx

And it has. But they do it the same way, with two small differences:

They don't compare the sheets with themselves via starting the second for loop at i+1
They don't check the last sheet by stopping i at myBook.Worksheets.Count - 1

Which I guess saves a few comparisons. You've got a weird sorting algorithm here too, a version of selection and insertion sort... which seems to have some flaws, in my opinion.

  8 3 2 6 4 5 1 7
  checks 8, 3, moves 3
  3 8 2 6 4 5 1 7
  checks 2, moves 2
  2 3 8 6 4 5 1 7
  checks 6, 4, 5, 1 moves 1
  1 2 3 8 6 4 5 7
  checks 7, next iteration
  compares 2 to 3, 8, 6, 4, 5, 7, next iteration
  compares 3 to 8, 6, 4, 5, 7, next iteration
  compares 8 to 6, moves 6
  1 2 3 6 8 4 5 7
  compares 6 to 8, 4 moves 4
  1 2 3 4 6 8 5 7
  compares 4 to 5, 7, next iteration
  compares 6 to 8, 5 moves 5
  1 2 3 4 5 6 8 7
  compares 5 to 7, next iteration
  compares 6 to 8, 7, next iteration
  compares 8 to 7, moves 7
  1 2 3 4 5 6 7 8, next iteration and end.

The problem with this sort is that it compares items[i] with items[i+1] even although on the previous iteration, multiple shifts happened. Take a look at the start:

  8 3 2 6 4 5 1 7
  checks 8, 3, moves 3
  3 8 2 6 4 5 1 7
  checks 2, moves 2
  2 3 8 6 4 5 1 7
  checks 6, 4, 5, 1 moves 1
  1 2 3 8 6 4 5 7
  checks 7, next iteration

After that, it should know that 2 > 1, 3 > 2, and 8 > 3 so it should skip those.

So instead of

  compares 2 to 3, 8, 6, 4, 5, 7, next iteration

it should be

  compares 2 to 6, 4, 5, 7, next iteration.

and

  compares 3 to 6, 4, 5, 7, next iteration.

You could do this by incrementing another variable for every move operation, and decrementing this variable after the j-for loop (with a minimum of 0). Start j at i + this other variable. Like that, it would, after the 3 moves in the first iteration, start the second iteration off at index 4 (i = 2, move counter = 3 - 1 iteration passed = 2).

Alternatively, accept that sorting sheets is likely to be fast already, and don't spend time on this beyond adding the two simple improvements. Maybe store UCase(myBook.Sheets(j).Name) and UCase(myBook.Sheets(i).Name) in temporary variables.


Efficiently return a ChatColor I am working on a plugin for a Minecraft server. In it, I have a function that returns a ChatColor depending on what permission level you are.

Whenever I need reference the user with proper colors, I always get a color from the below function.

/**
 * ChatColor
 *
 * @param Player
 * @return ChatColor
 */
public ChatColor getColor(Player p) {
    // By default the color is Guest. The color will stay at guest otherwise.
    ChatColor color = ChatColor.GRAY;
    if (p.hasPermission("level.owner")) {
        // Owner
        color = ChatColor.RED;
    } else if (p.hasPermission("level.staff")) {
        // Admin / Mod
        color = ChatColor.AQUA;
    } else if (p.hasPermission("level.builder")) {
        // Builder
        color = ChatColor.DARK_AQUA;
    } else if (p.hasPermission("level.member")) {
        // Member
        color = ChatColor.GOLD;
    } else if (p.hasPermission("level.guest")) {
        // Guest
        // We don't need to change the color, since the color defaults to gray.
    }
    return color;
}

One might call this function as below:

p.sendMessage("Welcome back, " + getColor(p) + p.getName() + gold + "!");

And it would send a message to the player like "Welcome back, (playerName)!" with playerName the level's color.

What I have found is that if you have the first permission level (level.owner) it can take a much longer time. After putting in some debugging (e.g getting a NanoTime, running my function, and subtracting the difference) I've found the following:

With level.owner it takes about 4580ns to run
With level.staff it takes about 6340ns to run
With level.builder it takes about 8000ns to run
With level.member it takes about 7630ns (sometimes going as high as 10240ns) to run
With level.guest it takes about 10000ns+ to run

Is there any way to improve my function run at nearly the same speed for all of my checks?

(I am a beginner at Java, but know many other languages, so this is somewhat new to me.)

I'm guessing that you call this method frequently.  If so, you might consider memoizing the result.  Something like

private Map<Player, ChatColor> playerChatColors = new HashMap<>();

in the class.  Possibly make this static.  And then at the beginning of the method, replace

    ChatColor color = ChatColor.GRAY;

with

    ChatColor color = playerChatColors.get(p);
    if (color != null) {
        return color;
    }
    color = ChatColor.GRAY;

And then before the return at the end, add something like

    playerChatColors.put(p, color);

I don't know if this is faster or not.  In the typical case, it will just do a single hash access.  If that is fast, then it's faster.  If that's slow, then it might not be.

There's also the problem that if the permissions change, this won't adjust.   I'm assuming that this will be rare enough not to matter.  If that's not so, you might need to find a way to invalidate the caching when permissions change.  Of course, if you invalidate too often, you lose the benefits of caching.  The first call to this method will be slower.  It's the subsequent calls that this might help.


Is it a good idea to keep Request querystring as a property? My code snippet is like this:

public static string EventId
{
    get { return HttpContext.Current.Request["eventId"]; }
}

And I will call this property EventId whenever I need it. Is it a good practice?

Can you guarantee that eventId will never change? If so it might not be a bad idea, but this comes down to a few things

eventId never changes;
how and where you're using it. It might be somewhat difficult for a new programmer in the project to locate and realize what it actually does (it returns the eventId for the current Http Request), solely based on the property name; and
Are you sure that both HttpContext.Current and Current.Request are initialized before retrieving eventId. There is a change these will throw a null reference exception. It might be a very good idea to do a simple if != null check before invoking the Request property.

If your code can accommodate for these points I don't see any inherently wrong with doing this.


Finding palindromes and the longest word in a string I have two little projects that I've been working on.

One is to find all palindromes:

function palindrome(str) {
  var replaceStrChr = str.replace(/\W/g,'').replace(/ /g,'').replace(/_/g,'').toLowerCase();
  var arrayifyStr = replaceStrChr.split('').reverse();
  var stringifyStr = arrayifyStr.join('');

  if (replaceStrChr === stringifyStr){
    return true;
  } else {
    return false;
  }
}

And the other one is to find the longest word in a string:

function findLongestWord(str){
    var arrayify = str.split(' ');
    var longest = 0;

    for(var x = 0; x < arrayify.length; x++){
        if (arrayify[x].length > longest){
            longest = arrayify[x].length;
        }
    }
    return longest;
}

What I'm looking for is a better way to do these, shortcuts, better syntax, etc.

For the palindrome:

You can probably just use one replace operation, but with a more extensive regex. My regex is rusty but something like /[\W_\s]/g could do.
split, reverse and join could possibly be just one chain.

var reversedString = replaceStrChr.split('').reverse().join('');

Comparisons are essentially boolean values. You can simply do:

return replaceStrChr === stringifyStr;

For the longest word:

Use reduce and carry through the longest word.

var longestWord = str.split(' ').reduce(function(prev, next){
  return next.length > prev.length ? next : prev;
});

str doesn't tell me what the string contains. Only when I saw split(' ') did I realize it was a space-separated string. Rename the variable to show its purpose.


Simplifying a class template I wrote this code over night for a little project that my friend and I have been working on.  This class template was created over a year ago during as a class assignment for C++ programming.  I recently reviewed this program and I saw that I created multiple instances of my classes in order to print out different data types in the output.

This is not efficient coding at all and I know it can be prone to bugs.  However, would anyone consider making abstract classes instead?  Please given me an opinion on how I should change this code in order for it to be efficient and reduce the creation of the same class.

#include <iostream>
#include <string>
#include <stdlib.h>

using namespace std;

template<class T>

class BinaryTree
{
    struct Node
    {
        T data;
        Node* lChildptr;
        Node* rChildptr;

        Node(T dataNew)
        {
            data = dataNew;
            lChildptr = NULL;
            rChildptr = NULL;
        }
    };
    private:

    Node* root;

    void Insert(Node* newData, Node* &theRoot)
    {
        if(theRoot == NULL)
        {
            theRoot = newData;
            return;
        }
        else if(newData->data < theRoot->data)
            Insert(newData, theRoot->lChildptr);
        else
            Insert(newData, theRoot->rChildptr);;
    }

    void insertnode(T item)
    {
        Node *newData;
        newData= new Node(item);

        Insert ( newData, root);
    }

    void PrintTree(Node* &theRoot)
    {
        if(theRoot)
        {
            cout << theRoot->data << endl;
            PrintTree(theRoot->lChildptr); //<< The error is here
            PrintTree(theRoot->rChildptr);
        }
    }

    public:
    BinaryTree()
    {
        root = NULL;
    }

    void AddItem(T item)
    {
        insertnode(item);
    }

    void PrintTree()
    {
        PrintTree(root);
    }
};

int main()
{
    BinaryTree<string> *tree = new BinaryTree<string>();
    BinaryTree<int> *myBT = new BinaryTree<int>();
    BinaryTree<double> *mydouble = new BinaryTree<double>();
    BinaryTree<char> *mychar = new BinaryTree<char>();

    tree->AddItem("Erick");
    myBT->AddItem(7);
    mydouble->AddItem(9.99999);
    mychar->AddItem('@');

    tree->PrintTree();
    myBT->PrintTree();
    mydouble->PrintTree();
    mychar->PrintTree();

    system ( "pause");
}

A few main points you should attempt to improve in this code:

Start using nullptr if you can (C++11). It is much better than the old NULL macro.
Never "use namespace" in a header file (Not sure if this is a header file? Doesn't seem like...). That would leak the imported declarations to every other place that included the header. Also be conservative about using namespace in a cpp file. Prefer to explicitly declare the things your are using (e.g.: using std::cout;) or don't do any of that and prefix everything with std::. It is not much more typing anyways. Further discussion here.
Properly initialize data in constructors by calling the constructors of the sub-objects:

Node(T dataNew)
    : data(dataNew)   // Better would be 'data(std::move(dataNew))' if C++11
    , lChildptr(NULL) // Better 'nullptr' if C++11
    , rChildptr(NULL) // Same here
{ }

#include <stdlib.h> is the C header file. For C++, use <cstdlib>.
Why this:

   Node *newData;
   newData= new Node(item);

Instead of just: Node * newData = new Node(item);?
PrintTree() would be more flexible if it took the output stream as a parameter. Also, consider providing the stream output operator << instead. It is the most idiomatic way of printing an object in C++.
Mind Const Correctness. PrintTree() is not mutating any member variables, so it should be const.
Your new allocated instances are never being freed and are leaking in your test code. If you are going to use dynamic memory, make sure to first attempt a smart pointer or a standard container. This will make your life a lot easier and your code a lot more bug-free. Side note: These local variables you've declared in main() have no reason for being pointers. They could very well be declared by value in the program stack.
A little inconsistency with naming: insertnode() is not following your PascalCase notation. As a side note, camelCase is more popular for method names (except for Windows-centric projects perhaps).


Making events not trigger so often on a datatable with dc.js I used this answer to make a table with datatables.net and dc.js.  However, I detected a performance issue in this loop:

function RefreshTable() {
    dc.events.trigger(function () {
        datatable.api()
        .clear()
        .rows.add( YearDimension.top(Infinity) )
        .draw() ;
    });
}

count = dc.chartRegistry.list().length
for (var i=count; i--;) {
    dc.chartRegistry.list()[i].on("filtered", RefreshTable);
}
RefreshTable() ;

Because when resetting all the filters with

dc.filterAll();dc.redrawAll();

RefreshTable() is executed once for every dc chart (or dimension?). In my case it's 5 loops at ~500ms each, thus more than 2s for 4 charts and 10k records.

Has someone any idea on how to make it faster? The table should be refreshed only once.

What is ineficient here is always running RefreshTable 5 times. Even if no filters where applied to any charts.

Make a new function that only resets filters from charts that have a filter.

function filterAll_performance(){
    var chartlist=[chart_year, chart_genre, chart_runtime, chart_globalscore]
    count=chartlist.length;
    for (var i=count; i--;) {
        if(chartlist[i].filters()!=0) {
            chartlist[i].filterAll();
        }
    }
}

and replace

dc.filterAll();

with

filterAll_performance();

This will still refresh the datatable more than once sometimes but it's still better.


Rock, Paper, Scissors in Python I was eventually going to add another computer and more options to this (i.e., Rock-paper-scissors-lizard-Spock) but I wanted to make sure I was using the best practice for this type of game. Any help would be appreciated.

# The traditional paper scissors rock game

import os
def clear():
    os.system("clear")
clear()
print ("\n\nPaper, Rock, Scissors Game -(Best of five games)")
x = 0 ;  l = 0 ;  w = 0 ; d = 0 ; lt = 0 ; wt = 0 ; dt = 0
while x < 5:
  x = x + 1
  import random
  class Computer:
         pass
  comp_is = Computer()
  comp_is.opt = ('r','p','s')
  comp_is.rand = random.choice(comp_is.opt)

  if comp_is.rand == 'r':
            comp  = 'rock'
  elif comp_is.rand == 'p':
            comp  = 'paper'
  else:
        comp  = 'scissors'

  class Human:
       pass
  human_is = Human
  print
  human_is.player = raw_input(' Enter your choice of\n   r\
 for rock\n   p for paper or\n   s for scissors ... ')
  print

  class Result:
     pass
  Result_is = Result
  if comp_is.rand == human_is.player:
    print ("draw - computer chose ",  comp)
    print
    d = d + 1
    dt = dt + 1
  elif comp_is.rand == 'r' and human_is.player == 'p':
      print ("  player beats computer -computer chose ",  comp)

      print
      w = w + 1
      wt = wt + 1
  elif comp_is.rand == 'p' and human_is.player == 's':
      print ("  computer chose ",  comp)
      print ("  player beats computer-because scissors cuts paper")
      print ()
      w = w + 1
      wt = wt + 1
  elif comp_is.rand == 's' and human_is.player == 'r':
     print (" computer chose ", comp)
     print (" player beats computer-because rock breaks scissors")
     w = w + 1
     wt = wt + 1
  else :
     print ("   computer wins - computer chose  ", comp)
     print
     l = l + 1
     lt = lt + 1

  if x == 5:
    print ()
    print ()
    print ("  games  won ... ",  w)
    print ("  games lost ... ",  l)
    print ("  games drawn ... ",  d)
    print ()
    print ("  Running total overall of games won ... ", wt)
    print ("  Running total overall of games lost ... ", lt)
    print ("  Running total overall of games drawn ... ", dt)
    print ()
    w = 0 ; l = 0 ; d = 0
    again = input('Do you want to play again y for yes, n for no ..  ')
    if again == 'y':
       x = 0
    else:
      print
      if lt > wt:
         print ("You are a miserable loser,\nYou have lost more than you have won,\nPoor show indeed ")
         print ('finish')

There is much to improve. I recommend you read PEP 8, the official Python style guide. It includes many important tips like:

Use consistent 4-space indentation
Multiple statements on the same line are discouraged

Furthermore:

You should use words instead of single letters for your variable names. All those variables like x, l, w, d, lt, wt, dt aren't self-explaining. What is their purpose? Instead: count_rounds, count_losses, count_wins, count_draws, ….
Your usage of object oriented features is extremely weird. Get rid of all classes for now.
Instead of x = x + 1 write x += 1.
Your code is really complicated because it munges together the user interface (prompting the user for choices, displaying results) with the actual logic of your program. Put the logic into separate functions, e.g.

def beats(choice_a, choice_b):
    if choice_a == 'rock' and choice_b == 'scissors':
        return 'smashes'
    if choice_a == 'scissors' and choice_b == 'paper':
        return 'cuts'
    if choice_a == 'paper' and choice_b == 'rock':
        return 'wraps'
    else:
        return None

This could be used as

def result_string(computer, player):
    verb = beats(computer, player)
    if verb:
        return "computer beats player because %s %s %s" % (computer, verb, player)
    verb = beats(player, computer)
    if verb:
        return "player beats computer because %s %s %s" % (player, verb, computer)
    return "draw"

which in turn could be used as print(result_string(computer_choice, player_choice)).

Please try to fix these issues and to clean up your code, then come back and ask a new question for a second round of review.


Number guessing game with statistics I am new to programming, and I wanted to ask, is my code inefficient / poorly written?

Here is my code below for an exercise I just did. The point is for the user to guess the number 0, but it also:

Calculates the average number
Calculates the number of attempts
Gives the sum of attempts

Are there ways of improving this code? If so, how do you know where to look? Am I overusing if/while statements?

#User must guess a number: until they reach value of 0
#Must calculate average sum of guesses, track sum of numbers entered, keep count of attempts

guess = ''
count = 0
total_sum = 0

while guess != 0:
    print("Guess the number: ")
    guess = int(input())

#Count no. of attempts
    count += 1

#Total Sum of guesses
    total_sum += guess

    if guess == 0:
        print("Congratulations!, you guessed the correct number")

    #Average number of guesses
        try:
            average = (total_sum / (count - 1))
            print("Average number:", average)
        except ZeroDivisionError:
            print("No average")

    print("Attempts: ", count)
    print("Total sum of guesses: ", total_sum)

Well, I always start with using a main block. It's just a good habit to have.

Name your constants. What if you wanted the change the number the user was supposed to guess to, say, 5? You already have at least two places in the code where 0 shows up, and what you actually mean by that is "the target value". Give it a name and use that name.

target = 0

while guess != target: # ...

You're checking the game-end logic twice, once in the outer while loop, once in the inner if check. Only do so in one place. E.g., make the outer loop while True: or, better yet, for _ in range(max_num_guesses): with an explicitly set maximum number of guesses. Then, inside the if statement that checks if the user got the answer right, just break to exit the loop.

Now, about what you particularly ask about: computing the sum/mean/count of the users' guesses. Your current approach scatters the logic of what you're trying to do: summation happens on one line, counting on another, division on another... this code sample isn't big, but already this goal is scattered across your code. It'd be a lot cleaner if we dealt with all those similar things in one place. To do this, let's just keep track of the guesses inside the loop, then later, we'll do all the relevant computations with them. Something like:

target = 0
guesses = []

while True:
    print("Guess the number: ")
    guess = int(input())
    guesses.append(guess)

    if guess == target:
        break

    print("Nope.")

print(f"Attempts: {len(guesses)}")
print(f"Total sum of guesses: {sum(guesses)}")
print(f"Average guess: {sum(guesses) / len(guesses)}")

This snippet doesn't do exactly what your code does, but hopefully it gives you an idea for how you can keep different chunks of your code focused on different purposes, and thus keep each bit clean and keep the logic simple. See how the while-loop section is solely focused on running the game logic, and the print section is solely focused on what details you want to print back to the user. The only crossover is the the guesses.append(guess) line in the loop, which just says "I'll want this info later" and let you, later, figure out exactly what you want to do. E.g., let's say you wanted to report the median guess, or how many guesses were above versus below the target; these are easy to add later if we have a list of guesses, but would add yet more fluff to the loop if we tried to keep track as we went along. Note that, since you're just keeping track of user input, this list is never expected to get so big that counting and summing it is worth bothering about; even if you want to report this value in each loop iteration, just compute it each time rather than trying to calculate things incrementally.

Oh, and finally, while I'm all for the try-and-fail when it makes your code cleaner, this probably isn't such an occasion. I'd consider:

if count > 1:
    average = (total_sum / (count - 1))
    print("Average number:", average)
else:
    print("No average")

or even:

print(f"Average number: {total_sum / (count - 1)}" if count > 1 else "No average")

to be cleaner than using a try/except ZeroDivisionError here.

(PS: Note the use of f-strings rather than relying on the print function to append things to strings... that's also a good habit, though not a significant improvement in this particular case.)


Outputting an inverted index to a text file When I run this function for outputting an inverted index to a text file in Debug Configuration, it takes nearly two minutes (96 seconds) with a comparatively tiny dataset, 1252 records with the longest being 76 entries.

std::ostream& operator<<(std::ostream& out_stream, const InvertedIndex& rhs) {
    std::ostringstream output_buffer;
    auto& rhs_index = rhs.GetIndex();
    for(auto map_elem : rhs_index) {
        output_buffer << map_elem.first;
        auto& cur_postingset = map_elem.second;
        for(auto set_elem : cur_postingset) {
            output_buffer << " <" << set_elem.GetDocumentId() << ' ' << set_elem.GetTokenFrequency() << ">";
        }
        output_buffer << '\n';
    }
    output_buffer.flush();
    out_stream.write(output_buffer.str().c_str(), output_buffer.str().size());
    return out_stream;
}

The index is a std::map<std::string, std::vector<std::pair<unsigned long, std::string>>> data type. It makes debugging other portions of the program tedious and time-consuming. Is there any way to speed it up or am I going to have to live with it?

In main add:

std::ios_base::sync_with_stdio(false); //   do not keep the C and C++ streams synced.
                                       //   No extra works make this quicker.

The quickest way to flush one stream into another is:

stream << otherStream.rdBuf();

So rather than this:

out_stream.write(output_buffer.str().c_str(), output_buffer.str().size());

// prefer
out_stream << output_buffer.rdBuf();

The other things that slows things down is excessive flushing. Don't flush your stream until you really want the output (so its not good practice to flush the stream while you are writing to it). Do it explicitly afterwords.

out_stream << object << std::flush;

But I see no reason for copying all the data into one stream then re-copying the data into another stream.

std::ostream& operator<<(std::ostream& out_stream, const InvertedIndex& rhs)
{
    auto& rhs_index = rhs.GetIndex();
    for(auto map_elem : rhs_index) {
        out_stream << map_elem.first;
        auto& cur_postingset = map_elem.second;
        for(auto set_elem : cur_postingset) {
            out_stream << " <" << set_elem.GetDocumentId() << ' ' << set_elem.GetTokenFrequency() << ">";
        }
        out_stream << '\n';
    }
    return out_stream;
}