C# handling of images - is this best for memory usage? I am working with a system that needs to consume product images uploaded by employees and resize them / add white padding to the sides to create square images to be consumed by our e-commerce site.  I'm attempting to keep things somewhat light on the memory usage because this will live in a windows service on an azure vm.  I know there are better solutions, but that's what I'm given as a platform.  I also don't want to use 3rd party libraries.

The following code is what I have so far.  Please, pick it apart and let me know if i'm making any mistakes.

public static Image ImageToFixedSize(Image originalImage, int width, int height)
{
    Graphics graphicController = null;

    try
    {
        if (originalImage != null)
        {
            int sourceWidth = originalImage.Width;
            int sourceHeight = originalImage.Height;
            int destX = 0;
            int destY = 0;

            float percent = 0;
            float percentW = 0;
            float percentH = 0;

            percentW = width / (float)sourceWidth;
            percentH = height / (float)sourceHeight;
            if (percentH < percentW)
            {
                percent = percentH;
                destX = Convert.ToInt16((width - (sourceWidth * percent)) / 2);
            }
            else
            {
                percent = percentW;
                destY = Convert.ToInt16((height - (sourceHeight * percent)) / 2);
            }

            int destWidth = (int)(sourceWidth * percent);
            int destHeight = (int)(sourceHeight * percent);

            Bitmap tempPhoto = new Bitmap(width, height, PixelFormat.Format24bppRgb);

            tempPhoto.SetResolution(originalImage.HorizontalResolution, originalImage.VerticalResolution);
            graphicController = Graphics.FromImage(tempPhoto);
            graphicController.FillRectangle(Brushes.White, 0, 0, width, height);

            // The chances of hitting this ONE SPECIFIC COLOR are very slim
            graphicController.SmoothingMode = SmoothingMode.HighQuality;
            graphicController.PixelOffsetMode = PixelOffsetMode.HighQuality;
            graphicController.InterpolationMode = InterpolationMode.HighQualityBicubic;
            graphicController.CompositingQuality = CompositingQuality.HighQuality;

            graphicController.DrawImage(
                originalImage,
                new Rectangle(destX, destY, destWidth, destHeight),
                new Rectangle(1, 1, sourceWidth - 1, sourceHeight - 1),
                GraphicsUnit.Pixel);

            return tempPhoto;
        }
        else
        {
            return null;
        }
    }
    catch
    {
        throw;
    }
    finally
    {
        graphicController.Dispose();
    }
}

Reversing the condition of if (originalImage != null) and returning early will remove one level of indentation. The less indentation some code shows the easier to read it will become.
catch to just rethrow, althought you did it in the correct way, doesn't buy you anything. Just remove the try..catch..finally and enclose the usage of graphics with an usage block.
Althought the code is mostly named well, one could stumble over tempPhoto and can't figure out at first glance wether this object is important or just temporary.
You can cast e.g ((width - (sourceWidth * percent)) / 2) directly to int you don't need a call to Convert.ToInt16() here.
percentW and percentH are first defined and on the next lines you assign a value to them. You should do it just at the declaration to save some (superflous) lines of code.
Instead of graphicController.FillRectangle(Brushes.White, 0, 0, width, height); you should use graphicController.Clear(Color.White); which is easier to read.
Comments, when used, should state, in a clear and understandable way, why the code is written as it is. I don't get the comment // The chances of hitting this ONE SPECIFIC COLOR are very slim. Where could be a problem with the code to make this comment justified?
The code validates the Image originalImage method parameter, but allows "illegal" values for int width and int height.

Implementing most of the mentioned points (validation is for you) will lead to

public static Image ImageToFixedSize(Image originalImage, int width, int height)
{
    if (originalImage == null) { return null; }

    int sourceWidth = originalImage.Width;
    int sourceHeight = originalImage.Height;
    int destX = 0;
    int destY = 0;

    float percent = 0;
    float percentW = width / (float)sourceWidth;
    float percentH = height / (float)sourceHeight;

    if (percentH < percentW)
    {
        percent = percentH;
        destX = (int)((width - (sourceWidth * percent)) / 2);
    }
    else
    {
        percent = percentW;
        destY = (int)((height - (sourceHeight * percent)) / 2);
    }

    int destWidth = (int)(sourceWidth * percent);
    int destHeight = (int)(sourceHeight * percent);

    Bitmap fixedSizedImage = new Bitmap(width, height, PixelFormat.Format24bppRgb);
    fixedSizedImage.SetResolution(originalImage.HorizontalResolution, originalImage.VerticalResolution);

    using (Graphics graphicController = Graphics.FromImage(fixedSizedImage))
    {
        graphicController.Clear(Color.White);

        // The chances of hitting this ONE SPECIFIC COLOR are very slim
        graphicController.SmoothingMode = SmoothingMode.HighQuality;
        graphicController.PixelOffsetMode = PixelOffsetMode.HighQuality;
        graphicController.InterpolationMode = InterpolationMode.HighQualityBicubic;
        graphicController.CompositingQuality = CompositingQuality.HighQuality;

        graphicController.DrawImage(
            originalImage,
            new Rectangle(destX, destY, destWidth, destHeight),
            new Rectangle(1, 1, sourceWidth - 1, sourceHeight - 1),
            GraphicsUnit.Pixel);
    }

    return fixedSizedImage;

}


Executing a function's flow My __main__.py file has a function's flow, based on methods of each class instance, which each one has its own methods and attributes, like this:

from xlrd                              import open_workbook
from pprint                            import pprint
from rubrica_manager                   import RubricaManager
from rectified_rubrica_manager         import RectifiedRubricaManager
from rectification_spreadsheet_manager import RectificationSpreadsheetManager
from gps_manager                       import GPSManager
from sys                               import exit
from data_comparison                   import DataComparison

def main():
    '''
    Execute PostDiagnosisChecker
    '''
    rm  = RubricaManager("File1.xlsx")
    rrm = RectifiedRubricaManager("File2.xlsx")
    rsm = RectificationSpreadsheetManager("File3.xlsx")
    gm  = GPSManager("File4.xlsx")

    object_collection = [rm, rrm, rsm, gm]

    for current_spreadsheet in object_collection:
        '''
        evaluate each object if it has the method or attribute before executing
        '''
        methods_list = dir(current_spreadsheet)
        if "load_data" in methods_list.get("load_data"):
            current_spreadsheet.load_data()
        if "spreadsheet_to_array" in methods_list:
            current_spreadsheet.spreadsheet_to_array()
        if "headers_array" in methods_list:
            current_spreadsheet.headers_array
        if "headers_used_indexes" in methods_list:
            current_spreadsheet.get_used_indexes(0)
        if "remove_duplicity_by_timestamp" in methods_list:
            current_spreadsheet.remove_duplicity_by_timestamp()
        if "array_to_dictionary" in methods_list:
            current_spreadsheet.array_to_dictionary()

There are methods commonly to all of instances, some of them are not. Because of that, I thought about a way to evaluate if each instance has its respective method, but I don't know if this is the most performative way. Any ideas?

Python version 2.6.

You've got a random string inside your function, it is unhelpful, you should remove that. If, however, you meant to make a comment, you should have used the comment sign (#).

For the existence of methods, combining dir and if .. in is inneficient as lists lookups are \$\mathcal{O}(n)\$ but attribute access is usually done through an instance's __dict__. You have two possibilities: either you execute it nonetheless and handle the AttributeError if it didn't exist, or use the getattr function to retrieve the method and then you execute it. The advantages of the latter being that:

You can easily encapsulate that behaviour in a function;
You can use the third parameter of getattr to provide a default value in case the attribute doesn't exist.

Something like this should be very generic, remove functionalities as you don't need them:

def do_nothing(*args, **kwargs):
    pass

def execute_method(name, instance, *args, **kwargs):
    method = getattr(name, instance, do_nothing)
    return method(*args, **kwargs)

Going through the rest of the code, you may also allow to pass the names of the files as parameters.

Proposed improvements look like:

FILES_TYPES = [
    RubricaManager,
    RectifiedRubricaManager,
    RectificationSpreadsheetManager,
    GPSManager,
]
METHODS = [
    'load_data',
    'spreadsheet_to_array',
    'headers_array',
    'get_used_indexes',
    'remove_duplicity_by_timestamp',
    'array_to_dictionary',
]

def do_nothing(*args, **kwargs):
    pass

def execute_method(name, instance, *args, **kwargs):
    method = getattr(name, instance, do_nothing)
    return method(*args, **kwargs)

def main(filenames=('File1.xlsx', 'File2.xlsx', 'File3.xlsx', 'File4.xlsx')):
    for filename, cls in zip(filenames, FILES_TYPES):
        spreadsheet = cls(filename)
        for method_name in METHODS:
            execute_method(method_name, speadsheet)


Start, stop and restart script for a MongoDB server I have created an start, stop, restart script for a unix-mongodb-server.

It would be nice if you could look over my code and give me some helpful hints on what I could change. I tested the script under my Mac and it works.

This is also available under my GitHub account and available under GitHub -> Mongo_Start_Stop.

VERSION=1.1.2
SCRIPTNAME=$(basename "$0")
MONGOHOME=
MONGOBIN=$MONGOHOME/bin
MONGOD=$MONGOBIN/mongod
MONGODBPATH=
MONGODBCONFIG=

if [ $# != 1 ]
then
    echo "Usage: $SCRIPTNAME [start|stop|restart]"
    exit
fi

pid() {
    ps -ef | awk '/[m]ongodb/ {print $2}'
}

stopServer() {
    PID=$(pid)
    if [ ! -z "$PID" ];
    then
        echo "... stopping mongodb-server with pid: $PID"
    sudo kill $PID
    else
        echo "... mongodb-server is not running!"
    fi
}

startServer() {
    PID=$(pid)
    if [ ! -z "$PID" ];
    then
        echo "... mongodb-server already running with pid: $PID"
    else
        echo "... starting mongodb-server"
        sudo "$MONGOD" --dbpath "$MONGODBPATH" --config "$MONGODBCONFIG"
    fi
}

restartServer() {
    stopServer
    sleep 1s
    startServer
}

case "$1" in

    start) startServer
           ;;

    stop) stopServer
          ;;

    restart) restartServer
         ;;

    *) echo "unknown command"
       exit
       ;;
esac

Bug

Can you spot the bug on this line?

    PID=$(ps -ef | grep "[m]ongodb" | awk {'print $2'})

The awk script here should be {print $2},
with the curly braces included,
so that needs to be within the single-quotes:

    PID=$(ps -ef | grep "[m]ongodb" | awk '{print $2}')

Quoting path variables

It's good to make it a habit to always double-quote path variables in command statements, to protect from globbing and word splitting. Instead of:

SCRIPTNAME=$(basename $0)

Write like this:

SCRIPTNAME=$(basename "$0")

The same goes for this line:

    sudo $MONGOD --dbpath $MONGODBPATH --config $MONGODBCONFIG

Useless variable and echo

PID is not used anywhere else inside the script but here:

function isRunning {
    PID=$(ps -ef | grep "[m]ongodb" | awk '{print $2}')
    echo $PID
}

You could simplify as:

pid() {
    ps -ef | awk '/[m]ongodb/ {print $2}'
}

I also renamed the function, because pid() reflects better its purpose.

User experience

Something's odd in this condition:

rc=$(isRunning)
echo "... stopping mongodb-server with pid: $rc"
if [ ! -z "$rc" ]; then
sudo kill $rc
fi

If the process is not running $rc will be empty, and users may think the output is weird and the script is buggy.
You might want to rearrange this part,
for example if the server is not running, then say so.

Make the most of awk

Very often a grep ... | awk ... combo can be simplified,
because awk can filter all by itself. Instead of:

PID=$(ps -ef | grep "[m]ongodb" | awk '{print $2}')

You can write:

PID=$(ps -ef | awk '/[m]ongodb/ {print $2}')

Style

This is fine:

function isRunning {
    # ...
}

But the recommended style for function declarations is like this:

isRunning() {
    # ...
}

Variable initialization

This is fine:

MONGOHOME=""
MONGOBIN="$MONGOHOME/bin"
MONGOD="$MONGOBIN/mongod"
MONGODBPATH=""
MONGODBCONFIG=""

But you could write simpler as:

MONGOHOME=
MONGOBIN=$MONGOHOME/bin
MONGOD=$MONGOBIN/mongod
MONGODBPATH=
MONGODBCONFIG=

Redundant semicolon

The semicolon at the end of exit; is redundant, I suggest to remove it:

if [ $# != 1 ]
then
    echo "Usage: $SCRIPTNAME [start|stop|restart]"
    exit;
fi


Toggling a panel and the clicker text I have written some jQuery that slides open a panel on click, and changes the toggles of the clickable bit from 'Show All' to 'Hide All'.

It works, but I'm trying to figure out what would be the most efficient way to do it, as it seems kind of long to me, and some HTML is repeated.

This is the working version and a working fiddle here:

$(function() {
    //Hide panel with jQuery and create toggle button so subjects will just show if there's no javascript
$("#panelSub").hide();
$('#toggleSubjects').html('
Show All Subjects
');
    //Toggle extra subject panel
    $("#toggleSubjects").click(function(){
        $("#panelSub").slideToggle("slow");
        //Change toggle text for show / hide
            if ($.trim($('.subjectOpen').text()) === 'Show All Subjects') {
            $(this).html('
Hide All Subjects
');
            } else {
            $(this).html('
Show All Subjects
');
                }
            });
            return false;
});

Please check out this updated jsFiddle: https://jsfiddle.net/b4mp044z/5/

I've changed your JS code into :

$(document).ready(function () {
    $("#panelSub").hide();

    $("#toggleSubjects").click(function () {
        $("#panelSub").slideToggle("slow");
        $(this).find('i').toggleClass('fa-arrow-circle-down fa-arrow-circle-up')
    });
    return false;
});

and updated your HTML and CSS to hold all your structure and content. It's a good practice to keep things separated (HTML, CSS and JS)


My login with AJAX and PHP In your opinion, do I need to do all these validations? I thought to do well and stay safe but it seriously raises the question if it isn't slower.

jQuery

$(document).ready(function () {

        // jQuery validate plugin
        $('#login > form').validate({

            rules: {
                username: { required: true },
                password: { required: true }
            },

            submitHandler: function () {

                var credentials = {
                    "username": $('#username').val(),
                    "password": $('#password').val()
                };

                $.post('ajax/tryLogin.php', credentials)
                    .done(function (data) {

                        if (data.hasOwnProperty('success') &&
                            data['success']) {

                            window.location = 'catalog.php';

                        } else if (data.hasOwnProperty('message')) {

                            alert(data['message']);
                        } else {
                            alert('Communication with the server failed.');
                        }
                    })
                    .fail(function () {
                        alert('Communication with the server failed.');
                    })
            }
        });
    });

Ajax PHP

if (empty($_POST['username']) || empty($_POST['password'])) {
    $data['success'] = false;
    $data['message'] = 'Username and password are required.';

} else {

    try {
        $data['success'] = Security::TryLogin($_POST['username'], $_POST['password']);
    } catch (Exception $e) {

        $data['success'] = false;
        $data['message'] = $e->getMessage();
    }
}
header('Content-type: application/json');
echo json_encode($data);

PHP

class Security
{

    //...

    public static function TryLogin($username, $password)
    {
        $username = strtolower($username);
        $password = sha1($password . $username);

        try {
            $user = Users::FindByUsernameAndPassword($username, $password);

            if (session_id() == '') {
                session_start();
            }

            $_SESSION[self::USER_IDENTIFIER] = $user;

            return true;

        } catch (Exception $e) {
            throw new Exception('Username of password incorrect.');
        }
    }

    //...

}

Since throw interrupts the program flow, you don't need to provide an else branch. That way, you also visibly follow the good practice of using 'watchdogs':

<?php

include_once('config.php');
include_once(ROOT . 'libs/database.php');
include_once(ROOT . 'libs/models/user.php');

class Security
{

    //...

    public static function TryLogin($username, $password)
    {
        $username = strtolower($username);
        $password = sha1($password . $username);

        $conn = Database::getConnection();
        if (empty($conn)) {
            throw new Exception('The connection to the database failed.');
        }

        $result = odbc_exec($conn, '{CALL [BruPartsOrderDb].[dbo].[tryLogin]("' . $username . '", "' . $password . '")}');
        if (empty($result)) {
            throw new Exception('The execution of the query failed.');
        }

        $row = odbc_fetch_row($result);
        if (empty($row)) {
            throw new Exception('Username or password incorrect.');
        }

        if (session_id() == '') {
            session_start();
        }

        $_SESSION['user'] = new User(odbc_result($result, 'id'), $username);

        return true;
    }

    //...
}

Suddenly the code looks much more handy.


Unique dictionary values - print keys I'm working on this code for exam in an MIT Python course. It's working but I'm trying to improve it. What could I do, and what should I avoid?

aDict = {   0: 1,
     1: 2,
     2: 2,
     4: 2,
     5: 2,
            6: 3,
     7: 2}

values=list(aDict.values())
keys=list(aDict.keys())
value_copy=values[:]
uniqe=[]

m=0

for value in value_copy:
    m+=1
    if value in value_copy[m:]:
        while(values.count(value)):
            try:
                values.remove(value)
            except:
                pass

for val in values:
    uniqe.append(keys[value_copy.index(val)])

uniqe.sort()

print (uniqe)

A more efficient way (in terms of lines of code) of doing the same thing would be to use this code. The code declares a new dictionary countMap and then loops over the items in aDict. The first loop increases the entry in countMap that corresponds to the current value of aDict which is being evaluated and then increases the value in countMap if it's already been found but if it hasn't then it sets the corresponding value in countMap to 0 and then increments the value (initially setting every value to 1 or if it's already been found then incrementing it). The second loop only adds values within countMap to the list uni if there values equal 1 (if the current entry is unique).

Taken from here:

aDict = {
        0: 1,
        1: 2,
        2: 2,
        4: 2,
        5: 2,
        6: 3,
        7: 2}

countMap = {}
for v in aDict.values():
    countMap[v] = countMap.get(v,0) + 1
uni = [ k for k, v in aDict.items() if countMap[v] == 1]
print(uni)


16-bit Parity Generator I have completed a VHDL 16-bit parity generator and I would like to know if I have programmed it correctly. I have compiled it 10 times and worked out any bugs that it found. I was finally able to compile it successfully. My problem is that I am trying to run a timing simulation to make sure it will work correctly but I am not sure what I should be looking for.

The basic operation is to XOR the A and B inputs to perform an iterative process with an output of '1' as odd and an output of '0' as even. My code is written such that a basic XOR block is then added as a component of the complete parity generator.

I would like a second opinion to make sure I have written it correctly and if it will do what it is designed to do. I thank you all in advance and look forward to any input, good or bad.

Basic XOR gate block VHDL Code

library ieee;
use ieee.std_logic_1164.all;

entity xor_gate is
port(
        a : in std_logic;
        b : in std_logic;
        pari : in std_logic;
        paro : out std_logic);

    end xor_gate;

architecture behavior of xor_gate is
    begin

    paro <= (a xor b) or pari;

end behavior;

16-bit Parity Generator Code

library ieee;
use ieee.std_logic_1164.all;
entity parity_2 is

port(a: in std_logic_vector (0 to 15);
      b: in std_logic_vector (0 to 15);
      paro: out std_logic);

end parity_2;

architecture behavior of parity_2 is
        signal parry: std_logic_vector (0 to 15);

component xor_gate
            port (a,b: in std_logic;
                   pari: in std_logic;
                    paro: out std_logic);

end component;

begin

c1: xor_gate port map (a(0), b(0), '0', parry(0));
c: for i in 1 to 15 generate
c2: xor_gate port map (parry(i-1), a(i), b(i), parry(i));

end generate;

paro <= parry(15);

end behavior;

As mentioned in the comments the or part of your "xor_gate" prevents it from actually working as an xor gate to calculate bit parity.

Instead your paro signal will be '0' when a=b  and '1' when a/=b (the 16 bit vectors, not the bits within xor_gate). If that was your intended functionality, then paro <= '0' when a=b else '1'; would be simpler.

Finally, it would be much clearer and cleaner to not have a whole component for such a simple operation. The following does an xor of all the bits (a typical parity calculation) and is shorter and easier to read. If I was only going to use this calculation in one or two places, I'd absorb this whole block into those as well, as it is only 6 additional lines.

library ieee;
use ieee.std_logic_1164.all;
entity parity_2 is
  port(a: in std_logic_vector (0 to 15);
       b: in std_logic_vector (0 to 15);
       paro: out std_logic
      );
end parity_2;

architecture behavior of parity_2 is
  signal parry: std_logic_vector (0 to 15);
begin

parry(0) <= a(0) xor b(0);
c: for i in 1 to 15 generate
  parry(i) <= parry(i-1) xor a(i) xor b(i);
end generate;

paro <= parry(15);

end behavior;


Substring function in PHP I wrote this function to test if one string is a substring of another.

I'm curious to get some feedback on the implementation - does my solution seem reasonable? Is the code "well written"? Is there a better way of doing this?

<?php
function isSubStr($a, $b){
    if(!strpos($a, $b[0])){
        return false;
    }
    $result = true;
    $start = strpos($a, $b[0]);
    for ($pos = 0; $pos < strlen($b); $pos++){
            if($b[$pos] != $a[$start + $pos]){
                $result = false;
            }
    }
    return $result;
}

Tests below:

<?php
require 'solution.php';
use PHPUnit\Framework\TestCase;

class myTests extends TestCase {
  public function testExamples() {
        $this->assertEquals(isSubStr('cat', 'at'), true);
        $this->assertEquals(isSubStr('cat', 'fluffy'), false);
        $this->assertEquals(isSubStr('cheeze', 'ee'), true);
        $this->assertEquals(isSubStr('snow', 'o'), true);
        $this->assertEquals(isSubStr('octopus', 'top'), true);
        $this->assertEquals(isSubStr('octopus', 'zzz'), false);
  }
}

Beside the obvious fact that you're reinventing the wheel as this functionality is already present in php, here are some comments to your code:

Strange to use strpos() – If you want to reinvent the wheel, then I wouldn't use strpos() at all.
Doesn't restart checks – As commented upon by mdfst13, cases like ababc vs abc fails, as you don't restart the test if the first candidate fails.
Too long substrings gives index errors – Testing with octopus vs pussy results in index errors on the source string.
$a and $b are not really good names – They don't convey anything regarding their intended purpose, as $source and $substr would do.*
No comments – It always nice to have a little comments to indicate what you are trying to achieve in a code block.

So an even more basic version which implements these comments could look like:

function isSubStr($source, $substr) {
  $len_src = strlen($source);
  $len_sub = strlen($substr);
  $max_start_pos = $len_src - $len_sub;

  // If longer, there isn't room for it
  if ($max_start_pos < 0) {
    return false;
  }

  // Find start position for substring
  $pos = 0;
  while ($pos <= $max_start_pos) {

    // Check if we have a valid start position
    if ($source[$pos] == $substr[0]) {

       // Assume it is a good candidate, and verify this
       $foundCandidate = true;

      // Check for equality between source and substring,
      // and verify length of both strings
      for ($subIdx = 0, $orgIdx = $pos;
           $orgIdx < $len_src && $subIdx < $len_sub;
           $orgIdx++, $subIdx++) {

        if ($source[$orgIdx] != $substr[$subIdx]) {
          $foundCandidate = false;
          break;
        }
      }

      // If we exited previous loop due to exhausting the substring,
      // and not have a falsified candidate we have a match
      if ($foundCandidate && $len_sub == $subIdx) {
            return true;
      }
    }

    $pos++;
  }

  // Passed max start position without finding start of substr
  if ($pos > $max_start_pos) {
    return false;
  }
}

This code passes for all of the following cases:

public function testExamples() {
      $this->assertEquals(isSubStr('cat', 'at'), true);
      $this->assertEquals(isSubStr('cat', 'fluffy'), false);
      $this->assertEquals(isSubStr('cheeze', 'ee'), true);
      $this->assertEquals(isSubStr('snow', 'o'), true);
      $this->assertEquals(isSubStr('octopus', 'top'), true);
      $this->assertEquals(isSubStr('octopus', 'zzz'), false);
      $this->assertEquals(isSubStr('octopus', 'pussy'), false);
      $this->assertEquals(isSubStr('octopus', 'tops'), false);
      $this->assertEquals(isSubStr('ababc', 'abc'), true);
      $this->assertEquals(isSubStr('abc', 'abc'), true);
}


How to make DRY logger class with debug, info, warn and error methods that extends private method I am trying to make logger class with debug, info, warn and error methods that "extends" basic log method.
However it is not written using DRY principle, how can I make it DRY, not WET?
Code snippet

const logLevels = {
  0: 'debug',
  1: 'info',
  2: 'warn',
  3: 'error',
  'debug': 0,
  'info': 1,
  'warn': 2,
  'error': 3,
}

class Logger {
  logLevel = logLevels.debug

  log(logLevel, ...messages) {
    if (logLevel >= this.logLevel) {
      const logType = logLevels[logLevel]
      messages.unshift(logType)

      console.log(...messages)
    }
  }

  debug(...messages) {
    this.log(logLevels.debug, ...messages)
  }

  info(...messages) {
    this.log(logLevels.info, ...messages)
  }

  warn(...messages) {
    this.log(logLevels.warn, ...messages)
  }

  error(...messages) {
    this.log(logLevels.error, ...messages)
  }
}

const myLogger = new Logger()
const dateNowHex = () => Date.now().toString(16)

myLogger.debug('this is a test', dateNowHex())
myLogger.info('this is a test', dateNowHex())
myLogger.warn('this is a test', dateNowHex())
myLogger.error('this is a test', dateNowHex())

Should I make better class composition? Can you provide examples?

Interesting question;

getting the logType and then unshifting it seems complicated, why not
console.log(logLevels[logLevel], ...messages)

The mapping from an integer to a string and back from string to integer must be addressed to make this DRY

This code seems a good fit for monads, which can also help with making this DRY

Other than that there is not much to say.

function Logger() {

  const logLevels = ['debug', 'info', 'warn', 'error'];
  this.logLevel = 0;

  this.log = function log(logLevel, ...messages) {
    if (logLevel >= this.logLevel) {
      console.log(logLevels[logLevel], ...messages)
    }
  }

  for(let i = 0; i< logLevels.length; i++){
    this[logLevels[i]] = this.log.bind(this, i);
  }

}

const myLogger = new Logger()
const dateNowHex = () => Date.now().toString(16)
//debugger;
myLogger.debug('this is a test', dateNowHex())
myLogger.info('this is a test', dateNowHex())
myLogger.warn('this is a test', dateNowHex())
myLogger.error('this is a test', dateNowHex())


CSS layout basics to follow I've been trough a few css tutorials. I've also been using basic css for a while now.

My question, How do you decide witch css to use. For instance.
If you want a div box to show on the left. You can either use.

float: left;

or

position: absolute;

Another one would be, if you have a paragraph tag inside a div and you want to set its width or height. Do you use

width: auto;
height: auto;

or

width: 100%;
height: 100%;

or

padding: 5px;

I'm afraid that there's no 'good' answer here. There are many ways to achieve a certain effect, but they are dependent on the context.

Floated containers differ from the ones that are positioned absolutely. An absolutely positioned container is 'taken out of the layout' - that means e.g. that if you have 2 divs in a parent div and you set position absolute on one, the other one is going to fill the whole parent and the absolutely positioned one is going to be 'on top' of it (provided it has a width & height set, has a higher z-index or is placed last in DOM, but that's another story). If in the same situation you float one of the divs (and it has width set), the other one will 'stick' to it, but again, it (the second div) has to fulfill some criteria (e.g. if it has width set to 100% or 'clear' property set to both or left/right, it will not stick to the other one)

As for width - if an element is a block element or it has display:block set, width: 100% will expand it to be as wide as the closest ancestor that has position:relative or position:absolute set (this might be a little tricky), whereas width:100% on inline elements will not do anything.

width:100% will probably not work as you expect if you have padding set on the element (element will be to wide).

width:auto behaves differently on floated an non floated, block elements...

There are just too many variations. The same solution works in some cases, but doesn't work in others. It's rarely possible to say 'this is just going to work' if you don't look at what's happening to the rest of the page.


Haskell limit number of recursions I want to create a monad which will prevent more than 10 instances of recursion (which I am implementing by preventing more than 10 instances of >>=). My >>= definition seems kind of long and messy though, so I am wondering if there is a better way to do it.

I am including a factorial function which uses this monad as an example of how it could be used.

import Control.Monad.State
type LimitState a = (State Int (Either String a))
newtype Limiter a = Limiter {getState :: (LimitState a)}

instance Monad Limiter where
    return x = Limiter $ return $ Right x
    (Limiter state) >>= f = Limiter $
        do
            increment
            newCount <- get
            if newCount >= 10
                then return $ Left "too many recursions"
                else getNextValue
        where
            increment = do
                count <- get
                put (count + 1)
            getNextValue = do
                value <- state
                getState $ case value of
                    (Left message) -> Limiter $ return $ Left message
                    (Right v) -> f v

limitedFactorial :: Int -> Limiter Int
limitedFactorial 1 = return 1
limitedFactorial n = do
    recursive <- limitedFactorial (n - 1)
    return $ n * recursive

testLimitedFactorial =
    let (aValue, _) = runState (getState $ limitedFactorial 5) 0
        (uValue, _) = runState (getState $ limitedFactorial 11) 0
    in print aValue >> print uValue

Sheriff voice: Halt! You are breaking the laws!

Your monad breaks the right- and left-identity law of >>=:

k        >>= return = k   -- right identity
return a >>= f      = f a -- left  identity

Suddenly, it makes a big difference whether I call return at the end of my function:

do
   replicateM 9 actions        -- fine

do
   k <- replicateM 9 actions
   return k                     -- fails

That simply breaks to much. Your Monad is also missing its Functor and Applicative instances, which would make it easy to "cheat". And if you implemented a fmap that limits the number of fmaps, you start to break Functor laws. Note that due to the Monad/Functor laws, we should be able to rewrite

limitedFactorial :: Int -> Limiter Int
limitedFactorial 1 = return 1
limitedFactorial n = do
    recursive <- limitedFactorial (n - 1)
    return $ n * recursive

to

limitedFactorial :: Int -> Limiter Int
limitedFactorial 1 = return 1
limitedFactorial n = fmap (n *) $ limitedFactorial (n - 1)

If you were to limit that, you would break fmap id = id immediately.

But even without all of these quirks, at long last, there's nothing preventing the user to use recursion:

example :: Limit Int
example = return 1      -- fine

evil :: Int -> a -> Int
evil x _ = if x > 0 then 1 + evil (x - 1) undefined else 0

do
    liftM (evil 100000000) example -- recursion!

So conceptionally, your Monad is broken, unfortunately.

Further review

Your code uses names from your imports. state is a function defined in Control.Monad.State, so it makes your instance slightly hard to read.
You use increment only once, so it makes sense to inline it.


"The Genuine Sieve of Eratosthenes" in C++14 The following C++ code implements the "Genuine Sieve of Eratosthenes" algorithm as described in Melissa O'Neill's classic paper.  On my MacBook it computes the first 1,000,000 primes in about 11 seconds.

$ ./a.out | head -1000000 | tail -1
15485863
$

Just looking for general code review comments here. At least two parts of sieverator smell really bad to me, but I'm not sure of the proper way to fix them while still preserving the general "STLishness" of this code. For example, I really want to keep using the ranged for-loop in main.

#include <cassert>
#include <functional>
#include <iostream>
#include <memory>
#include <queue>
#include <utility>

template<typename Int>
class iotarator {
    Int value = 0;
    Int step = 1;
public:
    explicit iotarator() = default;
    explicit iotarator(Int v) : value(v) {}
    explicit iotarator(Int v, Int s) : value(v), step(s) {}
    Int operator*() const { return value; }
    iotarator& operator++() { value += step; return *this; }
    iotarator operator++(int) { auto ret = *this; ++*this; return ret; }

    bool operator==(const iotarator& rhs) const {
        return value == rhs.value && step == rhs.step;
    }
    bool operator!=(const iotarator& rhs) const { return !(*this == rhs); }
};

template<class Int>
class sieverator {
    struct erased_iterator {
        virtual Int dereference() = 0;
        virtual void increment() = 0;
    };
    template<class It>
    class derived_iterator : public erased_iterator {
        It it;
    public:
        derived_iterator(It it) : it(std::move(it)) {}
        Int dereference() override { return *it; }
        void increment() override { ++it; }
    };

    Int m_current;
    std::unique_ptr<erased_iterator> m_ptr;

    explicit sieverator() {}  // used by .end()
public:
    template<class It>
    explicit sieverator(It it) :
        m_current(*it),
        m_ptr(std::make_unique<derived_iterator<It>>(std::move(it)))
    {}
    sieverator begin() { return std::move(*this); }
    sieverator end() const { return sieverator{}; }
    bool operator==(const sieverator&) const { return false; }
    bool operator!=(const sieverator&) const { return true; }

    Int operator*() const {
        return m_current;
    }

    sieverator& operator++() {
        cross_off_multiples_of_prime(m_current);
        do {
            m_ptr->increment();
            m_current = m_ptr->dereference();
        } while (is_already_crossed_off(m_current));
        return *this;
    }

    sieverator& operator++(int) = delete;

private:
    struct pair {
        Int next_crossed_off_value;
        Int prime_increment;
        explicit pair(Int a, Int b) : next_crossed_off_value(a), prime_increment(b) {}
        bool operator<(const pair& rhs) const {
            return next_crossed_off_value < rhs.next_crossed_off_value ? true
                 : next_crossed_off_value > rhs.next_crossed_off_value ? false
                 : prime_increment < rhs.prime_increment;
        }
        bool operator>(const pair& rhs) const { return rhs < *this; }
    };
    template<class T> using min_heap = std::priority_queue<T, std::vector<T>, std::greater<>>;
    min_heap<pair> m_pq;

    bool is_already_crossed_off(Int value) {
        if (value != m_pq.top().next_crossed_off_value) {
            return false;
        } else {
            do {
                auto x = m_pq.top();
                m_pq.pop();
                m_pq.emplace(x.next_crossed_off_value + x.prime_increment, x.prime_increment);
            } while (value == m_pq.top().next_crossed_off_value);
            return true;
        }
    }

    void cross_off_multiples_of_prime(Int value) {
        m_pq.emplace(value * value, value);
    }
};

int main()
{
    iotarator<__int128_t> iota(2);
    sieverator<__int128_t> sieve(iota);
    for (int p : sieve) {
        std::cout << p << std::endl;
    }
}

At least to me, this seems a little like another instance of the same basic problem pointed out in linked paper: although there are a few parts that I guess are sort of neat, overall it strikes me as a huge amount of code to solve a trivial problem, and (at least based on the numbers you're posting) apparently runs really slowly.

It doesn't attempt to make much in the way of style points, but lets consider sort of a reference implementation--roughly the simplest thing that works to some degree:

#include <vector>
#include <iostream>
#include <locale>

unsigned long primes = 0;

int main() {
    int number = 20'000'000;
    std::vector<bool> sieve(number,false);
    sieve[0] = sieve[1] = true;

    for(int i = 2; i<number; i++) {
        if(!sieve[i]) {
            ++primes;
            for (int temp = 2*i; temp<number; temp += i)
                sieve[temp] = true;
        }
    }
    std::cout.imbue(std::locale(""));
    std::cout << "found: " << primes << " Primes\n";
    return 0;
}

On my notebook (4 or 5 years old, and wasn't tremendously fast when it was new) this finds 1,270,607 primes in 147 milliseconds (so in round numbers, it's running something like 75 or 80 times as fast as the code in the question). Even compared to @Toby's optimized version, it's still something like 20 times faster.

Don't get me wrong: I'm all for using nice algorithms and making code generic where it's reasonable--and I'm pretty sure there are ways to do that in this case without paying much of a penalty. But, at least to me, it seems to me as if the code you presented is paying a pretty high penalty--and given its much greater length and overall complexity, I'm not convinced we're getting any real improvement in return for that speed penalty either.

I'd re-emphasize that this is not really optimized code, or anything like that either--we can do quite a bit better by taking the square root into account, using a segmented sieve, and switching to something like a sieve of Atkins, and using a more specialized bit vector instead of std::vector<bool>. If we did those, we could probably expect to improve speed by around another order of magnitude (and depending on the number of primes we decided to find, maybe even more than that).


Generic memoize utility function for pure functions Given the following generic memoize utility for pure functions with type hints:
function memoize(callable $fn, callable $serialize = null): callable {
    $serialize = $serialize ? $serialize : 'serialize';

    return function(...$args) use ($fn, $serialize) {
        static $cache = [];
        $k = $serialize($args);
        return isset($cache[$k]) ? $cache[$k] : ($cache[$k] = $fn(...$args));
    };
}

Is the callable type hint here preferred as opposed to closure? I'm also not sure if I'm being too pedantic with the type hints and whether my use of static here is recommended?
I'd appreciate any feedback on the code above.

To address your first question:

Is the callable type hint here preferred as opposed to closure?

There is a very similar question on StackOverflow: PHP: Type hinting - Difference between Closure and Callable. It has three answers.
The first line of the answer by Xorifelse explains:

The main difference between them is that a closure is a class and callable a type.

And the last line of the accepted answer by Rizier123 has this suggestion:

So if you only want to type hint anonymous function use: Closure and if you want also to allow normal functions use callable as type hint.

And now, for something completely different general code review comments:
Variable naming
The name $k is a little too brief... $key isn't very long and describes what the variable is.
Simplifying ternary expressions
Correct me if I am mistaken but as long as PHP 5.3+ is used then this line:

$serialize = $serialize ? $serialize : 'serialize';

could be shortened using the fact that:

The expression (expr1) ? (expr2) : (expr3) evaluates to expr2 if expr1 evaluates to TRUE, and expr3 if expr1 evaluates to FALSE. Since PHP 5.3, it is possible to leave out the middle part of the ternary operator. Expression expr1 ?: expr3 returns expr1 if expr1 evaluates to TRUE, and expr3 otherwise.1

to the following:
$serialize = $serialize ?: 'serialize';

If PHP 7+ is used (which hopefully is the case, since at the time of writing, 7.2+ are actively maintained2) then the null coalescing operator i.e. ?? could also be used to simplify cases where a fallback should be used if a non-null value does not exist - e.g. :

 return isset($cache[$k]) ? $cache[$k] : ($cache[$k] = $fn(...$args));

Can become:
return $cache[$k] ?? ($cache[$k] = $fn(...$args));


Partial implementation of parsing a config-file and command line options I need to be able to accept some options from either the command-line or a config-file.

This code is a snippet of the class that I am working on, I cannot post the entire class here. This code works, but I am wondering if it can be made better. I will need to have a class here, there is a lot more work that goes on in the real code, I have pulled out just the parsing stuff. Please note, there are also several more sub-parsers that I will need, but the sub-parser shown is the only one that will possibly use a config file.

There is an issue with argparse with nesting argument_groups and mutually_exclusive_groups. So, I am not trying to use mutually-exclusive, but I am checking that with my own code in the functions.

import argparse
import yaml
from pprint import pprint

_possible_config_file_keys = ('names', 'ids')

class Parser(object):

    def __init__(self):
        self.build_parser()
        self.options = self.parse_args()
        #do other stuff here using self.options

    def build_parser(self):
        self.root_parser = argparse.ArgumentParser(description="Tool for ...")
        self.root_sub_parsers = self.root_parser.add_subparsers(dest='action')
        self.add_person_parser()
        self.add_other_parser()

    def add_person_parser(self):
        parser = self.root_sub_parsers.add_parser('People')
        g = parser.add_argument_group('People Targets')
        g.add_argument(
            "--config-file",
            help='Supply a config file',
            type=argparse.FileType(mode='r'))
        g.add_argument('--name', dest='names', default=[], action='append')
        g.add_argument('--id', dest='ids', default=[], action='append')

    def add_other_parser(self):
        pass #would do actual work here and add more subparsers

    def parse_args(self):
        args = self.parse_command_line_args()
        if args.config_file:
            self.parse_config_file(args)
        delattr(args, 'config_file')

        #I must ensure that 'Bob' is in the list of names
        if 'Bob' not in args.names:
            args.names.append('Bob')
        return args

    def parse_command_line_args(self):
        return self.root_parser.parse_args()

    def parse_config_file(self, args):
        self.check_args(args)
        data = yaml.load(args.config_file)
        args.config_file.close()
        for key, value in data.items():
            if key not in _possible_config_file_keys:
                self.root_parser.error("Invalid key '{}' in config file".format(key))
            if isinstance(value, str):
                value = [value, ]
            setattr(args, key, value)

    def check_args(self, args):
        invalid_keys = [
            key for key in vars(args).keys() if key in _possible_config_file_keys
            and getattr(args, key)]
        if invalid_keys:
            self.root_parser.error("Cannot use config-file keys along with config-file")

#For my testing purpose
if __name__ == '__main__':
    parser = Parser()
    pprint(vars(parser.options))

It is good practice to set all of the instance attributes you need in __init__; moving across the contents of build_parser into __init__ would only add three lines!

It seems a bit... roundabout, in places. For example, __init__ calls parse_args which calls parse_command_line_args which calls the root_parser's parse_args. At the very least, I would roll the call to self.root_parser.parse_args into parse_args, as then you aren't going back and forth between method names as you follow the logic through.

There is an unexplained "magic string" in the middle of it all:

#I must ensure that 'Bob' is in the list of names
if 'Bob' not in args.names:
    args.names.append('Bob')

The comment just says what the code is doing, but doesn't explain why, so it's redundant. In general, you have no explanations of the logic - docstrings would be helpful.

Some of those functions seem like implementation detail, rather than functionality you want to expose, so I would prefix the names with underscores to indicate private-by-convention, e.g. _add_person_parser and _add_other_parser.

I would make _possible_config_file_keys = ('names', 'ids') a class attribute of Parser, and indicate it as constant using the style guide naming conventions, i.e.

class Parser(object):

    POSSIBLE_CONFIG_FILE_KEYS = ('names', 'ids')

    ...

You only seem to use pprint for testing, so I would move that import into the if __name__ == '__main__': block. Also, there should be a blank line between standard library and third-party imports:

import argparse

import yaml

Do you not need to check_args in cases where you don't parse_config_file? Again, some documentation to explain that logic would be helpful.


Find the number of swaps that has taken place. New Year Chaos. Hackerrank I am trying to solve the following Hackerrank Problem.

I have written the code for the problem. However, it runs for some of the test cases while for others it doesn't. It shows the message : "Your code did not execute within the time limits"

I know that bubble sort is not required for the problem. However, I have seen many people doing it using bubble sort.

I want to do the problem by employing bubble sort. Please tell me what changes do I need to make in the code in order to do so. Thanks for helping.

int N = 0 ,I = 0;
        int flag = 0;

        int n = q.length;
        int temp = 0;
        int noOfSwaps = 0;

        for(int i = 0 ; i < q.length ; i++)
        {
            N = q[i];
            I = i;
            if((N-I)>3)
            {
                flag = 1;
                break;
            }
        }

        if(flag == 1)
        {
            System.out.println("Too chaotic");
        }
        else if(flag == 0)
        {
            //System.out.println("Find Exchanges");

         for(int i=0; i < n; i++){
                 for(int j=1; j < (n-i); j++){
                          if(q[j-1] > q[j]){
                                 //swap elements
                                 temp = q[j-1];
                                 q[j-1] = q[j];
                                 q[j] = temp;

                                 noOfSwaps++;
                         }

                 }
         }

         System.out.println(noOfSwaps);

        }

Time limit improvements

You don't need to implement a full bubble sort (as the maximum distance is 2 bribes!)
You can skip if a person is in the correct position if you know the processed part is correct.
You don't need to process the list twice, you only need to check if the person you are checking is within the maximum swap distance.

I commented my algorithm below; it is basically a bubble sort, only with a limit on how far the inner loop goes.

public class Solution {

    static final int MAX_SWAPS = 2;

    // Complete the minimumBribes function below.
    static void minimumBribes(int[] q) {
           int bribes = 0;

           int n=q.length;

           //we process from right to left
           for (int i=n-1; i>=0; i--)
           {
               int pos = i+1;
               int s;

               //if we are correct, we can proceed LEFT,
               //we know the right tail is correctly sorted
               //so after this step, the tail is 1 longer ans STILL correctly sorted
               if (q[i] == pos) continue;

               //try swap our position with previous positions, look MAX_SWAPS left.

               //for example:   1 5 2 4 3

               //if we are investigating position 5 (currently number 3)
               //we start swapping the items at position 3 and 4 if number 5 is at position 3.

               //if number 5 was not at position 3,
               //we try to find it at position 4
               //if number 5 was also not at position 4, we are too chaotic, as you
               //cannot bribe yourself more than MAX_SWAPS to the left.
               //so we are TOO CHAOTIC!

               //for example:   1 2 5 4 3

               //if number 5 is in position 5, we can skip it
               //(it is not)
               //if number 5 is at position 3
               //we swap the numbers 5 and 4 (at position 3 and 4) and have 1 bribe (bribes++)
               //situation becomes: 1 2 4 5 3
               //if now number 5 is at position 4, we have bribed again (bribes++)
               //situation becomes: 1 2 4 3 5

               //now we know 5 is in the correct position, and can scan from position 4
               //situation becomes: 1 2 4 3 [5]    // [] mark list that is DONE :)
               //if number 4 is in position 4, we add it
               //(it is not)
               //now if number 4 is in position 2, swap,
               //(it is not)
               //now if number 4 is in position 3, swap
               //it IS, swap them and bribes++
               //1 2 3 [4 5]

               //if number 3 is in position 3, we add it
               //1 2 [3 4 5]
               //etc.

               for (int m = MAX_SWAPS; m>0; m--)
               {
                   int left = i-m;
                   if (left>=0 && q[left]==pos)
                   {
                        s = q[left];
                        q[left]= q[left+1];
                        q[left+1]  = s;
                        bribes++;
                   }
                   else
                   {
                        if (m == 1) //if we didn't find the correct number at the last left position
                                    //after MAX swaps left, we are too chaotic
                        {
                           System.out.println("Too chaotic"); return;
                        }
                   }
               }
           }
           System.out.println(bribes);

    }

    private static final Scanner scanner = new Scanner(System.in);

    public static void main(String[] args) {
        int t = scanner.nextInt();
        scanner.skip("(\r\n|[\n\r\u2028\u2029\u0085])?");

        for (int tItr = 0; tItr < t; tItr++) {
            int n = scanner.nextInt();
            scanner.skip("(\r\n|[\n\r\u2028\u2029\u0085])?");

            int[] q = new int[n];

            String[] qItems = scanner.nextLine().split(" ");
            scanner.skip("(\r\n|[\n\r\u2028\u2029\u0085])?");

            for (int i = 0; i < n; i++) {
                int qItem = Integer.parseInt(qItems[i]);
                q[i] = qItem;
            }

            minimumBribes(q);
        }

        scanner.close();
    }
}


Convert a String with comma separated latitudes and longitudes to a Seq[Coordinate] I have a function that receives a String like the following:

"-10.0 -10.0,10.0 10.0,0.0 0.0"

And needs to translate that into a Seq[Coordinate] that would look like this:

Seq(
    Coordinate(-10.0, -10.0),
    Coordinate( 10.0,  10.0),
    Coordinate(  0.0,   0.0)
)

Here is the function that I wrote to achieve this:

def toCoordinates(coordinates: String): Seq[Coordinate] = {
  val coordinatesList = coordinates.split(",")
  coordinatesList flatMap { coordinate =>
    coordinate.split("\\s+") match {
      case Array(lat:String, lon:String) => Some(Coordinate(lat.toDouble, lon.toDouble))
      case _                             => None
    }
  }
}

I see that if someone sends "foo bar" as a coordinate, my code would fail miserably, although I don't see a way to prevent this without some if conditions.

Any idea on how I can implement this check while still having functional code? Or any other tips that might be helpful for a Scala newbie.

Also you can write more human-readable and understandable algorithm with the help of extractors and method collect:

val Coord = """(.+?)\s+(.+)""".r

object Double { def unapply(s: String) = Try(s.toDouble).toOption }

def parseCoordinates(input: String): Seq[Coordinate] =
  input.split(',') collect {
    case Coord(Double(lat), Double(lan)) => Coordinate(lat, lan)
  }


BST implementation in Python I was asked to perform the following tasks at one of the technical interviews. I did the BST implementation in python. I think the solution passed all of the solution. I broke down the tasks in the following bulletin:

#  1) initilize the binary Search tree
#  2) Implement the "put" and "contains" methods with helper function
#  3) Test the "inOrderTraversal" method.
#  4) Add additional relevant tests

import unittest

class BST(object):
    def __init__(self):
        self.root = Node()

    def put(self, value):
        self._put(value, self.root)

    def _put(self, value, node):
        if node.value is None:
            node.value = value
        else:
            if value < node.value:
                if node.left is None:
                    node.left = Node()
                self._put(value, node.left)
            else:
                if node.right is None:
                    node.right = Node()
                self._put(value, node.right)

    def contains(self, value):
        return self._contains(value, self.root)

    def _contains(self, value, node):
        if node is None or node.value is None:
            return False
        else:
            if value == node.value:
                return True
            elif value < node.value:
                return self._contains(value, node.left)
            else:
                return self._contains(value, node.right)

    def in_order_traversal(self):
        acc = list()
        self._in_order_traversal(self.root, acc)
        return acc

    def _in_order_traversal(self, node, acc):
        if node is None or node.value is None:
            return
        self._in_order_traversal(node.left, acc)
        acc.append(node.value)
        self._in_order_traversal(node.right, acc)

class Node(object):
    def __init__(self, value=None, left=None, right=None):
        self.value = value
        self.left = left
        self.right = right

class TestBST(unittest.TestCase):
    def setUp(self):
        self.search_tree = BST()

    def test_bst(self):
        self.search_tree.put(3)
        self.search_tree.put(1)
        self.search_tree.put(2)
        self.search_tree.put(5)
        self.assertFalse(self.search_tree.contains(0))
        self.assertTrue(self.search_tree.contains(1))
        self.assertTrue(self.search_tree.contains(2))
        self.assertTrue(self.search_tree.contains(3))
        self.assertFalse(self.search_tree.contains(4))
        self.assertTrue(self.search_tree.contains(5))
        self.assertFalse(self.search_tree.contains(6))

        self.assertEqual(self.search_tree.in_order_traversal(), [1,2,3,5])

    def test_empty(self):
        self.assertEqual(self.search_tree.in_order_traversal(), [])

    def test_negative(self):
        self.search_tree.put(-1)
        self.search_tree.put(11)
        self.search_tree.put(-10)
        self.search_tree.put(50)
        self.assertTrue(self.search_tree.contains(-1))
        self.assertTrue(self.search_tree.contains(11))
        self.assertTrue(self.search_tree.contains(-10))
        self.assertTrue(self.search_tree.contains(50))

        self.assertEqual(self.search_tree.in_order_traversal(), [-10,-1,11,50])

    def test_dupes(self):
        self.search_tree.put(1)
        self.search_tree.put(2)
        self.search_tree.put(1)
        self.search_tree.put(2)
        self.assertTrue(self.search_tree.contains(1))
        self.assertTrue(self.search_tree.contains(2))

        self.assertEqual(self.search_tree.in_order_traversal(), [1,1,2,2])

    def test_none(self):
        self.search_tree.put(None)
        self.assertFalse(self.search_tree.contains(None))

        self.assertEqual(self.search_tree.in_order_traversal(), [])

if __name__ == '__main__':
    unittest.main()

class Node:

It is "strange" to have a Node with no value.  Consider removing the default value=None.

You never create a Node with an explicit "left" or "right" branch.  Consider removing these extra parameters from the constructor, and just assigning them to None.

Private members should be prefixed by an underscore.  Since no external entity should access the value, or the left or right branches, these should be renamed.

class Node:
   def __init__(self, value):
      self._value = value
      self._left = None
      self._right = None

Node is actually an internal detail of BST, so perhaps nest it as an inner class:

class BST:

   class _Node:
      def __init__(self, value):
        self._value = value
        self._left = None
        self._right = None

Again, a node with no value is "strange".  It is much more common to represent an empty tree with as root=None, rather than root=Node(None).

   def __init__(self):
      self._root = None

Your put() method calls _put(), which can call _put(), which can call _put() and so on.  In short, it is recursive.  There is no need to be recursive; you never need to return a value from a sub-call.  You are using tail-recursion, so the compiler/interpreter might simply jump back to the top of the function, instead of creating additional stack-frames.  Except python doesn't do tail recursion, and you could get a stack overflow!

Instead, you can simply do the loop yourself:

   def put(self, value):

      new_node = BST._Node(value)

      if self._root:
         node = self._root

         while node:
            prev = node
            node = node._left if value < node._value else node._right

         if value < prev._value:
            prev._left = new_node
         else:
            prev._right = new_node

      else:
         self._root = new_node

Ditto for contains().  Don't rely on tail-recursion, just create the loop yourself.

   def contains(self, value):
      node = self._root

      while node:
        if node._value == value:
           return True
        node = node._left  if  value < node._value  else  node._right

      return False

In-order traversal: building lists is so passé.  Generators can be much more efficient.  Starting with Python 3.3, we also get the cool new yield from syntax, to make them easier:

   def inorder(self):

      def _inorder(node):

         if node._left:
            yield from _inorder(node._left)

         yield node._value

         if node._right:
            yield from _inorder(node._right)

     if self._root:
         yield from _inorder(self._root)

If you want to return a list, and not the generator, you could simply pass the generator to the list() function.

   def inorder_list(self):
       return list(self.inorder())

Avoid polluting the global namespace.  I prefer my tests in a separate "xxxx_test.py" file, but for a single file solution, you can put the required imports and test classes in the if __name__ == '__main__': block:

if __name__ == '__main__':

   import unittest, random

   class TestBST(unittest.TestCase):

      def test_random_lists(self):

         """Test a bunch of lists of various sizes, randomly"""

         for length in range(20):

            bst = BST()
            lst = [ random.randint(1, 100) for _ in range(length) ]

            with self.subTest(lst=lst):

               for val in lst:
                  bst.put(val)

               lst.sort()

               self.assertEqual( bst.inorder_list(), lst)

   unittest.main()

Finally, documentation!  Use """docstrings"""

Run help(BST) and look at the output.  Is everything documented?  Are there any unnecessary (ie, private) classes or methods exposed to the outside world?


Fixed space between two dynamically sized blocks Trying to come up with a way to have a fixed space (eg 20px) between two blocks on a single line which dynamically resize to fit that line (50% each minus the fixed space between).

I ended up with the following code (demo):

HTML:

foo

bar

CSS:

.cont{
    outline: 3px solid #ddd;
    position: relative;
    height: 50px;
}

.cont div{
    border: 1px solid #f0f;
    background: #fef;
    height: 40px;
    position: absolute;
    top: 0;
}

.left{
    left: 0;
    right: 50%;
    margin-right: 10px;
}

.right{
    left: 50%;
    right: 0;
    margin-left: 10px;
}

Is this a common or a good way to do it? Are there other or better ways (which work in IE >= 7).

Tested successfully in IE8 & IE9, and latest versions of Chrome, Firefox, and Opera.

I guess this is the proper way to have fixed space between to half-fitted blocks, but maybe later you'll have small issuer with container DIV, because child elements are absolute position and parent - relative, so it will not follow heights of child elements.

for example: http://jsfiddle.net/XfStv/3/

.

------UPDATE------

You can also use table property for objects, but it's not supported by IE 7:

here is example of tabled version code: http://jsfiddle.net/zur4ik/ehGRk/


Count of anagrams of a text in a given string Problem Statement:

  Given a word and a text, return the count of the occurences of
  anagrams of the word in the text. For eg. word is “for” and the text
  is “forxxorfxdofr”, anagrams of “for” will be “ofr”, “orf”,”fro”, etc.
  So the answer would be 3 for this particular example.

The above is an interview question I saw here.

Here is the code I have written:

#include <iostream>
#include<string.h>
#include<stdio.h>
using namespace std;

int equals(char* A, char* B)
{
    int i,len = strlen(A);
    int a[26],b[26];
    for(i=0;i<26;i++)
    {
        a[i]=0;
        b[i]=0;
    }
    for(i=0;i<len;i++)
    {
        a[A[i]-'a']++;
    }
    for(i=0;i<len;i++)
    {
        b[B[i]-'a']++;
    }
    for(i=0;i<26;i++)
    {
        if(a[i]!=b[i])
            return 0;
    }
    return 1;
}
int check(char* s1, char* s2)
{
    int i,j,len1=strlen(s1), len2=strlen(s2), flag=0;
    char arr[len2];

    for(i=0;i<=len1-len2;i++)
    {
         for(j=0;j<len2;j++)
         {
             arr[j]=s1[i+j];
         }
        if(equals(arr,s2))
            flag++;
    }
    return flag;
}
int main()
{
    char text[50]="",pat[50]="";
    cout<<"Enter original string: ";
    cin>>text;
    cout<<"Enter pattern string: ";
    cin>>pat;
    cout<<"No. of anagrams in text are :"<<check(text,pat);
    return 0;
}

(I have also mentioned this code in my technical blog).

Questions:

How can I improve my code in terms of its complexity and efficiency? As of now, it uses an approach similar to the naive string matching algorithm.
Is my method to not compute all permutations and instead use an array good enough, or are there any better approaches?

You code is quite efficient and well designed. I think you can get a little bit more efficiency at the cost of making the code more specific to this particular task. In particular there are some shortcuts:

your pattern is always the same so you could cache the computation of the array b[26]. This would double the speed
when you fill array a[] you could stop whenever a[i]>b[i]. This could speed up your algorithm a lot in some scenarios. For example you skip a position at once if the letter in the text is not in the pattern.
you are making copies of strings just to add the null terminator. You can avoid this by adding the length of the string as a parameter to your first function. This would help point 2 to be quite fast in the scenario already mentioned.
you can update the array a[26] incrementally. You decrease the count of character which exit the window and increment the count of characters entering it.

Putting all together (not tested):

int check(const char *text, const char *pattern) {
    int pattern_frequency[26];
    int text_frequency[26];
    int matches = 0;
    int pattern_length;
    for (int i=0; i<26; ++i) {
        pattern_frequency[i] = 0;
        text_frequency[i] = 0;
    }
    for (pattern_length=0; pattern[pattern_length]; ++pattern_length) {
       int i = pattern[pattern_length]-'a';
       assert(i>=0 && i<26);
       pattern_frequency[i]++;
    }
    for (int i=0; text[i]; ++i) {
        if (i>=pattern_length) {
            text_frequency[text[i-pattern_length]-'a']--;
        }
        int c = text[i]-'a';
        assert(c>=0 && c<26);
        text_frequency[c]++;
        if (text_frequency[c] != pattern_frequency[c]) continue; // shortcut
        int j;
        for (j=0;j<26 && text_frequency[j]==pattern_frequency[j];++j);
        if (j<26) continue;
        matches ++;
    }
    return matches;
}

If n is the length of the text and m the length of the pattern, your algorithm is O(nm) while this one is O(n+m) which cannot be improved further.


Calculate Final Grade: follow-up I'm creating an online calculator to get the average grade of different tests my students take and also the final grade. I wonder if you see any problems in this update or any way to improve it:

var inputs = document.getElementsByTagName('input');
var i;

function calculateAverage(tests) {
  var total = 0;
  var count = 0;
  for (i = 0; i < tests.length; i++) {
    if (tests[i].value) {
      total += Number(tests[i].value);
      count++;
    }
  }
  if (!count) {
    return 'Please enter a grade.';
  }
  return 'Average: ' + (total / count).toFixed(1);
}
document.getElementById('calculator').addEventListener('click', function() {
  var physicsTests = document.getElementById('physics').getElementsByTagName('input');
  var physicsAverage = document.getElementById('physicsAverage');
  physicsAverage.value = calculateAverage(physicsTests);

  var historyTests = document.getElementById('history').getElementsByTagName('input');
  var historyAverage = document.getElementById('historyAverage');
  historyAverage.value = calculateAverage(historyTests);

  var finalGrade = document.getElementById('finalGrade');
  var averagesTotal = physicsAverage.value.slice(9) * 3 + historyAverage.value.slice(9) * 2;
  if (averagesTotal || averagesTotal == 0) {
    finalGrade.value = 'Final grade:' + (averagesTotal / 5).toFixed(1);
  } else {
    finalGrade.value = '';
  }
});
document.getElementById('resetter').addEventListener('click', function() {
  var form = document.getElementById('form');
  var edited = calculateAverage(inputs);
  if (edited != 'Please enter a grade.' && confirm('Your changes will be lost.\nAre you sure you want to reset?')) {
    form.reset();
  }
});
window.addEventListener('beforeunload', function(event) {
  var edited = calculateAverage(inputs);
  if (edited != 'Please enter a grade.') {
    event.returnValue = 'Your changes may be lost.';
  }
});
<form id="form">

    Physics:
    <input type="number">
    <input type="number">
    <input type="number">
    <output id="physicsAverage"></output>

    History:
    <input type="number">
    <input type="number">
    <input type="number">
    <output id="historyAverage"></output>

  <button type="button" id="calculator">Calculate</button>
  <button type="button" id="resetter">Reset</button>
  <output id="finalGrade"></output>
</form>

P.S. I've made changes to the previous code to shorten the script and added some new features such as a reset button.

Note

Please review the final product made thanks to the hints I've received in this thread.

I went ahead and refactored your code a lot. However, before I get into that, lets talk about what you did and what you could have done differently.

Feel free to let me know if you want me to expand on something specific.

Logic regarding: function calculateAverage(tests) {}

Calculate average function should not take in parameters a list of HTMLInputElements and should not return a string.

Why? Well it doesn't make sense. calculateAverage should take a list of values and calculate an average.

function calculateAverage(values){
  return values.reduce((a,c)=>a+c,0)/values.length;
}
const avg = calculateAverage([1,2,3]);

console.log(avg);

Because you return a string... later on you have to do this:

physicsAverage.value.slice(9)

Not good ! And can become extremely buggy !

Use querySelectorAll

querySelectorAll is your friend. It allows you to transform this:

document.getElementById('physics').getElementsByTagName('input');

into this:

document.querySelectorAll('#physics > input');

Declare static variables globally:

In your calculator listener callback, you declare a list of variables.

var physicsTests = document.getElementById('physics').getElementsByTagName('input');
var physicsAverage = document.getElementById('physicsAverage');
//etc

But these variables never change. So don't declare them in your callback, move them outside and at the top of your file.

Create generic code !

With the current code you have, a lot of manual labor is needed if you wish to add another subject. In my final solution, everything is built in a way where adding a new subject requires NO changes in the javascript.

To able to do this, use data-* HTML properties. It's super powerful when you need to add some extra info like the coefficients of each subject.

const subjects = document.querySelectorAll("div[data-coef]");

subjects.forEach(subject=>{
  console.log(subject.id, subject.dataset.coef);
});

Another powerful tool is Map. It allows you to easily keep track of specific subjects and other variables.

Solution:

const inputContainers = document.querySelectorAll('form#form > .subject');

const finalGradeElement = document.getElementById('finalGrade');
const form = document.getElementById('form');

const m = new Map();

inputContainers.forEach((container,i)=>{
  const output = container.querySelector(".output");
  const inputs = Array.from(container.getElementsByTagName("input"));
  const coef = Number(container.dataset.coef);

  const data = Array(inputs.length);
  m.set(i, {
    output, data, coef
  });

  inputs.forEach((item,j)=>item.addEventListener("input", function(){
    if(!this.value.length > 0){
       data[j] = undefined;
    } else {
       data[j] = Number(this.value);
    }
  }));
});

document.getElementById('calculator')
.addEventListener('click', function() {

  const res = Array.from(m.values()).reduce( (a,{output, data, coef})=>{

      const values = data.filter(item=>item!==undefined);

      if(values.length > 0){
        const avg = values.reduce((a,c)=>a+c, 0)/values.length;
        a.avgTotal  += avg * coef;
        a.coefTotal += coef;

        output.value = `Average: ${avg.toFixed(1)}`;
      } else {
        output.value = "Please enter a number";
      }

      return a;
  }, {avgTotal: 0, coefTotal: 0})

  const averagesTotal = res.avgTotal/res.coefTotal;

  finalGradeElement.value = `Final Grade: ${averagesTotal.toFixed(1)}`

});

function isEdited(){
  return !Array.from(m.values()).every(({data})=>{
    return data.every(i=>i===undefined);
  });
}

document.getElementById('resetter')
.addEventListener('click', function() {
  if (isEdited() && confirm('Your changes will be lost.\nAre you sure you want to reset?')) {
    form.reset();
    m.clear();
  }
});

window.addEventListener('beforeunload', function(event) {
  if (isEdited()) {
    event.returnValue = 'Your changes may be lost.';
  }
});
form > div {
  display: flex;
  flex-direction: column;
}
<form id="form">

    Physics:
    <input type="number">
    <input type="number">
    <input type="number">
    <output class="output"></output>

    History:
    <input type="number">
    <input type="number">
    <input type="number">
    <output class="output"></output>

  <button type="button" id="calculator">Calculate</button>
  <button type="button" id="resetter">Reset</button>
  <output id="finalGrade"></output>
</form>


Erlang concurrency in circles I am new to erlang (2 days to be honest) and I would highly appreciate some peer review. I am doing these exercises and was a bit stuck at this point:

  2) Write a function which starts N processes in a ring, and sends a
  message M times around all the processes in the ring. After the
  messages have been sent the processes should terminate gracefully.

With some help from SO, I came up with a working solution, but I am not sure about its quality in any aspect: Language comprehension, Coding style, Efficiency, etc.

I will be grateful for any comment you have.

-module (concur).
-export ( [pingCircle/3, pingCircle/2] ).

pingCircle (Names, Message, TTL) ->
    Processes = lists:map (fun (Name) -> spawn (?MODULE, pingCircle, [Name, nobody] ) end, Names),
    ProcessPairs = lists:zip (Processes, rot1 (Processes) ),
    lists:map (fun ( {Process, Recipient} ) -> Process ! {setRecipient, Recipient} end, ProcessPairs),
    Circle = lists:map (fun ( {Process, _} ) -> Process end, ProcessPairs),
    hd (Circle) ! {Message, TTL - 1, lists:last (Circle) }.

rot1 ( [] ) -> [];
rot1 ( [Head | Tail] ) -> Tail ++ [Head].

pingCircle (Name, Recipient) ->
    receive
        {setRecipient, NewRecipient} ->
            pingCircle (Name, NewRecipient);
        {Message, 0, Originator} ->
            io:format ("~s received ~p with TTL 0 and dying.~n", [Name, Message] ),
            if
                Originator == self () -> io:format ("All dead.~n");
                true -> Recipient ! {Message, 0, Originator}
            end;
        {Message, TTL, Originator} ->
            io:format ("~s received ~p with TTL ~p.~n", [Name, Message, TTL] ),
            if
                Originator == self () -> Recipient ! {Message, TTL - 1, Originator};
                true -> Recipient ! {Message, TTL, Originator}
            end,
            pingCircle (Name, Recipient)
    end.

You can replace

spawn (?MODULE, pingCircle, [Name, nobody] )

with

spawn (fun() -> pingCircle(Name, nobody) end)

This action allows do not export pingCircle/2 from the module.
It is also recommended to use spawn_link instead of spawn, because it helps to avoid lost processes.


Singleton using enum Is this the correct implementation of a singleton using enum?

public class Item3 {

public static void main(String[] args) {

    Singleton s=Singleton.Single.INSTANCE.getInstance();
    Singleton s2=Singleton.Single.INSTANCE.getInstance();
    System.out.printf("%b",s==s2);
}
}

class Singleton {
    enum Single{
    INSTANCE;
    Singleton s=new Singleton();
    public Singleton getInstance(){
        if(s==null)
            return new Singleton();
        else return s;
    }
    }
}

No, it is simpler to code than that:

enum Singleton
{
    INSTANCE;

    // instance vars, constructor
    private final Connection connection;

    Singleton()
    {
        // Initialize the connection
        connection = DB.getConnection();
    }

    // Static getter
    public static Singleton getInstance()
    {
        return INSTANCE;
    }

    public Connection getConnection()
    {
        return connection;
    }
}

Then you use can use final Singleton s = Singleton.getInstance(). Note however that since this is an enum you can always access this via Singleton.INSTANCE.

And NEVER do that:

public Singleton getInstance(){
    if(s==null)
        return new Singleton();
    else return s;
}

this is not thread safe! What is more, the new Singleton() value is never assigned to s... (thanks @DaveJarvis for noticing this!)


Returning the next node What is considered to be a good programming practice for returning value from the function? For example, if we have a function in C named list* after( list *node), which returns the node which is after the node pointer.

First option:

node *after(node *first_ptr, node *nod_ptr)
{

    if (!(is_last(first_ptr, nod_ptr)) && !( first_ptr==NULL) && nod_ptr != NULL ) {

        while (first_ptr != nod_ptr && first_ptr != NULL){

            first_ptr = first_ptr->next;

        }

        return first_ptr != NULL ? first_ptr->next : NULL;

    }

     return NULL;

}

Second option:

node *after(node *first_ptr, node *nod_ptr)
{

    node *temp=NULL;
    if (!(is_last(first_ptr, nod_ptr)) && !( first_ptr==NULL) && nod_ptr != NULL ) {

        while (first_ptr != nod_ptr){

            first_ptr = first_ptr->next;
            temp = first_ptr;
            if(temp == NULL){

              break;
            }
            temp = temp->next;
        }

    }

     return temp;

}

Here node is typedef structure and next is a pointer to next element in the list and is_last(node *,node *) checks whether the second argument pointer is the last element in the list having start pointer as the first argument.

I'm wondering, in general, if big functions are necessary. Which seems nicer to understand?

Your after() function is actually doing two things: it returns nod_ptr->next, but with the stipulation that the list starting at first_ptr contains nod_ptr.  Therefore, you should decompose it into two functions.

/**
 * Returns the content node if it is contained in the list starting at
 * head, otherwise NULL.  The list must not contain a cycle.
 */
node *find(node *head, node *content)
{
    for (node *n = head; n != NULL; n = n->next)
    {
        if (n == content)
        {
            return content;
        }
    }
    return NULL;
}

/**
 * If the list starting at head contains content, returns content->next.
 * Otherwise, returns NULL.
 */
node *after(node *head, node *content)
{
    return (content != NULL && find(head, content)) ? content->next
                                                    : NULL;
}


Calculating Garland Degree of String Purpose
A garland word is a word

formed by chopping off the last few letters and “wrapping around” to the start.
For example: You can chop off the the trailing “on” from “onion” and still form the word by wrapping around to the starting “on”. In other words, you can write the letters “o”, “n”, “i” in a circle, in that order, and form the word “onion”.
I say that a garland word is of degree n if you can do the above with the last n letters of the word.

Build an implementation that returns the garland degree of a String. (Note, this was a dailyprogrammer subreddit question)
Strategy

Check if String is size 0 (return garland degree of 0) or size 1 (return garland degree of 1).
Start with the first character in the String. Iterate through the characters in the String from the second character onwards. Do we see a matching character? If not, return size 0.
Move on to the first + second characters in the String. Iterate through the characters in the String from the 3rd character onwards for a matching substring - if we can't see one, return size 1.
Repeat.
Note that the upper limit of the garland degree should be the the floor of the length of the String divided by 2. (Right? Or am I missing something?)

Implementation
public class GarlandDegreeIdentifierImpl implements GarlandDegreeIdentifier {
    @Override
    public int identifyGarlandDegree(final String candidate) {
        switch (candidate.length()) {
            case 0: {
                return 0;
            }

            case 1: {
                return 1;
            }

            default: {
                final char[] chars = candidate.toCharArray();
                for (int subStringIndex = 1; subStringIndex <= chars.length - subStringIndex; subStringIndex++) {
                    final char[] startingChars = Arrays.copyOfRange(chars, 0, subStringIndex);
                    final char[] remainingChars = Arrays.copyOfRange(chars, subStringIndex, chars.length);
                    if (subsetIndexIdentifier(remainingChars, startingChars) == -1) {
                        return subStringIndex - 1;
                    }
                }

                return Math.floorDiv(chars.length, 2);
            }
        }
    }

    @Override
    public int subsetIndexIdentifier(final char[] chars, final char[] candidateChars) {
        for (int index = 0; index <= chars.length - candidateChars.length; index++) {
            int counter = 0;
            for (int candidateCharIndex = 0; candidateCharIndex < candidateChars.length; candidateCharIndex++) {
                if (chars[index + candidateCharIndex] != candidateChars[candidateCharIndex]) {
                    counter++;
                }
            }

            if (0 == counter) {
                return index;
            }
        }

        return -1;
    }
}

You are completely over-doing it. Your algorithm will be O(n3) (I think) on the length of the input when you can do it more simply in O(n2).

Consider the following instead:

Start at the middle of the input.
While the substring starting from this index is not equal the substring beginning the start of the input with the same length, we increment the index.
Finally, we return the length of the final substring that matched.

Basically, the logic can be summed up by this drawing:

-------------
^^^^^^ ^^^^^^ matches or not?

// if no continue

-------------
^^^^^   ^^^^^ matches or not?

// if no continue

-------------
^^^^     ^^^^ matches or not?

You can see the logic.

Implemented in code, it would be the following:

private static int identifyGarlandDegree(String candidate) {
    int length = candidate.length();
    int index = length / 2;
    while (!candidate.substring(0, length - index).equals(candidate.substring(index))) {
        index++;
    }
    return length - index;
}

Note that we don't need to add a check on index: it won't go out of bounds since when index == length, it will exit normally (empty string being equal to another empty string).


Recursive brute-force approach to maximum points you can obtain from cards I came across this question on Leetcode. The question description is as follows:

There are several cards arranged in a row, and each card has an associated number of points.
The points are given in the integer array cardPoints. In one step, you can take one card from the beginning or from the end of the row.
You have to take exactly k cards. Your score is the sum of the points of the cards you have taken.
Return the maximum score you can obtain.
Sample testcases
cardPoints = [1,2,3,4,5,6,1], k = 3 => Output is 12
cardPoints = [2,2,2], k = 2 => Output is 4
cardPoints = [9,7,7,9,7,7,9], k = 7 => Output is 55
cardPoints = [1,1000,1], k = 1 => Output is 1

After looking at the discussion forums, I realized that this could be converted into a sliding window problem where we have to find the smallest subarray sum of length len(cardPoints) - k.
While I do understand this,
The initial method I tried was brute-force recursive and using dynamic programming to cache intermediate results. Despite this, it still results in a timeout. Is there any other optimization I can make to make my code run faster using this approach?
class Solution {
public:
    int maxScoreUtil(int left, int right,vector<int>& cardPoints, int k,vector<vector<int>>& dp){
        if(k == 0 || left == cardPoints.size() || right < 0)
            return 0;

        if(dp[left][right] != -1)
            return dp[left][right];

        int val_1 = maxScoreUtil(left+1,right,cardPoints,k-1,dp) + cardPoints[left];
        int val_2 = maxScoreUtil(left,right-1,cardPoints,k-1,dp) + cardPoints[right];

        return dp[left][right] = max(val_1,val_2);
    }

    int maxScore(vector<int>& cardPoints, int k) {
        int n = cardPoints.size();
        vector<vector<int>> dp(n+1, vector<int>(n+1, -1));
        return maxScoreUtil(0,n-1,cardPoints,k,dp);
    }
};I

Before using DP => 16/40 test cases passed followed by TLE
After using DP => 31/40 test cases passed followed by TLE

RE60K has already critiqued your algorithm, so I'll just leave some minor comments on the code style. These comments won't help you beat the time limit, but they are good habits to get into if you're going to take a coding interview someday, for example.

Your class Solution has only public members. Classes with all public members are frequently defined as structs instead.

In fact, your class has no data members! Classes with no data members should probably just be free functions. See The "OO" Antipattern.

Alternatively, you could keep the class as a sort of "namespace" for these two functions, but mark both functions static, so that you don't waste CPU cycles passing around a this pointer that you'll never use. In this case, you should also mark maxScoreUtil as private because it should never be called by anyone outside the class itself.

Alternatively, notice that your current code is paying for a this pointer and also paying to pass dp as the last argument to maxScoreUtil! It should only ever pay for one or the other. If dp always points to the same vector, maybe dp should be a data member of class Solution.

In all of these cases, the elements of cardPoints are not intended to be modified, so you should mark that parameter as const.

To summarize: Right now your code's skeleton is

class Solution {
public:
    int maxScoreUtil(int left, int right,vector<int>& cardPoints, int k,vector<vector<int>>& dp);
    int maxScore(vector<int>& cardPoints, int k);
};

but it should probably be either

class Solution {
    static int maxScoreImpl(int left, int right, const vector<int>& cardPoints, int k, vector<vector<int>>& dp);
public:
    static int maxScore(const vector<int>& cardPoints, int k);
};

or

static int maxScoreImpl(int left, int right, const vector<int>& cardPoints, int k, vector<vector<int>>& dp);
int maxScore(const vector<int>& cardPoints, int k);

or arguably

class Solution {
    vector<vector<int>> dp;
    vector<int> cardPoints;
    int maxScoreImpl(int left, int right, int k);
public:
    static int maxScore(vector<int> cardPoints, int k) {
       int n = cardPoints.size();
       Solution s{ vector<vector<int>>(n+1, vector<int>(n+1, -1)), std::move(cardPoints) };
       return s.maxScoreImpl(0, n-1, k);
    }
};

Notice that I've quietly changed your maxScoreUtil to maxScoreImpl (for "implementation"); that's the usual convention for an internal implementation function. "Util" (for "utilities") is more often used as a namespace or class name for general-purpose utility functions.

    if(k == 0 || left == cardPoints.size() || right < 0)
        return 0;

Surely this should say ... || right < left. Also, here and on the next line, you should get in the habit of curly-bracing any time you indent. It'll save you a lot of debugging someday. Consider the difference between

if (failed) {
    puts("oops, exiting");
    exit(0);
}

and

if (failed)
    puts("oops, exiting");
    exit(0);


Basic password authentication system app This is a console app I wrote for an end-of-chapter assignment in Jumping into C++. The parameters were for it to have a basic password system, and if you are eager, a multiple password / username system.  I am going through this book completely on my own free will.  It's not through a school assignment or whatnot.

It works quite well, but as I have a limited programming background, I wanted to post it here so any bad techniques or poor syntax could be brought to my attention.

#include <iostream>

using namespace std;

string usrnam[] = { "admin", "user", "" };
string pasword[] = { "root", "default", ""};

string temp_pass;
string temp_usr;

bool loggedin;

int n = sizeof(usrnam) / sizeof(string);

int findPassword( string str[], int array_size, string query )
{
    for ( int i = 0; i < array_size; i++ )
    {
        if ( str[i] == query )
    {
        return i;
    }
}
}

int commandC()
{
    int stage;
    cout << "Enter '1' for calculator: ";
    cin >> stage;
    switch(stage)
{
    case 1: {
        cout << endl;
        cout << "Not fully developed" << endl << endl;

    }
}

cout << "_-_-_-_-_-_-_-_-_-_-_-_-_-_-_-_-";
}

void login(string user, string pass){
     if ( pass == pasword[findPassword(usrnam, n, user)] )
     {
        cout << "Logged in as " << user << endl << endl;
        loggedin = true;
        commandC();
     }
}

void registerUser()
{
    string answer;
    cout << "No matches found in the database...would you like to make new account? (Y/N)";
    cin >> answer;
    if (answer == "y" || answer == "Y")
    {
        cout << "Please enter a username: ";
        cin >> usrnam[n-1];
        cout << "Please enter a password: ";
        cin >> pasword[n-1];

        login(usrnam[n-1],pasword[n-1]);
    }
}

int main()
{
    cout << "Enter username: ";
    cin >> temp_usr;
    cout << "Enter password; ";
    cin >> temp_pass;

    login(temp_usr,temp_pass);

    if(!loggedin)
        registerUser();
}

Try using std::vector instead of arrays. As a result, you'll not be required to keep track of the array_size in a separate variable like n in your case.

Something like this:

std::vector<std::string> username;
username.push_back("admin");
//other entries go here
std::vector<std::string> password;
password.push_back("root");
//other entries go here

Then, the size of the std::vector is given by size().
Try to use proper names as variables. Avoid wrong spellings or short-names. Exact variable names make the code easy to understand, as the name represents the actual concept the variable is referring to.
Also, in registerUser() the variable answer can just be a char. This will help in saving a little memory.
Like:

char answer;
std::cin>>answer;
if(answer=='y' || answer=='Y')
{
    //code goes here
}

In your code, the method commandC() is defined to return an int. But it doesn't do so. Make the return type void instead.

void CommandC()
{
    //code goes here
}

Also, the main() method is defined to return an int too. It is a good practice to declare it like that, but make sure to return an integer. 0 (zero) is preferred as it refers to successful completion of main().

int main()
{
    //code goes here
    return 0; //indicates successful termination
}


Changing a sprite based on the class I have a navigation, with a sprite that changes based on the class. In my jQuery, I'm clearing out the classes so it will just have .nav and then addClass the right class based on the click.  It works but feels very redundant. Does anyone have suggestions on optimizing this?

HTML:

HOME

GAME STATS

GAME TALK

GAME INFO

JS:

$('DIV.content DIV.nav ul li.home').click(function(){
    $('DIV.content DIV.nav').attr('class', 'nav');
    $('DIV.content DIV.nav').addClass('home');
});

$('DIV.content DIV.nav ul li.game-stats').click(function(){
    $('DIV.content DIV.nav').attr('class', 'nav');
    $('DIV.content DIV.nav').addClass('gamestats');
});

$('DIV.content DIV.nav ul li.game-talk').click(function(){
    $('DIV.content DIV.nav').attr('class', 'nav');
    $('DIV.content DIV.nav').addClass('gametalk');
});
$('DIV.content DIV.nav ul li.game-info').click(function(){
    $('DIV.content DIV.nav').attr('class', 'nav');
    $('DIV.content DIV.nav').addClass('gameinfo');
});

You can add both classes at once with attr, since all you need to do is pass a string with the classes you want separated by spaces:

$('DIV.content DIV.nav ul li.home').click(function(){
    $('DIV.content DIV.nav').attr('class', 'nav home');
});


Valid Alphanumeric Palindrome Valid Alphanumeric Palindrome

Problem (from Leetcode)

  Given a string, determine if it is a palindrome, considering only alphanumeric characters and ignoring cases. For example, A man, a plan, a canal: Panama is a palindrome but race a car is not a palindrome.

Discussion

My approach was the following:

Start with first and last characters of the input string (keeping track of their respective indices).
While the index of the first character is less than the index of it's "opposite" character...
a. For both characters, increment / decrement their index if they're not alphanumeric (and first character index is
  less than it's opposite character index
b. The invalid case occurs when

The first character index is less than it's opposite
Both characters are not alphabetic or not numeric
Both characters are alphabetic and their lower case (or upper case) values are not equal
Both characters are numeric but their values are not equal
c. Increment the first character index and decrement the opposite character index

If able to exit the while loop, return true

Other discussion points

I could probably make the helper methods (like isAlphanumeric) private
Open to other (better) names
My if statement is pretty inelegant / hard to read - move to a helper method perhaps?
Convert the whiles to fors with conditionals?

Implementation

public class AlphanumericPalindromeValidator {
    public static boolean isValid(String value) {
        char[] chars = value.toCharArray();
        int i = 0;
        int j = value.length() - 1;
        while (i < j) {
            char character = chars[i];
            while (!AlphanumericPalindromeValidator.isAlphanumeric(character) && i < j) {
                i++;
                character = chars[i];
            }
            char oppositeCharacter = chars[j];
            while (!AlphanumericPalindromeValidator.isAlphanumeric(oppositeCharacter) && i < j) {
                j--;
                oppositeCharacter = chars[j];
            }

            if (i < j
                    && !(AlphanumericPalindromeValidator.isAlphabeticCharacterPair(character, oppositeCharacter)
                        && Character.toLowerCase(character) == Character.toLowerCase(oppositeCharacter))
                    && !(AlphanumericPalindromeValidator.isNumericCharacterPair(character, oppositeCharacter)
                        && character == oppositeCharacter)) {
                return false;
            }

            i++;
            j--;
        }

        return true;
    }

    public static boolean isAlphanumeric(char c) {
        return Character.isAlphabetic(c) || Character.isDigit(c);
    }

    public static boolean isAlphabeticCharacterPair(char c1, char c2) {
        return Character.isAlphabetic(c1) && Character.isAlphabetic(c2);
    }

    public static boolean isNumericCharacterPair(char c1, char c2) {
        return Character.isDigit(c1) && Character.isDigit(c2);
    }

Code style

Definitely make the helper methods private;
There is no need to specify the class name when calling a static method in the same call. Instead of AlphanumericPalindromeValidator.isNumericCharacterPair(), just call isNumericCharacterPair();
Do a static import of the static methods of Character;
There is no need for the variables oppositeCharacter and character. Only keep them if it helps you make the code easier to read.
it's possible to make the inner while loop a one liner with a for, but I actually think the while better expresses what is the intent in this case.

If condition

I don't see the point of the first check: i < j. At ths point you know that i<=j. If i == j, the comparison will succeed. This is the case of palindromes with an odd length.

You don't need to check if the char is alphabetic to use Character.toLowerCase(). The method will simply return the same char if there is no mapping to lower case for a given char.

You can also use Character.isLetterOrDigit instead of your isAlphanumeric, unless you care about the subtle differences between "alphatetic" and "letter".

Suggested solution

No helper methods are needed.

public static boolean isValid(String value) {
  char[] chars = value.toCharArray();
  int i = 0;
  int j = value.length() - 1;

  while (i < j) {
    while (!isLetterOrDigit(chars[i]) && i < j) {
      i++;
    }
    while (!isLetterOrDigit(chars[j]) && i < j) {
      j--;
    }

    if (Character.toLowerCase(chars[i]) != toLowerCase(chars[j])) {
      return false;
    }

    i++;
    j--;
  }
  return true;
}