Replacing values in a list This is my first Powershell script ever so bear with me.

I'm trying to replace values in a sharepoint list from a CSV file:

Param(
    [Parameter(Mandatory=$true)][string] $csvFileName = $null,
    [Parameter(Mandatory=$true)][string] $sharePointWebUrl = $null
)

$isError = $false

cls

$csvFileName = $csvFileName.Trim()
$sharePointWebUrl = $sharePointWebUrl.Trim()

if ([string]::IsNullOrEmpty($csvFileName))
{
    Throw "Please enter parameter values for CSVFileName"
}

if ([string]::IsNullOrEmpty($sharePointWebUrl))
{
    Throw "Please enter parameter values for sharePointWebUrl"
}

Add-PSSnapin Microsoft.Sharepoint.Powershell

try
{
    $csv = Import-Csv $csvFileName
    $subWeb = get-spweb -identity "$sharePointWebUrl"
    $mylistitems = $subWeb.Lists["MyList"].Items | sort Title

    $csvCollection = New-Object System.Collections.ArrayList
    $csv | ForEach-Object {
            $temp = "" | select "Topics", "Column1"
            $temp.Topics = $_.Topic
            $temp.Column1 = $_.NewTopic
            $csvCollection.Add($temp) | Out-Null
    }

    try
    {
            foreach ($li in $mylistitems) {
                    $loopcount=0
                    :loop foreach ($si in $csvCollection)
                    {
                        $loopcount =$loopcount + 1
                        if ($li["Topics"] -eq $si.Topics)
                        {
                            $li["Topics"] = $si.Column1
                            $isFound =$true
                            $li.Update()
                            break loop
                        }
                    }
            }
    }

    catch
    {
            $isError = $true
            $time = Get-Date
            $ErrorMessage = $_.Exception.Message
            "Error Occurred ($time) with error message: $ErrorMessage"| out-file ".\error.log" -append
    }
}

catch
{
    $isError = $true
    $time = Get-Date
    $ErrorMessage = $_.Exception.Message
    "Error Occurred ($time) with error message: $ErrorMessage"| out-file ".\error.log" -append
}

finally
{
    if ($web) {
        $web.dispose();
    }

    if ($isError) {
        write-host "script completed with error(s), please look into error.log file" -ForegroundColor Red
    } else {
        write-host "script completed successfully without error(s)" -ForegroundColor Green
    }
}

Remove-PsSnapin Microsoft.SharePoint.PowerShell

Basically I am looping through a sharepointlist with a nested loop for the CSV file and comparing values. Is this the most efficient/correct way to do this?

Here are some sample lines from the CSV:

Topic,NewTopic,Comments
Building,Appliance,
Your workplace,My Workplace

I'd replace the ArrayList with a hashtable for faster lookup and less code. Replace:

$csv = Import-Csv $csvFileName
$subWeb = get-spweb -identity "$sharePointWebUrl"
$mylistitems = $subWeb.Lists["MyList"].Items | sort Title

$csvCollection = New-Object System.Collections.ArrayList
$csv | ForEach-Object {
        $temp = "" | select "Topics", "Column1"
        $temp.Topics = $_.Topic
        $temp.Column1 = $_.NewTopic
        $csvCollection.Add($temp) | Out-Null
}

try
{
        foreach ($li in $mylistitems) {
                $loopcount=0
                :loop foreach ($si in $csvCollection)
                {
                    $loopcount =$loopcount + 1
                    if ($li["Topics"] -eq $si.Topics)
                    {
                        $li["Topics"] = $si.Column1
                        $isFound =$true
                        $li.Update()
                        break loop
                    }
                }
        }
}

With:

#Create hashtable for topics to update
$TopicUpdates = @{}
Import-Csv $csvFileName | ForEach-Object {
    #Assuming you only have one line per Topic (old value) as hashtable keys needs to be unique
    $TopicUpdates[$_.Topic] = $_.NewTopic
}

$subWeb = get-spweb -identity "$sharePointWebUrl"
$mylistitems = $subWeb.Lists["MyList"].Items | Sort-Object Title

try
{
    foreach ($li in $mylistitems) {

        if($TopicUpdates.ContainsKey($li["Topics"]) {
            #Found Topic in hashtable - Updating
            $li["Topics"] = $TopicUpdates[($li["Topics"])]
            #Not sure what this variable is for
            $isFound = $true
            $li.Update()
        }
    }
}

You should also cleanup your try/catch-design. There's no need for the inner try/catch as it does exactly the same as the parent try/catch.


Add an entry to every element in a nested data structure I currently use a data structure like this:

(def sample-board
  [[{:mine true} {:warn 2   } {:warn 1} {          }]
   [{:warn 2   } {:mine true} {:warn 1} {          }]
   [{:warn 1   } {:warn 1   } {:warn 2} {:warn 1   }]
   [{          } {          } {:warn 1} {:mine true}]])

Now I want to create a copy of the first structure and each element should be supplemented with :explored true.

My approach so far looks like this:

(def sample-board-solved
  (map (partial map
                (partial conj {:explored true}))
       sample-board))

I don't find the approach very appealing though. I thought about using for instead or maybe recur in combination with update-in but I did not bring it to work yet.

I'd be glad on suggestions how to improve the code.

You could maybe make your solution a bit neater by using a lambda in place of one of your partials.  assoc might be a little clearer in purpose than a conj as well:

(def sample-board-solved
  (map (partial map #(assoc % :explored true))
       sample-board))

Using map like this will return things in sequences rather than vectors like the input data.  If it's particularly important that you get vectors out you can always use mapv


Recording a customer's shopping cart as an order I have come up with the following method to post a transaction to a database, in the context of an e-commerce Java web application. The transaction consists in submitting a new order alongside its details:

public long insertOrder(Customer customer, Hashtable<String, CartItem> shoppingCart)
{
    long returnValue = 0L;
    Connection connection = null;
    Statement statement = null;
    try {
        connection = MySqlConnection.getConnection();
        connection.setAutoCommit(false);
        statement = connection.createStatement();
        long orderId = System.currentTimeMillis();
        OrderPeer.insertOrder(statement, orderId, customer);
        OrderDetailsPeer.insertOrderDetails(statement, orderId, shoppingCart);
        connection.commit();
        returnValue = orderId;
    } catch (SQLException e) {
        try { connection.rollback(); } catch (Exception ex) { }
        System.out.println("Could not insert order: " + e.getMessage());
    } finally {
        try { statement.close(); } catch (Exception e) { }
        try { connection.close(); } catch (Exception e) { }
    }
    return returnValue;
}

I would like to know if I am handling the exception, rolling back, and closing the statement/connection in the best possible manner.

Using the "try-with-resources" statement should clean up your code significantly. There are two advantages:

It allows you to declare your resources (in this case Connection and Statement objects) in exactly the scope they're going to be used. This means you don't have to first initialize them to null, which is ugly.
It will ensure your resources get closed no matter what.

Also, you should probably print the stack trace (to aid you and others in debugging), and you should print your error messages to stderr instead of stdout.

public long insertOrder(Customer customer, Hashtable<String, CartItem> shoppingCart)
{
    try (Connection connection = MySqlConnection.getConnection();
        Statement statement = connection.createStatement();) {
        connection.setAutoCommit(false);
        long orderId = System.currentTimeMillis();
        OrderPeer.insertOrder(statement, orderId, customer);
        OrderDetailsPeer.insertOrderDetails(statement, orderId, shoppingCart);
        connection.commit();
        return orderId;
    } catch (SQLException e) {
        try {
           connection.rollback();
        } catch (Exception ex) {
            System.err.println("Could not roll back connection: " + ex.getMessage())
            ex.printStackTrace();

        }
        System.err.println("Could not insert order: " + e.getMessage());
        e.printStackTrace()l
        return 0;
    }
}


Half precision reader/writer for C# I'm reading/writing half precision floating point numbers in C#. These are basically 16 bit floats, compared to the usual 32/64 bit floats and doubles we are used to working with.

I've taken some highly tested Java code from an obvious "expert on the subject" here and modified it to work with C#. Is this correct?

// ignores the higher 16 bits
public static float toFloat( int hbits )
{
    int mant = hbits & 0x03ff;            // 10 bits mantissa
    int exp =  hbits & 0x7c00;            // 5 bits exponent
    if( exp == 0x7c00 )                   // NaN/Inf
        exp = 0x3fc00;                    // -> NaN/Inf
    else if( exp != 0 )                   // normalized value
    {
        exp += 0x1c000;                   // exp - 15 + 127
        if( mant == 0 && exp > 0x1c400 )  // smooth transition
            return BitConverter.ToSingle(BitConverter.GetBytes( ( hbits & 0x8000 ) << 16
                                            | exp << 13 | 0x3ff ), 0);
    }
    else if( mant != 0 )                  // && exp==0 -> subnormal
    {
        exp = 0x1c400;                    // make it normal
        do {
            mant <<= 1;                   // mantissa * 2
            exp -= 0x400;                 // decrease exp by 1
        } while( ( mant & 0x400 ) == 0 ); // while not normal
        mant &= 0x3ff;                    // discard subnormal bit
    }                                     // else +/-0 -> +/-0
    return BitConverter.ToSingle(BitConverter.GetBytes(          // combine all parts
        ( hbits & 0x8000 ) << 16          // sign  << ( 31 - 15 )
        | ( exp | mant ) << 13 ), 0);         // value << ( 23 - 10 )
}
// returns all higher 16 bits as 0 for all results
public static int fromFloat( float fval )
{
    int fbits = BitConverter.ToInt32(BitConverter.GetBytes(fval), 0);
    int sign = fbits >>> 16 & 0x8000;          // sign only
    int val = ( fbits & 0x7fffffff ) + 0x1000; // rounded value

    if( val >= 0x47800000 )               // might be or become NaN/Inf
    {                                     // avoid Inf due to rounding
        if( ( fbits & 0x7fffffff ) >= 0x47800000 )
        {                                 // is or must become NaN/Inf
            if( val < 0x7f800000 )        // was value but too large
                return sign | 0x7c00;     // make it +/-Inf
            return sign | 0x7c00 |        // remains +/-Inf or NaN
                ( fbits & 0x007fffff ) >>> 13; // keep NaN (and Inf) bits
        }
        return sign | 0x7bff;             // unrounded not quite Inf
    }
    if( val >= 0x38800000 )               // remains normalized value
        return sign | val - 0x38000000 >>> 13; // exp - 127 + 15
    if( val < 0x33000000 )                // too small for subnormal
        return sign;                      // becomes +/-0
    val = ( fbits & 0x7fffffff ) >>> 23;  // tmp exp for subnormal calc
    return sign | ( ( fbits & 0x7fffff | 0x800000 ) // add subnormal bit
         + ( 0x800000 >>> val - 102 )     // round depending on cut off
      >>> 126 - val );   // div by 2^(1-(exp-127+15)) and >> 13 | exp=0
}

Is this correct?

Maybe.

For a start, the >>> operator doesn't exist in C#.

Taking a guess, I replaced >>> with >> and then wrote the following 'unit test' for it:

    static void assertFloat(float fval)
    {
        int i = fromFloat(fval);
        float f2 = toFloat(i);
        if (fval != i)
            throw new ApplicationException();
    }

    static void Main(string[] args)
    {
        assertFloat(0);
        assertFloat(1);
        assertFloat(0.5f);
        assertFloat(-0.5f);
        assertFloat(-0);
        assertFloat(float.PositiveInfinity);
        assertFloat(float.NaN);
        float big = 1024 * 1024;
        big *= big;
        assertFloat(big);
    }

It failed the second test: because 1.0 is round-trip-converted to 1.000122.

I am disappointed by an encoding scheme which cannot encode '1.0' exactly.

However you didn't say how exact you expect the round-trip to be, so I don't know whether it's correct.

The following is a list of some input with corresponding output:

0 -> 0
1 -> 1.000122
1.1 -> 1.099609
-1 -> -1.000122
0.5 -> 0.500061
-0.5 -> -0.500061
0.001 -> 0.001000404
5.5 -> 5.5
5.6 -> 5.601563
5.7 -> 5.699219
0 -> 0
Infinity -> Infinity
NaN -> NaN
1024 -> 1024.125
1048576 -> Infinity

So it seems approximately correct for those numbers.

I can't say whether it's the best encoding. For example, this encoding gains the ability to express decimals but loses the ability to express integers (they become approximated) and big integers (they become infinity).

A different encoding scheme could be devised (and might be more useful depending on your application) which cannot express decimals but which gains the ability to express (approximately) some numbers which are bigger-than-the-biggest integer.

There are a lot of 'magic numbers' (i.e. hard-coded constants) in the code. To inspect for correctness I would need to guess/reverse engineer the way in which you encode/use the 16 bits for your "half precision float" numbers. You could make it easier by documenting the format using comments: which bits do you use for what?

Note this comment in the post you linked to:

  I see what you mean but these NaN values wont be returned from Float.floatToIntBits which normalizes all NaNs to 0x7fc00000. The rounded val can thus never become nagative. Maybe it would be faster to use floatToRawIntBits (which does not do NaN normalization) and then deal with the overflow NaNs i.e. by adding || val < 0 to the first branch. This would also allow to preserve some of the extra NaN bits. I remember that I had planned to do this but couldn't find sufficient documentation on how to handle these bits and thus settled with normalized NaNs.

The BitConverter which you use may not (I haven't tested it) normalize NaN values in the same way.

The original OP linked to an spec for Half-precision floating-point format which says,

  Integers between 0 and 2048 can be exactly represented

So my testing, which shows error in round-tripping 1, suggests that this spec is NOT implemented correctly.

When I test it, 1.0f is encoded as 0x3c00 which is correct according to the spec. So the bug is presumably being introduced in the toFloat method, specifically this statement:

return BitConverter.ToSingle(BitConverter.GetBytes( ( hbits & 0x8000 ) << 16
                                        | exp << 13 | 0x3ff ), 0);

This is a line which the "obvious expert on the subject" said they "implemented [as a] small extension compared to the book".

IOW you may have made a translation from the Java, but the Java doesn't correctly/fully implement the spec (it tries to improve on the spec, perhaps resulting in an inability to accurately decode small integers).


Catch-style unit testing in JavaScript (phase 1) I decided to port my favorite unit testing framework, Catch, to JavaScript. So far I'm focusing on test cases and sections. I'll add assertions in phase 2, and output in phase 3.

The goal for this phase was to get all the assertions in the following test code to pass:

function assert(value, message) {
    console.log(value ? 'pass' : 'FAIL', message || '')
}

TEST_CASE('See if this works right', '[stuff]', function(){
    var x = 2, y = 4;

    console.log('============');
    SECTION('halving a good time', function(){
        x /= 2;
        y /= 2;
        assert(x == 1 && y == 2, 'halved values');
    });
    SECTION('double the fun', function(){
        x *= 2;
        y *= 2;
        assert(x == 4 && y == 8, 'doubled values');
        SECTION('up high', function(){
            x += 5;
            y += 5;
            assert(x == 9 && y == 13, 'high five');
        });
        SECTION('down low', function(){
            x -= 5;
            y -= 5;
            assert(x == -1 && y == 3, 'low five');
        });
    });
});

Notice that the entire test case should be run from the beginning, once for each "leaf" section, just like Catch (follow the second link in this question for a more complete explanation).

Here's what I've written so far. The documentation is very minimal right now; I'll be annotating and commenting properly later.

( /** @type function(Window) **/ function(global) {
    'use strict';

    var context = [],
        testCases = [],
        currentTestCase,
        nextTask;

    /** Section.
    */
    function Section(name, body, tags) {
        this.name = name;
        this.tags = tags;
        this.body = body;
        this.subsections = [];
        this.subsectionIndex = 0;
        this.subsectionsEncountered = 0;
    }

    /** Run section.
    */
    Section.prototype.run = function() {
        context.unshift(this);
        this.subsectionsEncountered = 0;
        this.body();
        context.shift();
    }

    /** Update subsection.
    */
    Section.prototype.updateSubsection = function(name, body) {
        var index = this.subsectionIndex,
            subsection = this.subsections[index];

        if (!subsection) {
            subsection = new Section(name, body);
            this.subsections[index] = subsection;
        } else {
            subsection.body = body;
        }

        return subsection;
    }

    /** Test case.
    */
    global['TEST_CASE'] = function(name, tags, body) {
        var next;

        currentTestCase = new Section(name, body, tags);
        testCases.push(currentTestCase);
        currentTestCase.run();
        while (nextTask) {
            next = nextTask;
            nextTask = null;
            next();
        }
    };

    /** Test case section.
    */
    global['SECTION'] = function(name, body) {
        var outer = context[0],
            oldNext = nextTask;

        if (outer.subsectionIndex == outer.subsectionsEncountered++) {
            nextTask = function() {
                if (++outer.subsectionIndex < outer.subsectionsEncountered) {
                    currentTestCase.run();
                } else if (oldNext) {
                    oldNext();
                }
            };
            outer.updateSubsection(name, body).run();
        }
    };

}(this));

I've kept the macro-style naming conventions from Catch, and dumped a bunch of things in the global scope. This may or may not change, but this is one thing I'm probably not interested in having reviewed, since it's easier for me to keep the names exactly the same while porting. The rest is fair game.

  Update: phase 2 is mostly complete.

Overall Design

For such a small amount of code, I find the logic extremely difficult to follow. I understand what is happening at a high level, but it seems every function is responsible for managing everything, and program control bounces around a lot.

As an example, the final function assigned to SECTION has several responsibilities:

Capture the enclosing section and current next task.
Do nothing if this new section should be skipped.
Create a new next task that will either

rerun the test case (if more sections have been added),
run the previous next task (if there is one), or
do nothing.

Add the new section to the enclosing section.

BTW, under what condition does updateSubsection not create a new section? I can see it looks for a section with the same name, but is that supposed/allowed to happen?
Finally, run the new section.

That's way too much work for one nine-line function to do.

As a first step, add a TestCase class (subclass of Section) to own the overall state of running a single test case, and move the logic from the last two functions into it, breaking them up into smaller functions. While test cases are run sequentially, I would move context and nextTask into this new class as I believe they belong to a single test case run.

Names

Most of the variable names are pretty clear, but as I dig into the code some are misleading.

context sounds more like a map of values passed to each section rather than a stack of running sections. I would rename it to reflect that nature, e.g., activeSections.
subsectionsEncountered is a little cumbersome; how about the more standard subsectionCount? And subsectionIndex only tells me it's an index into some container--not why that subsection is special. I'd go with activeSubsectionIndex though I would prefer to store the actual Section instance in it and drop the Index suffix if possible.
You're creating a tree of sections, and parent is more obvious than outer.

Other Thoughts

I'd be tempted to drop the stack of sections (context) in favor of storing just the active section since that's all you need. To pop a completed section off the stack, a) store a reference to the parent section in each section instance and b) assign the parent to activeSection after each section completes.

Finally, you need to handle exceptions so other sections can be run. You should be able to do it once in Section.prototype.run.

On to Phase Two!

I'm very curious to see this in action. With QUnit I am often forced to perform the same actions on the fixture as a previous test did in order to set up the current test. This causes code duplication whereas the whole point of setting up a fixture is to avoid it.

In Java I've been building separate fixture classes that provide methods for setting up common fixture states that can be reused across many tests. While that's helpful, it's not as easy to do in JavaScript.


Refactoring a function with different signature I call several WebServices which return each a different exception type:

try {
   //calling first WS1 here
} catch (my.package1.FaultWS_Exception e) {
        StringBuilder sb = getMessageFromException(e);
        log(sb);
}

try {
   //calling first WS2 here
} catch (my.package2.FaultWS_Exception e) {
        StringBuilder sb = getMessageFromException(e);
        log(sb);
}

my getMessageFromException simply returns a pretty printed message from the exception.

But as my exceptions are in different packages (each method is in a different WS), I had to create different getMessageFromException() functions for each exception:

private StringBuilder getMessageFromException(my.package1.FaultWS_Exception e) {
    StringBuilder sb = new StringBuilder();
    if (e.getFaultInfo() != null) { //returns my.package1.FaultWS
        sb.append(e.getFaultInfo().getDescfault());
        sb.append("\n");
        sb.append(e.getFaultInfo().getCodefault());
        sb.append("\n");
    } else if (e.getMessage() != null) {
        sb.append(e.getMessage());
        sb.append("\n");
    }
    return sb;
}

private StringBuilder getMessageFromException(my.package2.FaultWS_Exception e) {
    StringBuilder sb = new StringBuilder();
    if (e.getFaultInfo() != null) { //returns my.package2.FaultWS
        sb.append(e.getFaultInfo().getDescfault());
        sb.append("\n");
        sb.append(e.getFaultInfo().getCodefault());
        sb.append("\n");
    } else if (e.getMessage() != null) {
        sb.append(e.getMessage());
        sb.append("\n");
    }
    return sb;
}

This code looks very ugly and a bit repetitive (I have 4 different signatures so far), is there a way to refactor it ? knowing that the Exceptions are defined the same, except from the package name.

Simply use one method for all type of Exceptions.

Make it public static and move into a utility class.

public static StringBuilder getMessageFromException(Exception e){...}

Why do you need overloaded methods for different type of Exceptions?

User defined Exceptions are also subtype of Exception itself.

--EDIT--

create an interface for all your custom exceptions having  getFaultInfo() method.

interface MyFaultWS {} // marker interface
class FaultWS implements MyFaultWS {} // do it for both the packages

interface FaultInfoException {FaultWS getFaultInfo();}

// do it for both the packages
class FaultWS_Exception extends Exception implements FaultInfoException {

    @Override
    public FaultWS getFaultInfo() {
        return ...; // return your package specific exception
    }
}

Finally use

public static StringBuilder getMessageFromException(FaultInfoException e){...}


Tic Tac Toe algorithm using itertools I'm looking for feedback on the readability of my code and opinions on the pick_best algorithm. I do this stuff for fun when I have time, and never get a chance to have anyone else look at it. Hopefully, I can use this site to improve my coding.

from itertools import permutations
from random import randint
class TTT:
    def __init__(self):
        self.game_grid = [0 for x in range(9)]
        self.winning_matrix = ((0,1,2), (3,4,5), (6,7,8), (0,3,6), (1,4,7), (2,5,8), (0,4,8), (2,4,6))

    def game_state(self, grid):
        '''Prints current game state'''
        print "\n\n\n0 1 2 \n3 4 5 \n6 7 8 \n"
        for square in range(9):
            if square%3==0: print
            print ['.', 'O', 'X'][grid[square]],
        print

    def check_game(self, grid):
        '''Checks game state for a winner or tie'''
        for row in self.winning_matrix:
            total = sum([grid[x] for x in row])
            if abs(total) == 3:
                return self.winner_is(total)
        return self.winner_is(total)

    def winner_is(self, total):
        '''Establishes which player wins in current node'''
        return total/3 if abs(total)==3 else 0

    def pick_best(self):
        '''A non-recursive minimax style function utilizing itertools.permutations'''
        avail_squares = [pos for pos, x in enumerate(self.game_grid) if x==0]
        score = {x:0 for x in avail_squares}    #keeps minimax score for current game state
        for perm in permutations(avail_squares): #thank you itertools
            player = -1 #start with computer player
            node = list(self.game_grid) #node is a copy of the current game state
            for square in perm:
                node[square]=player #makes a move in the permutation
                state_score = self.check_game(node) #checks to see if anyone won
                if state_score:
                    ''' if there is a winning play in this node,
                        a score based off the depth of the play is added
                        to the total score of the possible move.
                        Then we break out of the current permutation'''
                    score[perm[0]] += (10**(len(perm)-perm.index(square)))*-1*state_score
                    break
                player *= -1 #swaps to the next player in the node
        return max(score, key=lambda x: score[x]) #returns square with highest score

    def player_choice(self):
        start = raw_input("Would you like to start? (Y/N) ")
        print start
        if start.upper() == 'Y':
            return False, 1
        elif start.upper() == 'N':
            return False, -1
        else:
            print "Please choose (Y)es or (N)o"
            return True, 0

    def pick_a_spot(self):
        move = raw_input("Where would you like to move 0-8? ")
        if move.isdigit() and 0<=int(move)<9:
            m = int(move)
            if self.game_grid[m] == 0:
                return False, m
        return True, 9

    def play(self):
        '''A very basic game play interface to test algorithm.'''
        invalid = True
        while invalid:
            invalid, player = self.player_choice()
        if player == -1:
            self.game_grid[randint(0,8)]=-1
            player = 1
        self.game_state(self.game_grid)
        while 0 in self.game_grid and self.check_game(self.game_grid)==0:
            if player == 1:
                invalid = True
                while invalid:
                    invalid, move = self.pick_a_spot()
                self.game_grid[move]=1
                player = -1
            else:
                move = self.pick_best()
                player = 1
                self.game_grid[move]=-1
            self.game_state(self.game_grid)

if __name__=='__main__':
    TTT().play()

TTT is not a clear name, TicTacToe is only 6 characters more, and much clearer.

You should have winning_matrix be a constant, defined just inside the class declaration:

class TicTacToe:
    WINNING_MATRIX = ((0,1,2), (3,4,5), (6,7,8), (0,3,6), (1,4,7), (2,5,8), (0,4,8), (2,4,6))

Indexing a literal is odd to see, you should declare the constant ICONS and index that.

ICONS = ['.', 'O', 'X']

...

print ICONS[grid[square]],

I personally prefer using print('\n') to make it more clear what's happening as it can look funny to see just a print. You also shouldn't use an inline if as they're very easy to misread.

if square % 3 == 0: print
print ['.', 'O', 'X'][grid[square]],

The above looks like the if is meant to apply to the print call beneath it, but has an indentation error. Even when you spot the print, it seems like that was put there accidentally because it doesn't seem to print anything.

if square % 3 == 0:
    print('\n')
print(ICONS[grid[square]])

Also, instead of iterating over numbers and using those to index you should iterate over grid itself. You need the index of the values to detect when to print a new line, but you can still do this using enumerate.

for index, square in enumerate(grid):
    if index % 3 == 0:
        print('\n')
    print(ICONS[square])

This is much simpler than a double index. Plus, if you were to change the size of grid for any reason, you'd then be able to handle it just fine. The one last change, particularly in that case, could be to replace the 3 with a constant like ROW_WIDTH.

You could also make functions a lot clearer, like this:

def winner_is(self, total):
    '''Establishes which player wins in current node'''
    return total/3 if abs(total)==3 else 0

Your docstring is vague, and more abstract than it should be. Tell me what the function does, not what it's for.

def winner_is(self, total):
    '''Returns -1 or 1 to indicate which player won, or 0 for a tie.'''

However what's odd is that you're doubling down on your check for if the total is 3? You're calling self.winner_is when you've already determined that the total is three, or when it's definitely a tie. I'd combine the functions into one:

def check_game(self, grid):
    '''Returns -1 or 1 to indicate which player won, or 0 for a tie.'''

    for row in self.winning_matrix:
        total = sum([grid[x] for x in row])
        if abs(total) == 3:
            return total / 3
    return 0

Now it's much simpler and contained, but still does exactly the same. I'd also consider renaming this, as it's unclear what a check would do, or what aspect of the game it applies to. eval_winner could improve the clarity.


Countdown Timer evaluation Please consider evaluating the code below for best practices, efficiency, and mistakes. I got some of the code from W3 schools (I know, I know, but they've come a long way) and assimilated it to what I need. Particularly, the days = hours = minutes = seconds = 0; is likely going to cause people stress but after research and evaluation, I found that it works well in this case.
Please evaluate my JS code. Thank you. CODEPEN
IMPRORTANT: The code below checks for the existence of an ACF variable (exposed via the functions.php with a WordPress backend), upon which this entire code is based. I replaced the variable with an example string which would be returned by this field.
(function () {
    let acf_vars = {timer: "2021 1 21 13 08 00"}; // this code is a replacement for the ACF variable

    if (!acf_vars.timer) {
        return document
            .querySelectorAll('.mobile-ticker')
            .forEach((container) => (container.style.display = 'none'));
    }

    const backendTimer = acf_vars.timer.split(' ');
    const y = backendTimer[0];
    const M = +backendTimer[1] - 1; // monthIndex in Date Object begins with 0, so we subtract 1
    const d = backendTimer[2];
    const h = backendTimer[3];
    const m = backendTimer[4];
    const s = backendTimer[5];

    // new Date(year, monthIndex [, day [, hours [, minutes [, seconds [, milliseconds]]]]])
    const countDownDate = new Date(y, M, d, h, m, s, 0).getTime();

    const timerInterval = setInterval(() => {
        const now = new Date().getTime();
        const distance = countDownDate - now;
        const expiredTimer = distance <= 0;

        let days = Math.floor(distance / (1000 * 60 * 60 * 24));
        let hours = Math.floor((distance % (1000 * 60 * 60 * 24)) / (1000 * 60 * 60));
        let minutes = Math.floor((distance % (1000 * 60 * 60)) / (1000 * 60));
        let seconds = Math.floor((distance % (1000 * 60)) / 1000);

        if (expiredTimer) {
            days = hours = minutes = seconds = 0;
            clearInterval(timerInterval);
        }

        if (typeof window.innerWidth === 'number' && window.innerWidth < 768) {
            document.querySelector('.ticker-mobile #days').innerHTML = days;
            document.querySelector('.ticker-mobile #hours').innerHTML = hours;
            document.querySelector('.ticker-mobile #minutes').innerHTML = minutes;
            document.querySelector('.ticker-mobile #seconds').innerHTML = seconds;
        } else {
            document.querySelector('.ticker-desktop #days').innerHTML = days;
            document.querySelector('.ticker-desktop #hours').innerHTML = hours;
            document.querySelector('.ticker-desktop #minutes').innerHTML = minutes;
            document.querySelector('.ticker-desktop #seconds').innerHTML = seconds;
        }
    }, 1000);
})();

Can you send the timestamp instead? Manipulating dates in JS is a bit tedious without a library. This is probably pretty clear to you already from having to declare all the y M d h m s variables. PHP, on the other hand, has createFromFormat, which has a flexible and friendly API. For example, I believe you could pass a formatting string of 'Y m d H i s' (corresponding to the input of '2021 1 4 13 08 00') and get the timestamp out of it immediately.
Working with timestamps is easier than working with custom formatted strings anyway. I don't know the source of the original date string, but I'd turn it into a timestamp as soon as possible once your app receives it (and, for example, save the timestamp in the database instead of the string).
When you do have to pass around a string instead of a timestamp, use a standardized ISO 8601 string, which can be parsed automatically in most languages.
Ticker display Currently, the tickers are hidden via the JS if there's no timer. Consider if you could carry out this check (and this whole conditional code) in the PHP, rather than sending the HTML markup regardless and then possibly hide it if there's no timer. For example, you might change it so that the server only serves this .js if there's a timer to begin with, otherwise the


Multiplication Exercise Generator I'm new to programming in Python, and I am trying to create a multiplication exercise generator. I've got the following code for the solution, but I'm wondering if there is any more elegant solutions than this one.
Note: I have not covered lists, tuples, or dictionaries as of yet so I'm using the absolute basics to create this program.

#solution.py
#A script that allows a student to learn multiplication using the random
#module. Answers will be made and the script must display to try again
#or to present the next questions.

from random import randrange
answer = 0

def generate_question() :
    global answer

    #Random numbers
    number_one = randrange(1, 13)
    number_two = randrange(1, 13)

    #Correct answer
    answer = number_one * number_two

    #Display question
    print "What is %d times %d?" % (number_one, number_two)

def check_answer(user_input) :
    global answer

    #Answer check
    if user_input == answer :
        print "Very good!"
        generate_question()
    else :
        print "Try again!"

#Script Section
generate_question()

while True :
    #Answer to question
    user_input = int(raw_input("Answer: "))
    check_answer(user_input)

You're using functions, that's good. But, your program would be easier to understand, and better, if it didn't.

As a rule of thumb; don't put things in global scope.
Whether it be via global or by not putting your code in a function.
This is the global scope becomes un-reliable and you proceed to not know what your code is doing.

I would say that you should move check_answer into your while loop.
This is as rather than using generate_question you should use break.
This at the moment would end the program, but if you add another while True loop you can get the same functionality.
This is better as then you are starting to separate generating a question and checking if it's correct.
As we now only rely on the outer while to correctly setup the inner while, rather than changing globals to get things to work.

And so I'd use:

from random import randrange

def generate_number():
    return randrange(1, 13)

def main():
    while True:
        num1, num2 = generate_number(), generate_number()
        answer = num1 * num2
        print "What is %d times %d?" % (num1, num2)
        while True:
            user_input = int(raw_input("Answer: "))
            if user_input == answer:
                print "Very good!"
                break
            else:
                print "Try again!"

You should notice that this is your code. With a second while loop and a break.
But you should notice it's easier to understand what it's doing, when coming as a third party.

As at first, I didn't notice your code asks more than one question.

Just so you know you're already using a tuple! In print "What is %d times %d?" % (num1, num2).
The (num1, num2) is creating a tuple.
If you decided to use this rather than num1 and num2 then you would change num1 to num[0] and num2 to num[1].
This is as in most programming languages they start lists with zero rather than one.
Lists actually work the same way as tuples, but are created using [] rather than ().
The only difference is you can change num[0] when using a list, but you can't with a tuple.

You should also learn what try-except are. If I enter abc rather than 123 into your program it would exit abruptly.
What these do is try to run the code, if an error occurs run the except code.
But I'll warn you now, never use a 'bare except'. Which is an except that doesn't say what it's protecting against.
This is as if the code raises a different error than you'll run code as if it were another, which can hide bugs.
And makes finding those bugs really hard to find.

And so I'd further change the code to use:

from random import randrange

def generate_number():
    return randrange(1, 13)

def main():
    while True:
        num1, num2 = generate_number(), generate_number()
        answer = num1 * num2
        print "What is %d times %d?" % (num1, num2)
        while True:
            try:
                user_input = int(raw_input("Answer: "))
            except ValueError:
                print('That is not a valid number.')
                continue
            if user_input == answer:
                print "Very good!"
                break
            else:
                print "Try again!"


Conditional callback for asynchronous file copy I'm trying to figure out the most elegant solution to do the following asynchronously using JavaScript (specifically node):

Given a destination file name, check if it is a directory
If it is a directory, delete it
Copy a source file onto the destination file name

I would like to do this asynchronously using callbacks.

Here is what I have come up with:

  var preCopyOp = fs.lstatSync(dstFilename).isDirectory() ?
    function(callback) {
      rimraf(dstFilename, callback)
    } :
    function(callback) {
      callback();
    };

  preCopyOp(function() {
    ncp(srcFilename, dstFilename, function(err) {
      if(err) {
        return console.error(err);
      }
    });
  });

The no-op passthrough function seems like a bit of a kludge to me. Is there a more elegant way to do this?

You can write

function(callback) {
  callback();
}

as just

callback


Caesar Cipher implementation in Java I wanted to create a class that encrypts provided clear text using Caesar Cipher. Do you find my implementation clean? Any suggestions?

package biz.tugay.caesarcipher;

import java.util.Locale;

/*
    Encyrpts a clear text using Caeser Cipher (https://en.wikipedia.org/wiki/Caesar_cipher)
    with given shift amount.

    Provided shift amount (i.e. key) must be a positive integer less than 26.
    Only English alphabet is supported and encyrpted text will be in uppercase.

    Shift amount 0 will return the same clear text.
 */
public final class CaesarCipher {

    private final String clearText;
    private final int key;

    public CaesarCipher(final String clearText, final int key) {
        if (clearText == null) {
            throw new UnsupportedOperationException("Clear text to be encrypted can not be null!");
        }

        if (key < 0 || key > 26) {
            throw new UnsupportedOperationException("Key must be between 0 and 26");
        }

        this.clearText = clearText;
        this.key = key;
    }

    public String encryptText() {
        final StringBuilder cipherTextBuilder = new StringBuilder();

        final String clearTextUpperCase = clearText.toUpperCase(Locale.US);
        final char[] clearTextUpperCaseCharArray = clearTextUpperCase.toCharArray();

        for (final char c : clearTextUpperCaseCharArray) {
            if (c < 65 || c > 90) { // If the character is not between A .. Z, append white space.
                cipherTextBuilder.append(" ");
                continue;
            }
            final Character encryptedCharacter = encryptCharacter(c);
            cipherTextBuilder.append(encryptedCharacter);
        }

        return cipherTextBuilder.toString();
    }

    private Character encryptCharacter(final char c) {
        final int initialShift = c + key;
        final int finalShift;

        if (initialShift > 90) {
            // This is the case where we go beyond Z, we must cycle back to A.
            finalShift = (initialShift % 90) + 64;
        } else {
            // We are in the boundries so no need to cycle..
            finalShift = initialShift;
        }

        return (char) finalShift;
    }
}

and I have 2 simple tests:

package biz.tugay.caesarcipher;

import org.junit.Assert;
import org.junit.Test;

public class CaesarCipherTest {

    @Test
    public void shouldReturnBCDForClearTextABCAndKey1() {
        final String clearText = "abc";
        final CaesarCipher caesarCipher = new CaesarCipher(clearText, 1);
        final String encryptedText = caesarCipher.encryptText();
        Assert.assertTrue(encryptedText.equals("BCD"));
    }

    @Test
    public void shouldReturnAForZAndKey1() {
        final String clearText = "Z";
        final CaesarCipher caesarCipher = new CaesarCipher(clearText, 1);
        final String encryptedText = caesarCipher.encryptText();
        Assert.assertTrue(encryptedText.equals("A"));
    }
}

package biz.tugay.caesarcipher;

import java.util.Locale;

/*
    Encyrpts a clear text using Caeser Cipher (https://en.wikipedia.org/wiki/Caesar_cipher)
    with given shift amount.

There is a typo in "Encrypts", and the URL should be a hyperlink, like Caesar Cipher.

    Provided shift amount (i.e. key) must be a positive integer less than 26.

Depending on who you ask, the word positive may exclude the 0. You should just say must be between 0 and 25.

But even more important: the documentation must match the code. Currently the code says <= 26.

    Only English alphabet is supported and encyrpted text will be in uppercase.

The same typo as above. And, the Turkish lowercase dotless i (ı) is also "supported", although not intentionally.

    Shift amount 0 will return the same clear text.
 */
public final class CaesarCipher {

    private final String clearText;

There is no reason that the Cipher class ever stores the clear text. Therefore this field should be replaced with a method parameter.

    private final int key;

    public CaesarCipher(final String clearText, final int key) {
        if (clearText == null) {
            throw new UnsupportedOperationException("Clear text to be encrypted can not be null!");
        }

        if (key < 0 || key > 26) {
            throw new UnsupportedOperationException("Key must be between 0 and 26");
        }

        this.clearText = clearText;
        this.key = key;
    }

    public String encryptText() {
        final StringBuilder cipherTextBuilder = new StringBuilder();

        final String clearTextUpperCase = clearText.toUpperCase(Locale.US);
        final char[] clearTextUpperCaseCharArray = clearTextUpperCase.toCharArray();

        for (final char c : clearTextUpperCaseCharArray) {
            if (c < 65 || c > 90) { // If the character is not between A .. Z, append white space.

This comment is redundant if you write c < 'A' || c > 'Z'.

                cipherTextBuilder.append(" ");
                continue;
            }
            final Character encryptedCharacter = encryptCharacter(c);
            cipherTextBuilder.append(encryptedCharacter);
        }

        return cipherTextBuilder.toString();
    }

    private Character encryptCharacter(final char c) {
        final int initialShift = c + key;
        final int finalShift;

        if (initialShift > 90) {
            // This is the case where we go beyond Z, we must cycle back to A.
            finalShift = (initialShift % 90) + 64;
        } else {

This can never happen since you already check the condition in the encryptText method.

            // We are in the boundries so no need to cycle..
            finalShift = initialShift;
        }

        return (char) finalShift;
    }
}

I would write the encryptCharacter method as follows and remove the bounds check from the encryptText method:

private char encryptCharacter(char c) {
    if ('A' <= c && c <= 'Z') {
        int position = c - 'A';
        int shiftedPosition = (position + shift) % 26;
        return (char) ('A' + shiftedPosition);
    } else {
        return c;   // Or ' ', as in your code
    }
}

The tests

package biz.tugay.caesarcipher;

import org.junit.Assert;
import org.junit.Test;

public class CaesarCipherTest {

    @Test
    public void shouldReturnBCDForClearTextABCAndKey1() {
        final String clearText = "abc";
        final CaesarCipher caesarCipher = new CaesarCipher(clearText, 1);

        final String encryptedText = caesarCipher.encryptText();

        Assert.assertTrue(encryptedText.equals("BCD"));
    }

I usually split each test into three paragraphs. The first prepares everything, the second does the interesting work, and the third asserts that the result is correct. When you follow this style, you can easily see which part of the code is worth stepping through with a debugger.

Instead of assertTrue, you should call assertEquals("BCD", encryptedText). Because when that assertion fails, the error message is much nicer, giving you the expected and the actual result.

    @Test
    public void shouldReturnAForZAndKey1() {
        final String clearText = "Z";
        final CaesarCipher caesarCipher = new CaesarCipher(clearText, 1);
        final String encryptedText = caesarCipher.encryptText();
        Assert.assertTrue(encryptedText.equals("A"));
    }
}

You forgot to test lowercase letters and non-alphabetic characters. And emojis, which by the way would result in two spaces per emoji.


Dirty Bash hack to run the ssh connections in the Parallel mode A Few years back I used to opt the below method to run the shell program in a Parallel way to open multiple ssh session at a time to get a quick result.
I know there are various other good utilities available, however due to some pressing needs I've to use this way, this dirty piece of code works well however I would like to know if there are some suggestion to improve it further considering the fact I have to use it for some compelling reasons over other better tools.
#!/bin/bash
echo "$(date) Starting the SSH Connection Check...."
#set -x

read -rsp $'Please Enter password below: ' SSHPASS
export SSHPASS
SSH_Connection () {
    hostTarget=${1}
    sshpass -e ssh -q "${hostTarget}" "true" -o StrictHostKeyChecking=no -o ConnectTimeout=60 2>>/dev/null
    if [[ $? -eq 0 ]]
    then
        echo  "$CurrntTime $MachineName   : SSH connection is Up"
    elif [[ $? -eq 255 ]]
        then
        echo  "$CurrntTime $MachineName   : SSH Authentication Failed"
    elif [[ $? -eq 2 ]]
        then
        echo  "$CurrntTime $MachineName   : SSH connection is Down"
    elif [[ $? -eq 1 ]]
        then
        echo  "$CurrntTime $MachineName   : SSH Authentication Failed"
    else
        echo  "$CurrntTime $MachineName   : SSH connection is Down"
    fi
}

HostList=$(<~/host2)

CurrntTime=$(date +'%m/%d/%Y %T')

for MachineName in ${HostList}
do
    SSH_Connection "${MachineName}" &
done
# wait for all outstanding background jobs to complete before continuing
wait
# at last clear the exported variable containing the password
unset SSHPASS

The only part I would change are the chained if statements. Use case:
CurrntTime="some_timestamp"
MachineName="some_host"
for retval in 0 255 2 1 8; do
   echo "Testing exitcode ${retval}"
   case ${retval} in
   0)
        echo  "$CurrntTime $MachineName   : SSH connection is Up";;
   255)
        echo  "$CurrntTime $MachineName   : SSH Authentication Failed";;
   2)
        echo  "$CurrntTime $MachineName   : SSH connection is Down";;
   1)
        echo  "$CurrntTime $MachineName   : SSH Authentication Failed";;
   *)
        echo  "$CurrntTime $MachineName   : SSH connection is Down";;
   esac
done

More optimisation is not needed. The alternative beneath is slightly shorter but makes the code harder to read (more fun, though):
CurrntTime="some_timestamp"
MachineName="some_host"

ssh_err_0="connection is Up"
ssh_err_1="Authentication Failed"
ssh_err_2="connection is Down"
ssh_err_255="Authentication Failed"
ssh_err_other="connection is Down"

for retval in 0 255 2 1 8; do
   echo "Testing exitcode ${retval}"
   printf "%s" "$CurrntTime $MachineName   : SSH "
   case ${retval} in
   0|1|2|255)
      message="ssh_err_${retval}"
      echo "${!message}";;
   *)
      echo "${ssh_err_other}";;
   esac
done

When you use the first case in the total script, and move the SSH_Connection() function to the start of the script, it would be
#!/bin/bash

SSH_Connection () {
    hostTarget=${1}
    sshpass -e ssh -q "${hostTarget}" "true" -o StrictHostKeyChecking=no \
       -o ConnectTimeout=60 2>/dev/null

    case $? in
    0)
       echo  "$CurrntTime $hostTarget: SSH connection is Up";;
    255)
       echo  "$CurrntTime $hostTarget: SSH Authentication Failed";;
    2)
       echo  "$CurrntTime $hostTarget: SSH connection is Down";;
    1)
       echo  "$CurrntTime $hostTarget: SSH Authentication Failed";;
    *)
       echo  "$CurrntTime $hostTarget: SSH connection is Down";;
    esac
}

echo "$(date) Starting the SSH Connection Check...."

read -rsp $'Please Enter password below: ' SSHPASS
export SSHPASS

HostList=$(<~/host2)

CurrntTime=$(date +'%m/%d/%Y %T')

for MachineName in ${HostList}; do
   SSH_Connection "${MachineName}" &
done
# wait for all outstanding background jobs to complete before continuing
wait
# at last clear the exported variable containing the password
unset SSHPASS

I did not move the CurrentTime=$(..) inside the function. When you want to see the date/time after the ssh is finished, then you will need to store the exit code in a variable:
...
sshpass -e ssh ...
retval=$?
CurrentTime=$(date +'%m/%d/%Y %T')
case ${retval} in ...


Tree Implementation in Java We are implementing a Tree structure and came up with two possible Tree Implementations. Implementation-1 is a straight forward class while Implmentation-2 uses a nested Static Class.

  My Question to reviewers is what are the advantages/disadvantages of
  each of the methods below and what is the preferable way ?

Tree Implementation -1

 public class TreeNode<T> implements Serializable, Iterable<TreeNode<T>> {
      private static final long serialVersionUID = 1L;

      private T name;
      private TreeNode<T> parent;
      private List<TreeNode<T>> children;

    public TreeNode(T data) {
        this.name = data;
        this.children = new ArrayList<TreeNode<T>>();
    }

    public TreeNode<T> addChild(T child) {
        TreeNode<T> childNode = new TreeNode<T>(child);
        childNode.parent = this;
        this.children.add(childNode);
        return childNode;
    }

 }

Tree Implementation-2: Here, we use a Static Nested Class

 public class Node {

    public static class TreeNode1<T>  implements Serializable {

            private static final long serialVersionUID = 1L;

            private String name;
            private TreeNode1<T> parent;
            private List<TreeNode1<T>> children;

            public TreeNode1(String name) {
                this.name = name;
            }

            public void addChild(TreeNode1<T> node) {
                if (children == null)
                    children = new ArrayList<TreeNode1<T>>();
                children.add(node);
                 node.parent = this;
            }

          }
    }

I don't see any reason for the added complexity of the second implementation. The Node class doesn't provide any methods or fields nor does it serve as a base class for your TreeNode1. It just makes naming the tree nodes worse: TreeNode is definitely a better name than Node.TreeNode1.

So in version 2 you end up with two classes:

The outer class Node does nothing useful.
The node class Node.TreeNode1 doesn't do anything a non-nested class couldn't do as well.

Even if you embed the TreeNode1 class into a more functional outer class, there isn't much benefit from static nesting.

So I'd definitely prefer implementation 1.

A nested class can be a good solution in cases where access to the enclosing class is useful, but I'd hide it from the API user, e.g. by exposing a Node interface which is internally implemented by the nested class.


One time property reading I have following code that is used of one time skipping some functionality(actually do something only one time until SkipSomeStuff is true):

private bool _skipSomeStuff;
public bool SkipSomeStuff
{
  set
  {
    _skipSomeStuff = value;
  }
  get
  {
    if (_skipSomeStuff)
    {
        _skipSomeStuff = false;
        return true;
    }
    return false;
  }
}

Is such construction ok to use, or I should change it with something?

It is very confusing to set a property just to discover that it returns something else just after you set it. Properties are supposed to keep the value you give them. (At least logically)

And since your SkipSomeStuff property is public, you have no guarantee you won't access that property by accident before the intended recipient accesses it, and the stuff isn't skipped after all.

How about keeping the skipSomeStuff member, but set it using a method called SkipStuffNextTime() or something like that? Nobody will misunderstand the intent of your code that way, and only the code it's relevant for will access and reset it.

private bool skipSomeStuff = false;

public void SkipStuffNextTime() {
    skipSomeStuff = true;
}

public void DoSomeStuff() {
    if (skipSomeStuff) {
        skipSomeStuff = false;
        return;
    }
    // do the stuff
}


JavaScript: A drawing program in OOP I made a little drawing program in OOP.
The game consists of a canvas which is basically a 2d array where the user can draw various shapes on it. There can be multiple shapes overlapped on top of each other at the same spot. The shape that was drawn the last would appear on the top of the canvas and that is what gets printed out when we print the canvas - only those shapes at the top would be printed out.
Here I have two classes Rectangle and Canvas

class Rectangle {
  constructor(positions) {
    this.positions = positions
    this.character = Symbol()
  }

  move(dx, dy) {
    for (const position of this.positions) {
      const nx = position[0] + dx
      const ny = position[1] + dy
      position[0] = nx
      position[1] = ny
    }
  }
}

class Canvas {
  constructor(size) {
    this.canvas = Array.from({ length: size }, () =>
      Array.from({ length: size }, () => null)
    )
  }

  draw(rectangle) {
    const { positions, character } = rectangle
    for (const [row, col] of positions) {
      if (this.canvas[row][col] === null) this.canvas[row][col] = [character]
      else this.canvas[row][col].push(character)
    }
  }

  remove(rectangle) {
    const { positions, character } = rectangle
    if (positions.length === 0) return
    for (const position of positions) {
      const list = this.canvas[position[0]][position[1]]
      list.splice(list.indexOf(character), 1)
    }
  }

  print(palette) {
    let rows = ''

    for (let i = 0; i < this.canvas.length; i++) {
      for (let j = 0; j < this.canvas[i].length; j++) {
        if (this.canvas[i][j] === null || this.canvas[i][j].length === 0) {
          rows += '*' // empty
        } else {
          rows += palette[this.canvas[i][j][this.canvas[i][j].length - 1]]
        }
      }

      rows += '\n'
    }
    console.log(rows)
  }

  moveToTop(rectangle) {
    this.remove(rectangle)
    this.draw(rectangle)
  }

  drag(startPosition, endPosition, map) {
    const list = this.canvas[startPosition[0]][startPosition[1]]
    if (list?.length === 0) return

    const character = list[list.length - 1]

    const rectangle = map[character]
    const dx = endPosition[0] - startPosition[0]
    const dy = endPosition[1] - startPosition[1]
    this.remove(rectangle)
    rectangle.move(dx, dy)
    this.draw(rectangle)
  }
}

And I have a map that maps the Rectangle's character to that Rectangle instance. and another object called palette which tells the canvas how that shape or pixel would look like when being printed (initially palette is part of canvas but I feel like I should decouple them so canvas only holds the state of the current canvas and is not opinionated on how the shapes should look like when they are printed out).
const rectangle1 = new Rectangle(
  [
    [0, 1],
    [1, 2],
    [0, 2],
  ]
)
const rectangle2 = new Rectangle(
  [
    [0, 0],
    [0, 1],
    [1, 0],
  ]
)
const rectangle3 = new Rectangle([[0, 0]])

const palette = {
  [rectangle1.character]: 'A',
  [rectangle2.character]: 'B',
  [rectangle3.character]: 'C',
}

const map = {
  [rectangle1.character]: rectangle1,
  [rectangle2.character]: rectangle2,
  [rectangle3.character]: rectangle3,
}

here is the link to a live demo
Please feel free to give me any feedback. Specifically, I would love to know:

if there is a better to approach the OO design here? Did I do a good job at separating the concerns here? Does every class have the methods here?
I think I need some help to refactor the map and palette. Specifically map, it is manually constructed right now. I am not sure if I need to create two classes for them.
Is there any alternatives to approach implementing such a canvas? What are some of the pros and cons?

Question responses

if there is a better to approach the OO design here?

It seems okay. As was pointed out in answers to your other recent questions there currently is no use of OO features like inheritance.

Did I do a good job at separating the concerns here?

The separation seems okay.
Suggestions
Simplify move method
The Rectangle method move is currently implemented as this:

move(dx, dy) {
    for (const position of this.positions) {
      const nx = position[0] + dx
      const ny = position[1] + dy
      position[0] = nx
      position[1] = ny
    }
  }

Variables nx and ny are only used once. The method could greatly be simplified:
position[0] += dx;
position[1] += dy;

functional approach to print
In the Canvas method print two for loops are used. A functional approach, similar to how the canvas is created in the constructor, would allow fewer lines and the variable rows could be assigned in one statement, which would allow const to be used, though this would not be wise to change if performance is a major concern.
print(palette) {
    const rows = this.canvas.map(column => column.map(cell => {
      if (cell === null || !cell.length) {
        return '*';
      }
      return palette[cell[cell.length - 1]];
    }).join('') + "\n" ).join('');

check parameter types
Since JavaScript is loosely typed, there isn’t much support for ensuring a method is called with parameters of certain types. However, if a method expects to be called with certain types of arguments (e.g. Canvas::draw() and ::remove() expect a Rectangle instance) then the type of those arguments can be checked - e.g. using the typeof keyword. If an argument doesn’t match an expected type then perhaps an Error should be thrown.


Is this login page safe and can it be optimized? <?php
$page = 'Login';
session_start();
include 'header.php';
$user_error = '';
$pass_error = '';
$login_error = '';

if(isset($_SESSION['username'])){
header('location: control-painel.php');
}
if(isset($_POST['login'])){
$username = $_POST['username'];
$password = $_POST['password'];
$cost = '11';
$salt = 'Cf1f11ePArKlBJomM0F6aJ';
$password_hash = crypt($password, '$2a$' . $cost . '$' . $salt . '$');
$id = 0;

if(empty($username)){
    $user_error = 'Please insert a username';
}
if(empty($password)){
    $pass_error = 'Please insert a password';
}
elseif(!empty($username) && !empty($password)){
    $stmt = $mysqli -> prepare('SELECT id FROM user WHERE username = ? AND password = ?');
    $stmt -> bind_param('ss', $username, $password_hash);
    $stmt -> execute();
    $stmt -> bind_result($id);
    $stmt -> fetch();

    if($id){
        $_SESSION['username'] = $username;
        $_SESSION['email'] = $email;
        header('location: control-painel.php');
        die();
    }
    else{
        $login_error = 'User or Password invalid';
    }
}
}
?>

    <?php echo $user_error; ?>

    <?php echo $pass_error; ?>

    <?php echo $login_error; ?>

        <form method="post">
            Username:
            <input type="text" name="username" class="text bradius">
            Password:
            <input type="password" name="password" class="text bradius">
            <input type="submit" class="submitbutton bradius" name="login" value="Login">
        </form>

<?php
include "footer.php";
?>

The first thing I spotted was this: control-painel.php. I'm not sure if that's an error with your page name or code, but whichever it is needs to be fixed.

  Can it be optimized?

The best optimisation is only doing what's necessary. When checking the login, you don't check whether they've even been specified until after you hash it, consider reordering it like so:

$username = $_POST['username'];
$password = $_POST['password'];
if(empty($username))
{
    $user_error = 'Please insert a username';
}
if(empty($password))
{
    $pass_error = 'Please insert a password';
}
elseif(!$user_error) //Password can't be empty here, don't need to check again
{
    $cost = '11';
    $salt = 'Cf1f11ePArKlBJomM0F6aJ';
    $password_hash = crypt($password, '$2a$' . $cost . '$' . $salt . '$');
    $id = 0;
    $stmt = $mysqli->prepare('SELECT id FROM user WHERE username = ? AND password = ?');
    $stmt->bind_param('ss', $username, $password_hash);
    $stmt->execute();
    $stmt->bind_result($id);
    $stmt->fetch();
    if($id)
    {
        $_SESSION['username'] = $username;
        $_SESSION['email'] = $email;
        header('location: control-painel.php');
        die();
    }
    else
    {
        $login_error = 'User or Password invalid';
    }
}

So the hashing is only performed when you have a username and password.


Multiple Pandas Ranking Operations within a Loop - Better Optimization and Performance I have implemented the following code which works as intended. However, I would like to improve my code in terms of performance and efficiency
Code in Question
import pandas as pd
from scipy.stats import norm

# data frame of length 40,000 rows, containing 25 columns
for indx in df.index:
    matrix_ordered_first = df.loc[indx].rank(method='first',na_option='bottom')
    matrix_ordered_avg = df.loc[indx].rank(method='average', na_option='bottom')
    matrix_ordered_first.loc[df.loc[indx] == 0] = matrix_ordered_avg
    matrix_computed = norm.ppf(matrix_ordered_first / (len(df.columns) + 1))
    df.loc[indx] = matrix_computed.T

A peak of the dataframe
Here is a part view of my dataframe df:
s            s1     s2           s3         s4      ...     s21        s23        s24  s25
0            NaN   5.406999   5.444658   4.640154  ...  4.633389   5.517850       NaN  6.121492
1            NaN   2.147866   1.758245   1.274754  ...  1.465129   1.200157       NaN  1.789203
2       2.872652   5.492498   2.547415   3.754654  ...  3.686420   1.540947  4.405961  1.715685
3            NaN  46.316837  27.197062  72.910797  ...       NaN  46.812153       NaN       NaN
4       1.365775   1.329316   1.852473   1.208155  ...  1.489296   1.313321  1.462968  1.249645

[5 rows x 25 columns]

Explanation
The code above is the part of a long python script in which this part runs slower than the other parts of the program. So what I am trying to do in the above code is to iterate over the data frame in a row-wise fashion. Then, for each row I have to perform a chain of pandas ranking operations followed by a statistical test equivalent to the "One-tail test”.Finally, transpose the matrix which will then be fed as a row for the data frame.
How can I improve this block of code in terms of efficiency, speed, and performance?
On a separate note, I not experienced in pandas so my code may seem amateur and for that I kindly seek your guidance.
Thank you so much in advance,

A basic principle of performance optimization in pandas is to avoid for-loops and vectorize as much computation as possible.
Here is one version of improved code:
order_df = df.rank(axis=1, method="first", na_option="bottom")
order_df_avg = df.rank(axis=1, method="average", na_option="bottom")
order_df.where(df.astype(bool), order_df_avg, inplace=True)
matrix_computed = norm.ppf(order_df / (len(df.columns) + 1))
df[:] = matrix_computed

One potential further improvement is to compute only the orders that need to be updated rather than order_df_avg.
order_df = df.rank(axis=1, method="first", na_option="bottom")
mask = (df == 0)
orders_to_update = np.broadcast_to(np.expand_dims(order_df[mask].mean(axis = 1), 1), df.shape)
order_df.mask(mask, orders_to_update, inplace=True)
matrix_computed = norm.ppf(order_df / (len(df.columns) + 1))
df[:] = matrix_computed


Unit testing a bash script manipulating the working directory path bd is a bash script to conveniently jump multiple directory levels up from the current working directory instead of a tedious and possibly inaccurate cd ../../../..

Say, for example, that you are in the directory /home/user/project/src/org/main and you want to jump to project. Instead of counting the necessary levels to jump and typing cd ../../.., you can do bd -s pro (where -s is to match the start of the directory name, in this example "pro" for "project").

I came across this script in another question, and looking at the github repository I decided to jazz it up a little. But before going ahead and refactoring it, I wanted to make sure I'm not breaking anything, so I needed some sort of unit tests as a sane baseline first.

This is the main function in the bd script under testing,
and this is not for code review,
I'm including it here only for your reference in case it helps in some way:

# NOT for code review: the reference implementation under test
newpwd() {
  OLDPWD=$1
  if [ "$2" = "-s" ]
  then
    NEWPWD=`echo $OLDPWD | sed 's|\(.*/'$3'[^/]*/\).*|\1|'`
    index=`echo $NEWPWD | awk '{ print index($0,"/'$3'"); }'`
  elif [ "$2" = "-si" ]
  then
    NEWPWD=`echo $OLDPWD | sed 's|\(.*/'$3'[^/]*/\).*|\1|I'`
    index=`echo $NEWPWD | awk '{ print index(tolower($0),tolower("/'$3'")); }'`
  else
    NEWPWD=`echo $OLDPWD | sed 's|\(.*/'$2'/\).*|\1|'`
    index=`echo $NEWPWD | awk '{ print index($1,"/'$2'/"); }'`
  fi
}

What I'd like reviewed is my implementation of "unit tests" to verify the behavior of the newpwd function inside the bd script:

#!/bin/bash

# loading the "newpwd" function from the bd script, a necessary ugliness for now
. bd nonexistent > /dev/null

success=0
failure=0
total=0

assertEquals() {
    ((total++))
    expected=$1
    actual=$2
    if [[ $expected = $actual ]]; then
        ((success++))
    else
        ((failure++))
        echo "Assertion failed: $expected != $actual" >&2
        caller 0
        echo
    fi
}

sample=/home/user/project/src/org/main/site/utils/file/reader/whatever

# test run with no args
newpwd $sample
assertEquals $sample $NEWPWD
assertEquals 0 $index

# test run with exact match
newpwd $sample src
assertEquals /home/user/project/src/ $NEWPWD
assertEquals 19 $index

# test run with prefix but no -s so not found
newpwd $sample sr
assertEquals $sample $NEWPWD
assertEquals 0 $index

# test run with prefix found
newpwd $sample -s sr
assertEquals /home/user/project/src/ $NEWPWD
assertEquals 19 $index

# test run with prefix not found because case sensitive
newpwd $sample -s Sr
assertEquals $sample $NEWPWD
assertEquals 0 $index

# test run with prefix found thanks to -si
newpwd $sample -si Sr
assertEquals /home/user/project/src/ $NEWPWD
assertEquals 19 $index

sample='/home/user/my project/src'

# test run with space in dirname
newpwd "$sample" -s my
assertEquals '/home/user/my project/' "$NEWPWD"
assertEquals 11 $index

red='\e[0;31m'
green='\e[0;32m'
nocolor='\e[0m'

echo
[[ $failure = 0 ]] && printf $green || printf $red
echo "Tests run: $total ($success success, $failure failed)"
printf $nocolor
echo

My questions:

Is this easy to read? If not, how to improve it?
Is this a good way to test stuff in Bash? Is there a better way? I'd rather not add external dependencies to the project though, I'm looking for something ultra-light, or a technique
Are there any corner cases I missed?
Any other improvement ideas?

Just 2 cents:

I'd quote the right hand side of the [[ ... = ... ]] to avoid pattern interpretation. Cf.

x=123
y=*2*
[[ $x =  $y  ]] && echo Equal without quotes
[[ $x = "$y" ]] || echo Not equal with quotes

I like naming the tests. I'd probably add a third parameter to assertEquals and include it in the report. It also improves readability. Inspired by Test::More for Perl.


Make a command line application with parameter I made a command line application which uses parameters. So I try to make a system like the linux command.
So I have 4 parameters, initialized to a default value, used if wrong or none parameter are given. The user can modify those parameter by calling associate options "-options".

   public static void main(String [] args) {
    String locParameter = "centralOffice";
    String dirResult = "//home//"+System.getProperty("user.name")+"//Documents//ResultMyApp";
    int periodParameter = 24;
    String counterParameter = "ALL";
    int i = 0;

    for (String s: args) {
        switch (s) {
            case "-o":  if (args[i+1].equalsIgnoreCase("remote")){
                            locParameter = "remoteOffice";
                            break;
                        } else if (args[i+1].equalsIgnoreCase("central")){
                            locParameter = "centralOffice";
                            break;
                        } else {
                            System.err.println("Unknow parameter for location, default value: centralOffice");
                            break;
                        }
            case "-p":  dirResult = args[i+1];
                        break;
            case "-kpi": counterParameter = args[i+1].toUpperCase();
                        break;
            case "-t":  try {
                            periodParameter = Integer.parseInt(args[i+1]) ;
                        } catch(NumberFormatException e){
                            System.err.println("Error parsing parameter, default value 24 hours");
                        } finally {
                            break;
                        }
        }
        i++;
    }

    System.out.println("parameter choosed: " + locParameter + ", " + periodParameter + ", " + counterParameter );}

So I am wondering, is this ok? Of course it works but maybe it is not the easiest way to do it or the most optimized code.

Parameter validation is sorely missing. For example, if you call this with -o as last argument (no more arguments), the program will crash with an array index out of bounds exception and a stack trace. Getting a lengthy stack trace for invalid output is not very UNIX-y ;-)

The finally in the exception handling is not necessary and a bit confusing. I suggest simply moving the break after the catch.

The indenting is a bit strange. I suggest to use an IDE to reformat nicely.

In any case, instead of worrying your own, I suggest using a library. Recently I tried Argparse4j, and I'm happy to say I really like it, works very well for me.


Reverse a linked list recursively This recursive calls reverseList on each node of the linked list. When returned from the function call, it changes the next link to the previous node.

public static Node reverseList(Node curNode , Node prevNode , Node reverseListHead)
{
    //empty list
    if(curNode == null)
    {
        return null;
    }
    //node with single element
    if(curNode.next == null)
    {
        curNode.next = prevNode;
        reverseListHead = curNode;
        return reverseListHead;
    }
    reverseListHead = reverseList(curNode.next, curNode, reverseListHead);
    curNode.next = prevNode;
    return reverseListHead;
}

Only one argument needed
First of all, your third argument reverseListHead should be a local variable because it is never used before being set.
Next, if you change your algorithm slightly, you don't need the second argument either:
public static Node reverseList(Node head)
{
    // Empty list or list with one element.
    if (head == null || head.next == null) {
        return head;
    }

    Node second = head.next;
    Node reversedList = reverseList(second);

    // The second node is now the tail of the reversed list,
    // so when we append head to it, head becomes the new tail.
    second.next = head;
    head.next   = null;

    return reversedList;
}


Can someone review my implementation of a Queue? So here is my Queue interface:

public interface Queue<E> {
    /**
     * Add an item to the rear of the queue.
     *
     * @param item
     *            Item to be added.
     */
    public void enqueue(E item);

    /**
     * Remove and return the item at the front of the queue.
     *
     * @return The item that was removed.
     */
    public E dequeue();

    /**
     * @return The item at the front of the queue.
     */
    public E frontValue();

    /**
     * @return How many elements are in the queue.
     */
    public int length();

    /**
     * Remove all the items in the queue.
     */
    public void clear();
}

And here is my actual implementation:

public class LinkedQueue<E> implements Queue<E> {
    private SinglyLinkedListNode<E> front;
    private SinglyLinkedListNode<E> rear;
    private int size;

    /**
     * Create a new LinkedQueue object.
     */
    public LinkedQueue() {
    this.init();
    }

    private void init() {
    // only one object is constructed
    this.front = this.rear = new SinglyLinkedListNode<E>(null, null);
    this.size = 0;
    }

    @Override
    public void enqueue(E value) {
    this.rear.setNextNode(new SinglyLinkedListNode<E>(value, null));
    this.rear = this.rear.getNextNode();
    this.size++;
    }

    @Override
    public E dequeue() {
    if (this.size == 0) {
        throw new IllegalStateException("In method dequeue of class "
            + "LinkedQueue the linked queue is empty can cannot"
            + " be dequeued");
    }

    E item = this.front.getNextNode().getValue();
    this.front.setNextNode(this.front.getNextNode().getNextNode());

    if (this.front.getNextNode() == null) {
        this.rear = this.front;
    }
    this.size--;
    return item;
    }

    @Override
    public E frontValue() {
    if (this.size == 0) {
        throw new IllegalStateException("In method frontValue of class "
            + "LinkedQueue the linked queue is empty");
    }
    return this.front.getNextNode().getValue();
    }

    @Override
    public int length() {
    return this.size;
    }

    @Override
    public void clear() {
    this.init();
    }

    /**
     * Creates a easy to read String representation of the linked queue's
     * contents.
     *
     * Example: < 1 2 3 4 5 6 >
     */
    public String toString() {
    StringBuilder linkedQueueAsString = new StringBuilder();
    linkedQueueAsString.append("< ");
    System.out.println("front.getNextNode: " + this.front.getNextNode());
    for (SinglyLinkedListNode<E> node = this.front.getNextNode(); node != null; node = node.getNextNode()) {
        linkedQueueAsString.append(node.getValue());
        System.out.println(node.getValue() + ",");
        linkedQueueAsString.append(" ");
    }
    linkedQueueAsString.append(">");
    return linkedQueueAsString.toString();
    }
}

You should not name your interface Queue, since java.util.Queue already exits.

Also it would be helpful if you post the source of SingleLinkedListNode.

The source looks fine to me, you could however add two methods like (append/en-queue, remove/de-queue):

SingleLinkedListNode<E> append(SingleLinkedListNode<E> newNode) that will replace

 this.rear.setNextNode(new SingleLinkedListNode<E>(value, null));
 this.rear = this.rear.getNextNode();

and returns the next node.


Use parameter to filter results in a database without directly allowing users to access database So background context:

I wrote this script for a personal project that holds a large amount of items, I already display and am able to filter the results based on its country/location. I am a student PHP coder and would like constructive criticism on my thought process/code.

Thought Process:

I decided to code my country filter script as the countries are from a pre-defined list in my database (table: countries) and for extra safety precautions I wanted to ensure the end-user never gains direct access to the database queries - as from my understanding this is where security issues can become apparent. Essentially, the idea is that I grab a list of the countries from the database and store it in an array, then I compare that list to the users input. If there is a match then filter the results as such, if not change their input to NULL which produces a "No Result" message further on.

Table: countries

+----+-------------+-------------+
| id | name        | slug        |
+----+-------------+-------------+
| 1  | Australia   | australia   |
+----+-------------+-------------+
| 2  | New Zealand | new-zealand |
+----+-------------+-------------+

Table: items

+----+----------+-----------------------------------+-----------+---------+
| id | name     | description                       | countryid | suspend |
+----+----------+-----------------------------------+-----------+---------+
| 1  | Item One | description of Item One           | 1         | 0       |
+----+----------+-----------------------------------+-----------+---------+
| 2  | Item Two | description of suspended Item Two | 2         | 1       |
+----+----------+-----------------------------------+-----------+---------+

PHP Code:

if(isset($_GET['l'])) {
$location = strip_tags($_GET['l']);
$sql = "SELECT slug FROM countries";
$countries = $db->prepare($sql);
$countries->execute();
$getCountryList = $countries->fetchAll(PDO::FETCH_COLUMN); // Get all country slugs
if(in_array($location, $getCountryList)) { // Match country slugs to inputted country - this ensures the user input never touches the database >:)
  $sql = "SELECT countryid, name FROM countries WHERE slug = :slug LIMIT 1";
  $matchCountry = $db->prepare($sql);
  $matchCountry->execute(array(':slug' => $location));
  $countryDetails = $matchCountry->fetch(PDO::FETCH_ASSOC);
  $location = $countryDetails['countryid']; // The ID so I can search the items
  $locationName = $countryDetails['name']; // This now equals the nice to read name e.g "Australia"
} else {
  $location = '';
  $_GET['l'] = '';
}
}

The snippet that filters the items:

if(isset($_GET['l']) && $_GET['l'] != '') {
                // Location
                $sql = "SELECT * FROM items WHERE items.suspend = 0 AND items.countryid = :countryid GROUP BY items.id";
                $results = $db->prepare($sql);
                $results->execute(array(':countryid' => $location));
}

I hope I have given all the information needed! I'd like to know specifically if this extreme measure is over the top and if there is a better way to achieve such functionality.

It's completely unnecessary to execute the request SELECT slug FROM countries only to verify the user input before running SELECT countryid, name FROM countries WHERE slug = :slug LIMIT 1.
You can safely use the user input in the second query directly.
This is thanks to using prepared statements.
Even if the user input is malicious, PDO prevents SQL injection.

In other words, the first part of your snippet could be simply this:

if (isset($_GET['l'])) {
    $location = strip_tags($_GET['l']);
    $sql = "SELECT countryid, name FROM countries WHERE slug = :slug LIMIT 1";
    $matchCountry = $db->prepare($sql);
    $matchCountry->execute(array(':slug' => $location));
    $countryDetails = $matchCountry->fetch(PDO::FETCH_ASSOC);
}

If your main goal is to do SELECT * FROM items,
then if items.countryid is indexed, and countries.countryid is indexed,
then it would be best to write a single JOIN:

SELECT items.* FROM items JOIN countries USING (countryid)
WHERE items.suspend = 0 AND countries.slug = :slug GROUP BY items.id

This optimizes performance with the fewest necessary queries, without compromising safety.

However, it's recommended to avoid SELECT * queries, and always specify the fields you want to fetch explicitly.


Apply function to array of decimals I am writing some code to transform an array of decimals so that they are discounted over time.

I initially wrote the following code:

private decimal[] deflateArray(decimal[] items, decimal deflateRate)
{
    decimal[] deflatedItems = new decimal[items.Count()];

    decimal factorAdjustment = 1;

    for (int i = 0; i < items.Length; i++)
    {
        deflatedItems[i] = items[i] / factorAdjustment;
        factorAdjustment = factorAdjustment * (1 + deflateRate);
    }

    return deflatedItems;
}

Then I realised that instead of having the factorAdjustment variable, I can just use to the power of:

private decimal[] deflateArray(decimal[] items, decimal deflateRate)
{
    decimal[] deflatedItems = new decimal[items.Count()];

    for (int i = 0; i < items.Length; i++)
    {
        deflatedItems[i] = items[i] / (decimal)Math.Pow((double)(1 + deflateRate), i);
    }

    return deflatedItems;
}

As Math.Pow() only accepts and returns doubles, I think it's easier to create a wrapper function for it:

private decimal PowerOf(decimal x, decimal y)
{
    return (decimal)Math.Pow((double)x, (double)y);
}

and then simplify:

private decimal[] deflateArray(decimal[] items, decimal deflateRate)
{
    decimal[] deflatedItems = new decimal[items.Count()];

    for (int i = 0; i < items.Length; i++)
    {
        deflatedItems[i] = items[i] / PowerOf(1 + deflateRate, i);
    }

    return deflatedItems;
}

Then I realised that I really just want to transform the array and don't need a second array:

private void deflateArray(decimal[] items, decimal deflateRate)
{
    for (int i = 0; i < items.Length; i++)
    {
        items[i] = items[i] / PowerOf(1 + deflateRate, i);
    }
}

and then I can simplify this to:

private void deflateArray(decimal[] items, decimal deflateRate)
{
    for (int i = 0; i < items.Length; i++)
    {
        items[i] /= PowerOf(1 + deflateRate, i);
    }
}

so my final version is:

private void deflateArray(decimal[] items, decimal deflateRate)
{
    for (int i = 0; i < items.Length; i++)
    {
        items[i] /= PowerOf(1 + deflateRate, i);
    }
}

private decimal PowerOf(decimal x, decimal y)
{
    return (decimal)Math.Pow((double)x, (double)y);
}

I'm sure it can probably done in a one-liner by someone who is cleverer than me. Which version is the most easily understood?

I am not really worried about the performance as the array of items will only ever have 100 or 200 items and it's never done in a loop.

Arguably a better variant for DeflateArray:

private decimal[] DeflateArray(decimal[] items, decimal deflateRate)
{
    return items
        .Select((item, index) => item /= PowerOf(1 + deflateRate, index))
        .ToArray();
}

It's better in a way it helps avoiding OBOE.

And by the way, another thing you can do is define it as an extension method in a separate static class, and will be able to invoke it as myArrayOfDecimals.DeflateArrayWithRate(1.5m):

public static class DecimalArrayExtensions
{
    public static decimal[] DeflateArrayWithRate(this decimal[] items, decimal deflateRate)
    {
        return items
            .Select((item, index) => item /= PowerOf(1 + deflateRate, index))
            .ToArray();
    }

    private static decimal PowerOf(decimal x, decimal y)
    {
        return (decimal)Math.Pow((double)x, (double)y);
    }
}


Optimizing minHeap implementation I have implemented a java class for minHeap but it is not working as fast as I want . I want some help to change it to work faster in insertion or deleting the minimum node.

Here is my implementation:

public class MinHeap {
    private static final int d = 2;
    private int heapSize;
    private long[] heap;

    MinHeap(int capacity) {
        heapSize = 0;
        heap = new long[capacity + 1];
        Arrays.fill(heap, -1);
    }

    boolean isEmpty() {
        return heapSize == 0;
    }

    boolean isFull() {
        return heapSize == heap.length;
    }

    private int parent(int i) {
        return (i - 1) / d;
    }

    private int kthChild(int i, int k) {
        return d * i + k;
    }

    void insert(long x) {
        if (isFull())
            throw new NoSuchElementException("Overflow Exception");
        heap[heapSize++] = x;
        bubbleUp(heapSize - 1);
    }

    long findMin() {
        if (isEmpty())
            throw new NoSuchElementException("Underflow Exception");
        return heap[0];
    }

    void deleteMin() {
        delete(0);
    }

    void delete(int ind) {
        if (isEmpty())
            throw new NoSuchElementException("Underflow Exception");
        heap[ind] = heap[heapSize - 1];
        heapSize--;
        bubbleDown(ind);
    }

    private void bubbleUp(int childInd) {
        long tmp = heap[childInd];
        while (childInd > 0 && tmp < heap[parent(childInd)]) {
            heap[childInd] = heap[parent(childInd)];
            childInd = parent(childInd);
        }
        heap[childInd] = tmp;
    }

    private void bubbleDown(int ind) {
        int child;
        long tmp = heap[ind];
        while (kthChild(ind, 1) < heapSize) {
            child = minChild(ind);
            if (heap[child] < tmp)
                heap[ind] = heap[child];
            else
                break;
            ind = child;
        }
        heap[ind] = tmp;
    }

    private int minChild(int ind) {
        int bestChild = kthChild(ind, 1);
        int k = 2;
        int pos = kthChild(ind, k);
        while ((k <= d) && (pos < heapSize)) {
            if (heap[pos] < heap[bestChild])
                bestChild = pos;
            pos = kthChild(ind, k++);
        }
        return bestChild;
    }

}

Thanks in advance for your help!

Without more information what, when asked for a reason for two "time limit exceeded" results, motivated your teacher to mention the possibility to "get" a faster minHeap, you deprive yourself of getting more useful answers. See How to get the best value out of Code Review - Asking Questions
First and foremost, just what is the task timed?
I can't see anything seriously detrimental in your implementation:
I guess a bigger task than, say, handle a batch of insertions and min extractions gets timed, in need of an algorithmic improvement more likely than any tuning.

That said, there are principles like YAGNI, KISS and DRY.
Let me start with the bright side of YAGNI - You Are Gonna Need It:

Documentation. The type that does not get separated from the code:
Documentation in the code

Otherwise, the code presented looks a pretty decent beginner's stab at coding a heap.
Catch:

There are omissions from the deletes:
• not relevant when using an array of primitives as presented in the question:
 Failing to set the reference at the index deallocated to something innocent (as null) does not immediately impede garbage collection, as it gets copied to a lower index. Enter the next delete…

bug in delete(int d):
It replaces element atD with formerLast and restores the heap relations in formerLast and its descendants.
But what about its ancestors?
Not to worry if
1) it ends up at an index abode d or
a) it was a descendant of atD: none of those has higher priority
There are many ways to handle the situation -
I'd prefer just dropping delete(int d) over just calling bubbleUp() or some elaborate handling trying to keep the number of comparisons low.

Room for improvement:

Part of Don't Repeat Yourself is using interfaces:
Define an interface for an, um, PriorityCollection<E> extends Collection<E>.
One advantage is that implementations don't get to reduce visibility of interface methods like isEmpty().
Another is inheriting documentation, too.
• have an E extractTop() return the top priority element it just removed from the Collection
• nowadays, Java interfaces can include unit test code, too.
Keep It Short&Simple meets You Ain't Gonna Need It in fixing arity at 2:
• simpler/shorter code
• fixes a naming issue in d (maximal degree?)
• maximises information in relative position of keys
• doesn't show in the interface, anyway

If and when competent (→ framework) measurements suggest the performance being a problem,
• consider returning to an array of primitives.
• If the cost of comparing elements is high, change tactics in bubbleDown(sink?): find the path along which to sink/bubble down, and look for elements that should stay in place starting from the bottom: this may almost half the number of comparisons.
• an E replaceTop(E replacement) may save considerable work
  it may be useful to additionally have the similar E addAndExtractTop(E addition)


Math calculation refactoring in JavaScript Here's the full source to a plugin I'm developing, the relevant code I want to refactor is:

function measure($element, $bubble){
    var scrollTop = $window.scrollTop(),
        viewportHeight = $window.height(),
        readableTop = $element.position().top,
        readableHeight = $element.height(),
        readableBottom = readableTop + readableHeight,
        scrollHeight = viewportHeight / readableHeight * viewportHeight,
        progress = (scrollTop - readableTop) / (readableHeight - viewportHeight),
        total = getTotalMinutes($element),
        remaining = Math.ceil(total - (total * progress)),
        distanceProgress = (scrollTop - readableTop) / readableHeight,
        distanceLiteral = readableTop + distanceProgress * readableHeight + viewportHeight / 2 - $bubble.height() / 2,
        distance = Math.max(readableTop, Math.min(distanceLiteral, readableBottom));

    return {
        remaining: remaining,
        progress: progress,
        distance: distance
    };
}

How might I make this calculation more understandable? I'm not terribly good at math, so I got there by trial and error, pretty much.

Now I'd like to refactor into something more readable or at least trim the repetitive nonsense, but I have no clue where to start.

Could you lend me a hand or give me any advice on how to tackle this?

Strictly in terms of code readability, you want to break down a function like this into it's core parts. Additionally, to reduce repetitive behavior you could make it more of a prototype and use some object oriented style to get to your result without a block of math.

Things to Consider

Law of Demeter -
Passing into your function only what you need and not entire objects.
Breaking your function into segments, why not use methods to
systematically get to your answer? Easily readable/maintainable.
Reduce the amount of variable creation, you dont need to set each
value to a new variable when one is already passed in from the
function arguments. Law of Demeter helps here, too.
Keeping your code expandable.

Here's an example of a function that follows the three above rules, a simple example that leaves you to put in your mathematics.

Example

var measure = function(elementHeight, elementPositionTop, bubbleHeight) {
    var remaining = 0, progress = 0, distance = 0;

    return {
        init: function() {
            measure.buildRemaining();
            measure.buildProgress();
            measure.buildDistance();
        },
        buildRemaining: function() {
            remaining = elementHeight * bubbleHeight;
        },
        buildProgress: function() {
            progress = remaining / 2.5;
        },
        buildDistance: function() {
            distance = (elementPositionTop - progress) + remaining;
        },
        result: function() {
            measure.init();
            return {
                remaining: remaining,
                progress: progress,
                distance: distance
            }
        }
    }
}($element.height(), $element.position().top, $bubble.height());

Accessing the Function

measure.result();

Returns

Object { remaining=5850, progress=2340, distance=3582 }


Formatting a datetime string in the YYYYMMDD format I'm working with some strange APIs that requires the dates to be sent in the YYYYMMDD format.

I was thinking of doing something like this:

string date = string.Concat(DateTime.Now.Year, DateTime.Now.Month, DateTime.Now.Day);

Is there a better practice?

Yes there is: Date Formatting

var dateString = DateTime.Now.ToString("yyyyMMdd");