Script for keeping track of teams in a class I wrote this script to help me keep track of who has worked together and for what project or discussion in my classes.

I make a Google Sheet with the names of the students going across the first row and down the first column. Each student gets a number corresponding to their location in the spreadsheet. The value is the letter or number to represent the project or discussion and the numbers are the students who are on a team for that project.

I was particularly unsure of my use of getDataRange() because it seemed like that might not really be necessary since I'm just trying to get a cell. I just didn't see a more efficient way to do that. In my function for setting the backgrounds of the cells to white, I tried using getRange('upperleft:lowerright'), but I wasn't sure if that was really any better (and could possibly introduce errors if I tried to reuse the code for similar purposes in a larger class since I might forget to change it). The updateTeams() code is just where I enter the teams to see how that balances things out since I am trying to have students pair up with as many people as possible in the first few discussions or projects. It's there so I can try out different arrangements without having to remember much about what I tried before. If there is a yellow square at the intersection of two names, it means they have worked together before and the letters or numbers tell on what they worked together.

function fillInTeam(value, numbers){
  var spreadsheet = SpreadsheetApp.getActiveSpreadsheet();
  var first = 1;
  for (var i = 0; i< numbers.length; i++) {
    var theNum1 = numbers[i];
    for (var z = first; z < numbers.length; z++) {
      var y = numbers[z];
      var cell = spreadsheet.getDataRange().getCell(theNum1, y).setBackground('yellow');
      cell.setValue(cell.getValue() + value);
    }
    first++;
  }
}

function updateTeams(){
  clearBackgrounds();
  fillInTeam('s', [15, 14]);
  fillInTeam('s', [13, 12]);
  fillInTeam('s', [11, 10]);
  fillInTeam('s', [9, 8]);
  fillInTeam('s', [7, 6]);
  fillInTeam('s', [5, 4]);
  fillInTeam('s', [3, 2]);
  fillInTeam('h', [14, 13]);
  fillInTeam('h', [12, 11]);
  fillInTeam('h', [10, 9]);
  fillInTeam('h', [8, 7]);
  fillInTeam('h', [6, 5]);
  fillInTeam('h', [4, 3]);
  fillInTeam('h', [15, 2]);
  fillInTeam('j', [15, 13, 11, 9]);
  fillInTeam('j', [14, 12, 10, 8]);
  fillInTeam('j', [7, 5, 3]);
  fillInTeam('j', [6, 4, 2]);

}

function clearBackgrounds() {
  SpreadsheetApp.getActiveSpreadsheet().getDataRange().setBackground('white');
  SpreadsheetApp.getActiveSpreadsheet().getRange('B2:O15').setValue('');
}

I like your argument names for fillInTeam. I think they were good choices.
You declare the variable first and only use it here.

for (var z = first; z < numbers.length; z++) {

You can just set z = i + 1 instead.

for (var z = i + 1; z < numbers.length; z++) {

Speaking of, use j instead of z. Loop counters traditionally start with i and work their way up the alphabet as nesting goes deeper.
theNum1 and y are both absolutely atrocious names. Find something better and more meaningful for them, or better yet, just get rid of them entirely. Just use i and j to access the array elements directly.

function fillInTeam(value, numbers){
  var spreadsheet = SpreadsheetApp.getActiveSpreadsheet();
  for (var i = 0; i< numbers.length; i++) {
    for (var j = i + 1; j < numbers.length; j++) {
      var cell = spreadsheet.getDataRange().getCell(numbers[i], numbers[j]).setBackground('yellow');
      cell.setValue(cell.getValue() + value);
    }
  }
}

getDataRange().getCell() was the right choice here. getRange() is your other option, but it only takes A1 notation. That would have been less flexible in the long run.
Getting the DataRange inside of the loop is a bit inefficient. Instead of declaring spreadsheet as a variable, declare a dataRange variable and use it to access the individual cells.

function fillInTeam(value, numbers){
  var dataRange = SpreadsheetApp.getActiveSpreadsheet().getDataRange();
  for (var i = 0; i< numbers.length; i++) {
    for (var j = i + 1; j < numbers.length; j++) {
      var cell = dataRange.getCell(numbers[i], numbers[j]).setBackground('yellow');
      cell.setValue(cell.getValue() + value);
    }
  }
}

One last transformation. setBackground() returns a cell, so you can chain the calls together like this. (Note that I like chaining, but I might have taken it too far and made it less clear by getting rid of the cell variable.)

function fillInTeam(value, numbers){
  var dataRange = SpreadsheetApp.getActiveSpreadsheet().getDataRange();
  for (var i = 0; i< numbers.length; i++) {
    for (var j = i + 1; j < numbers.length; j++) {
      dataRange.getCell(numbers[i], numbers[j])
        .setBackground('yellow')
        .setValue(cell.getValue() + value);
    }
  }
}

updateTeams() has a definite smell to it, but I suspect that's because you're manually changing those prior to each run. I'm not sure what to do about it though.

clearBackgrounds() is a little clunky. It could definitely benefit from a spreadsheet variable like you have in fillInTeam().

I also don't like this.

.getRange('B2:O15').setValue('');

What happens if you add columns to your spreadsheet beyond O? You'll have to update your code to handle it. I'm thinking you can use getLastRow and offset to do this instead. Something like this untested code here.

var dataRange = spreadsheet.getDataRange();
dataRange.offset(1,0,dataRange.getLastRow() - 1, dataRange.getLastColumn()).setValue('');

I'm admittedly unhappy with how far it scrolls, but it's dynamic at least.


Animation effects for menu items I have pieced together some code that works for my project. However, it is pretty long, but self-similar. I am a jQuery beginner and I tried to shorten the code by using variables, but it didn't work so far. I guess I should use some for()-statement. Could you help me to simplify this code?

//apply the class "active" to the menu items whose div is in the viewport and animate
$('#home').bind('inview', function (event, visible) {
    if (visible == true) {
    $('#menu li:nth-child(1)').stop().animate(
            {backgroundPosition:"(0 0)"},
            {duration:500});
    $('#menu li:nth-child(1) a').css( {'color': 'white'} );
    $('#nav li:nth-child(1)').stop().animate({opacity: 1}, 500);
    } else {
    $('#menu li:nth-child(1)').stop().animate(
            {backgroundPosition:"(-200px 0)"},
            {duration:500});
    $('#menu li:nth-child(1) a').css( {'color': 'black'} );
    $('#nav li:nth-child(1)').stop().animate({opacity: 0.2}, 500);
    }
});
$('#referenzen').bind('inview', function (event, visible) {
    if (visible == true) {
    $('#menu li:nth-child(2)').stop().animate(
            {backgroundPosition:"(0 0)"},
            {duration:500});
    $('#menu li:nth-child(2) a').css( {'color': 'white'} );
    $('#nav li:nth-child(2)').stop().animate({opacity: 1}, 500);
    } else {
    $('#menu li:nth-child(2)').stop().animate(
            {backgroundPosition:"(-200px 0)"},
            {duration:500});
    $('#menu li:nth-child(2) a').css( {'color': 'black'} );
    $('#nav li:nth-child(2)').stop().animate({opacity: 0.2}, 500);
    }
});
$('#unternehmen').bind('inview', function (event, visible) {
    if (visible == true) {
    $('#menu li:nth-child(3)').stop().animate(
            {backgroundPosition:"(0 0)"},
            {duration:500});
    $('#menu li:nth-child(3) a').css( {'color': 'white'} );
    $('#nav li:nth-child(3)').stop().animate({opacity: 1}, 500);
    } else {
    $('#menu li:nth-child(3)').stop().animate(
            {backgroundPosition:"(-200px 0)"},
            {duration:500});
    $('#menu li:nth-child(3) a').css( {'color': 'black'} );
    $('#nav li:nth-child(3)').stop().animate({opacity: 0.2}, 500);
    }
});
$('#taetigkeit').bind('inview', function (event, visible) {
    if (visible == true) {
    $('#menu li:nth-child(4)').stop().animate(
            {backgroundPosition:"(0 0)"},
            {duration:500});
    $('#menu li:nth-child(4) a').css( {'color': 'white'} );
    $('#nav li:nth-child(4)').stop().animate({opacity: 1}, 500);
    } else {
    $('#menu li:nth-child(4)').stop().animate(
            {backgroundPosition:"(-200px 0)"},
            {duration:500});
    $('#menu li:nth-child(4) a').css( {'color': 'black'} );
    $('#nav li:nth-child(4)').stop().animate({opacity: 0.2}, 500);
    }
});
$('#kontakt').bind('inview', function (event, visible) {
    if (visible == true) {
    $('#menu li:nth-child(5)').stop().animate(
            {backgroundPosition:"(0 0)"},
            {duration:500});
    $('#menu li:nth-child(5) a').css( {'color': 'white'} );
    $('#nav li:nth-child(5)').stop().animate({opacity: 1}, 500);
    } else {
    $('#menu li:nth-child(5)').stop().animate(
            {backgroundPosition:"(-200px 0)"},
            {duration:500});
    $('#menu li:nth-child(5) a').css( {'color': 'black'} );
    $('#nav li:nth-child(5)').stop().animate({opacity: 0.2}, 500);
    }
});
$('#anfahrt').bind('inview', function (event, visible) {
    if (visible == true) {
    $('#menu li:nth-child(6)').stop().animate(
            {backgroundPosition:"(0 0)"},
            {duration:500});
    $('#menu li:nth-child(6) a').css( {'color': 'white'} );
    $('#nav li:nth-child(6)').stop().animate({opacity: 1}, 500);
    } else {
    $('#menu li:nth-child(6)').stop().animate(
            {backgroundPosition:"(-200px 0)"},
            {duration:500});
    $('#menu li:nth-child(6) a').css( {'color': 'black'} );
    $('#nav li:nth-child(6)').stop().animate({opacity: 0.2}, 500);
    }
});

I assume you have set up the links in the menu properly, i.e. they reference the elements they correspond to, something like

Home

Referenzen

So you can just search for the corresponding link with the attribute selector [docs] and get the enclosing li element:

$('#home, #referenzen, ...').bind('inview', function (event, visible) {
    var $menu_element = $('#menu a[href=#' + this.id + ']').closest('li'),
        // get the '#nav li' element at the same position
        $nav_element =  $('#nav li').eq($menu_element.index());

    $menu_element.stop().animate(
        {backgroundPosition: visible ? '(0 0)' : '(-200px 0)'},
        {duration: 500}
    ).find('a').css({color: visible ? 'white' : 'black'});
    $nav_element.stop().animate({opacity: visible ? 1 : 0.2}, 500);
});

If you don't have set up the links correctly (you really should), give the li items in the #menu (and maybe #nav) list a (for example) data-id attribute with a value corresponding to the element's ID and again, use the attribute selector to get them.


Ruby Command Line Tic-Tac-Toe The following is my implementation of a command-line Tic-Tac-Toe game, written in Ruby. This was my first attempt at practicing object-oriented design principles.

require 'colored'

module TicTacToe
  class Player
    attr_accessor :symbol
    def initialize(symbol)
      @symbol = symbol
    end
  end

  class Board
    attr_reader :spaces
    def initialize
      @spaces = Array.new(9)
    end

    def to_s
      output = ""
      0.upto(8) do |position|
        output << "#{spaces[position] || position}"
        case position % 3
        when 0, 1 then output << " | "
        when 2 then output << "\n-----------\n" unless position == 8
        end
      end
      output
    end

    def check_space(cell, sym)
      if spaces[cell].nil?
        place_symbol(cell, sym)
        @current_turn += 1
      else
        puts "Space unavailable! Please select another cell"
      end
    end

    def place_symbol(cell, sym)
      spaces[cell] = sym
    end

    WINNING_COMBOS = [
      [0, 1, 2], [3, 4, 5], [6, 7, 8],
      [0, 3, 6], [1, 4, 7], [2, 5, 8],
      [0, 4, 8], [2, 4, 6]
    ]

    def winning_scenarios
      WINNING_COMBOS.each do |set|
        if spaces[set[0]] == spaces[set[1]] && spaces[set[1]] == spaces[set[2]]
          return true unless spaces[set[0]].nil?
        end
      end
      false
    end

    def tie
      if !spaces.include?(nil) && !winning_scenarios
        return true
      end
    end
  end

  class Game < Board
    attr_reader :player1, :player2, :symbol
    def initialize
      super
      play_game
    end

    def play_game
      @player1 = Player.new("X")
      @player2 = Player.new("O")
      puts Board.new
      @current_turn = 1
      turn
      win_message
      tie_message
      play_again
    end

    def move(player)
      while !winning_scenarios && !tie
        puts "Where would you like to move 'player #{player.symbol}'?".red
        choice = gets.chomp.to_i
        check_space(choice, player.symbol)
        puts "Player #{player.symbol}'s move:".green
        puts self
        turn
      end
    end

    def tie_message
      puts "It's a Draw!".cyan if tie
    end

    def win_message
      puts "Game over!".cyan if winning_scenarios
    end

    def turn
      @current_turn.even? ? move(@player2) : move(@player1)
    end

    def play_again
      puts "Play again? (yes or no)".yellow
      answer = gets.chomp.downcase
      if answer == "yes"
        TicTacToe::Game.new
      else
        puts "Goodbye".cyan.bold
      end
    end
  end
end

TicTacToe::Game.new

Not bad.

Unbounded recursion

Inside the Game class:

def initialize
  # ...
  play_game
end

def play_game
  @player1 = Player.new("X")
  @player2 = Player.new("O")
  # ...
  play_again
end

def play_again
  # ...
  if answer == "yes"
    TicTacToe::Game.new
  else
    # ...
  end
end

You're using recursion to repeat the game, which can get arbitrarily deep, leading to a stack overflow error. Use a loop instead. Also, to avoid violating the Single Responsibility Principle, put this loop outside the Game class whose sole responsibility should be to play a game.

Don't call play_again in play_game, and move play_again outside the class. The main code should be:

def play_again?
  puts "Play again? (yes or no)".yellow
  answer = gets.chomp.downcase
  return answer == "yes"
end

loop do
  TicTacToe::Game.new
  unless play_again?
    puts "Goodbye".cyan.bold
    break
  end
end

(I renamed play_again to play_again?. It's conventional to use a question mark in names of methods that return a boolean)

Unbounded recursion 2

def turn
  @current_turn.even? ? move(@player2) : move(@player1)
end

def move(player)
  while !winning_scenarios && !tie
    # ...
    turn
  end
end

This recursion is not bounded either (it's not bounded by the number of spaces in the board, because a function call occurs for (rejected) invalid moves too). Use a loop instead:

def play_game
  # ...
  while !winning_scenarios && !tie
    turn
  end
  # ...
end

def turn
  @current_turn.even? ? move(@player2) : move(@player1)
end

def move(player)
  # ...
  # Don't call turn here
end

Object oriented design

class Board; ...; end

class Game < Board; ...; end

A game is not a board. A game has a board. Use composition instead of inheritance:

class Game # NOTE: Don't inherit Board
  def initialize
    @board = Board.new # NOTE
    play_game
  end

  def play_game
    # ...
    while !@board.winning_scenarios && !@board.tie
      turn
    end
    # ...
  end

  def move(player)
    # ...
    space_available = @board.check_space(choice, player.symbol)
    @current_turn += 1 if space_available
    puts "Player #{player.symbol}'s move:".green
    puts @board # NOTE
  end

  def tie_message
    ... if @board.tie
  end

  def win_message
    ... if @board.winning_scenarios
  end

  # ...
end

Note I moved @current_turn += 1 to Game. A game has a current turn, a board doesn't. Modify check_space so it returns a boolean.

Encapsulation

Make internal methods private, so they aren't visible outside the class:

class Board
  def initialize; ...; end
  def to_s; ...; end
  def check_space(cell, sym); ...; end
  def winning_scenarios; ...; end
  def tie; ...; end

  private

    WINNING_COMBOS = [
      [0, 1, 2], [3, 4, 5], [6, 7, 8],
      [0, 3, 6], [1, 4, 7], [2, 5, 8],
      [0, 4, 8], [2, 4, 6]
    ]

    def place_symbol(cell, sym); ...; end
      spaces[cell] = sym
    end
end

class Game
  def initialize; ...; end
  def play_game; ...; end

  private

    def move(player); ...; end
    def tie_message; ...; end
    def win_message; ...; end
    def turn; ...; end
end

Remove this line which exposes internal fields of Game: (you never actually use Game#symbol, by the way)

attr_reader :player1, :player2, :symbol

And this line from Board: (then change every occurence of spaces in Board to @spaces)

attr_reader :spaces

Board methods

def check_space(cell, sym)
  if @spaces[cell].nil?
    place_symbol(cell, sym)
  else
    puts "Space unavailable! Please select another cell"
  end
end

Single Responsibility Principle: Board should not do any printing, as it's not its core responsibility. It's better to return a boolean instead and let Game do the printing.
Naming: This method both checks if the space is free and places a symbol. Rename it to place_symbol_if_free.
Naming 2: Use position instead of cell to be consistent with other methods.
Consider splitting this to two methods: space_free?(position) and place_symbol(position, sym) (already exists) and then call both from Game.

WINNING_COMBOS = [
  [0, 1, 2], [3, 4, 5], [6, 7, 8],
  [0, 3, 6], [1, 4, 7], [2, 5, 8],
  [0, 4, 8], [2, 4, 6]
]

Put this inside winning_scenarios, the only method where it's used.

def winning_scenarios
  WINNING_COMBOS.each do |set|
    if @spaces[set[0]] == @spaces[set[1]] && @spaces[set[1]] == @spaces[set[2]]
      return true unless @spaces[set[0]].nil?
    end
  end
  false
end

Rename winning_scenarios to game_won?. Or to winner and return the winner's symbol
Use Array#any? instead of looping.
Use Array#map to obtain the symbols at the locations of a set, instead of repeatedly accessing @spaces.

Result:

def winner
  WINNING_COMBOS.any? do |set|
    symbols = set.map { |position| @spaces[position] }
    if symbols[0] == symbols[1] && symbols[1] == symbols[2]
      symbols[0]
    end
  end
end

def tie
  if !@spaces.include?(nil) && !winning_scenarios
    return true
  end
end

Rename to tie?, and simply return the boolean:

def tie?
  return !@spaces.include?(nil) && !winner
end

I also suggets extracting first part to a new private method full?.

to_s can be simplified by using functional programming. See my implementation of as_string in this answer.

Game methods

play_game

puts Board.new

Change this to puts @board.

while !@board.winner && !@board.tie?

Looks like Board is missing a game_over? method.

win_message
tie_message

These method names are misleading. Each one conditionally prints a message. Merge them to a single print_game_result method.

turn

def turn
  @current_turn.even? ? move(@player2) : move(@player1)
end

The call to move is duplicated because the method does two things: (1) determine who's turn it is (2) call move.

It's better to do only the first one:

def current_player
  @current_turn.even? ? @player2 : @player1
end

Then use move(current_player) in play_game.

move

def move(player)
  puts "Where would you like to move 'player #{player.symbol}'?".red
  choice = gets.chomp.to_i
  space_available = @board.check_space(choice, player.symbol)
  @current_turn += 1 if space_available
  puts "Player #{player.symbol}'s move:".green
  puts @board
end

I'd move the last two puts statements to play_game, because they're not part of making a move.

If you split check_space as I suggest earlier, you can make a loop that gets input until Board#space_free? returns true and then call Board#place_symbol. Otherwise, you'll have to rename this method to try_make_a_move.

initialize

Don't call game_play. Constructors should only initialize an object, and should not do IO. Instead, call game_play in the main code (TicTacToe::Game.new.play_game).

Further Improvements

Say who won.
Use digits 1 to 9 when printing the board and getting user input. It's hard to tell 0 and O apart.


Get mod date to attach a version number to JS src script Instead of the browser checking if the file was modified before serving the JS script (adds a few seconds to ping the server), or waiting for an arbitrary amount of time until the JS script expires, I have the PHP script instead attach a version number (e.g. script.js?v=123) to the JS script based off of the last modification date.

/**
 * Creates a new version once the file has been modified ensuring the user always receives the most updated file while still utilizing the browser cache
 *
 * @param $path string|array root: "/js/master/example.js" | local: "script.js"
 *
 * @return string
 */
function js($path){
    if(!empty($path)){
        if(is_array($path)){
            $return = '';
            foreach($path as $value){
                $return .= js($value);
            }
            return $return;
        } else {
            if (substr($path, 0, 1) <> '/') {
                $version = dechex(filemtime(getcwd().'/'.$path));
            } else {
                $version = dechex(filemtime(__DIR__.$path));
            }
            return "<script src='$path?v=$version'></script>\r\n";
        }
    }
    return false;
}

API

The documentation says the function returns a string, yet the last line of the implementation says otherwise...

/**
 * ...
 *
 * @return string
 */
function js($path){
    ...
    return false;
}

Replace a loop with array_map

Instead of this:

$return = '';
foreach($path as $value){
    $return .= js($value);
}
return $return;

You can write a bit simpler using array_map and join:

if (is_array($path)) {
    return join('', array_map('js', $path));
}

Getting the first character from a string

Instead of substr($path, 0, 1), when you know that $path is not empty (as in the posted code), a more natural way is to use indexing: $path[0].

Don't repeat yourself

dechex(filemtime(...)) is common in the two branches of this conditional:

if (substr($path, 0, 1) <> '/') {
    $version = dechex(filemtime(getcwd().'/'.$path));
} else {
    $version = dechex(filemtime(__DIR__.$path));
}

Extract the common part:

if ($path[0] <> '/') {
    $dir = getcwd().'/';
} else {
    $dir = __DIR__;
}

Use early returns

Instead of this kind of code:

if (cond1) {
    if (cond2) {
        // ...
        return X;
    } else {
        // ...
        return Y;
    }
}
return Z;

You can use "early returns" to reduce the nesting,
and thereby making the code slightly easier to read:

if (!cond1) {
    return Z;
}

if (cond2) {
    // ...
    return X;
}

return Y;

Alternative implementation

Putting the above tips together:

function js($path) {
    if (empty($path)) {
        return '';
    }

    if (is_array($path)) {
        return join('', array_map('js', $path));
    }

    $dir = $path[0] <> '/' ? getcwd().'/' : __DIR__;
    $version = dechex(filemtime($dir.$path));
    return "<script src='$path?v=$version'></script>\r\n";
}


Sum of digits in a^b I'm trying to calculate a problem which requires sum of digits in \$a^b\$, where \$0 \le a \le 9\$ and \$0 \le b \le 4000\$.

I have seen various other posts similar to this, but I haven't acquired best way. I have created this:

#include<stdio.h>
int main() {
    int digits[4000]={0} ;
    long long int test,a,b,temp,newVar ;
    scanf("%lld",&test) ;
    newVar=1 ;
    while(test--) {
        scanf("%lld %lld",&a,&b) ;
        if(b==0) {
            printf("Case %lld: 0\n",newVar) ;
            newVar++ ;
            continue ;
        }
        b=b-1 ;
        digits[0]=a ;
        long long int len=1,carry=0,i ;
        while(b--) {
            carry=0 ;
            for(i=0 ; i<len  ; i++) {
                temp=digits[i]*a + carry ;
                digits[i]=temp%10;
                carry=temp/10 ;
            }
            if(carry!=0) {
                digits[i]=carry ;
                len++ ;
            }
        }
        long long int sum=0 ;
        for(i=len-1 ; i>=0 ; i--)
            sum+=digits[i] ;
        printf("Case %lld: %llu\n",newVar,sum) ;
        newVar++ ;
    }
    return 0 ;
}

I tested my code for large powers, which is giving the right results.

For example:

Input:

3
2 32
3 2
2 60

Output:

Case 1: 58
Case 2: 9
Case 3: 82

Is there any way to increase the speed or running time of this code? I have a time limit of 1 sec and this code is showing Time Limit Exceeded.

This is not a contest that is going to effect my(or anyone else) ratings. For me, it's a way to learn new things. So any help will be appreciated.

Some general remarks:

Use more and consistent spacing, e.g.

digits[i]=temp%10;

should be

digits[i] = temp % 10;

Define variables only at the scope where they are used, e.g. carry
is used only inside the while (b--) { ... } loop.
Move the actual computation to a separate function so that main() becomes
shorter. That makes it also easier to add test cases.
Assuming that int has (at least) 32 bits (which is the case on all modern
platforms), there is no need to use long long at all in your code.
There is no need to treat the case b = 0 separately. a ^ 0 is 1, and that can be covered with the general case if you
start with digits[] = { 1 } instead of digits[] = { a }.

The performance can be increased by storing more than a single decimal
digit in each array element digits[i]. Since 10^9 < 2^31, you can store
8 decimal digits in digits[i] and digits[i] * a + carry will still not
overflow.

Your program would then look like this:

const int BASE = 100000000; //  10^8

int powerDigitSum(int a, int b) {
    int digits[4000] = { 1 } ;
    int len = 1;

    while (b-- > 0) {
        int carry = 0;
        for (int i = 0; i < len; i++) {
            int temp = digits[i] * a + carry;
            digits[i] = temp % BASE;
            carry = temp / BASE;
        }
        if (carry != 0) {
            digits[len++] = carry;
        }
    }

    int sum = 0;
    for (int i = 0; i < len; i++) {
        int d = digits[i];
        while (d > 0) {
            sum += d % 10;
            d /= 10;
        }
    }
    return sum;
}

int main() {
    int test, a, b;
    scanf("%d",&test) ;
    for (int i = 0; i < test; i++) {
        scanf("%d %d",&a,&b) ;
        int sum = powerDigitSum(a, b) ;
        printf("Case %d: %d\n", i, sum) ;
    }
    return 0 ;
}

Your original algorithm corresponds to BASE = 10. Using BASE = 100000000
instead reduced the computation time for a=9, b=4000 from 0.095 seconds
to 0.014 seconds on my computer.

If that is not sufficient, you can use (unsigned) long long for the digits[] array  and work with an even larger BASE.


Beautiful Word: HackerRank Here is my code for hacker rank question. Can somebody suggest improvement?

Question:

  We consider a word, to be beautiful if the following two conditions
  are satisfied:

  No two consecutive characters are the same.
  No two consecutive characters are in the following vowel set: a, e, i, o, u, y. Note that we consider y to be a vowel in this challenge.

Solution:

import java.io.*;
import java.util.*;
import java.text.*;
import java.math.*;
import java.util.regex.*;

public class Solution {

   public static void main(String[] args) {
        Scanner in = new Scanner(System.in);
        String w = in.next();

        int result = isBeautiful(w);

        if (result == 1) {
            System.out.print("Yes");
        } else
            System.out.print("No");

    }

    static int isBeautiful(String w) {
        for (int i = 0; i < w.length() - 1; ++i) {
            if (w.charAt(i) == w.charAt(i + 1)) {
                return 0;
            }
            if ( ( isVowel(w.charAt(i)) == isVowel(w.charAt(i + 1)) && (isVowel(w.charAt(i)) == 1))) {
                return 0;
            }
        }
        return 1;
    }

    static int isVowel(char c) {
        if ( (c == 'a') || (c == 'e') || (c == 'i') || (c == 'o') || (c == 'u') || (c == 'y') ) {
            return 1;
        }
        return 0;
    }

}

First of all, you did well splitting it up into those methods.

The major issue I have with your code is that the natural translation from the English answers yes/no to Java is to use a boolean. Where true means yes and false means no.

So your isVowel method should be written like this:

static boolean isVowel(char c) {
    if ( (c == 'a') || (c == 'e') || (c == 'i')
        || (c == 'o') || (c == 'u') || (c == 'y') ) {
        return true;
    }
    return false;
}

Now the next thing you need to know is that a check like c == 'a' is already a boolean, and that the || operator combines 2 booleans into a new boolean. Since we want to return a boolean, instead of writing

if (<boolean check>) {
    return true;
} else {
    return false;
}

we should just write the following:

return <boolean check>;

which in your case would be:

static boolean isVowel(char c) {
    return (c == 'a') || (c == 'e') || (c == 'i')
           || (c == 'o') || (c == 'u') || (c == 'y');
}

A final issue with this method is that it's actually package-visible. There is no reason to use this method outside of this class, so it should be private.

private static boolean isVowel(char c) {

The same reasoning applies to the isBeautiful method. So let us also make this private and return a boolean.

private static boolean isBeautiful(String w) {
    for (int i = 0; i < w.length() - 1; ++i) {
        if (w.charAt(i) == w.charAt(i + 1)) {
            return false;
        }
        if (isVowel(w.charAt(i)) && isVowel(w.charAt(i + 1))) {
            return false;
        }
    }
    return true;
}

Notice how I also simplified the check for 2 consecutive vowels so that it now actually reads: if this character is a vowel and the next character is a vowel.

Now the last thing left to do is use this new isBeautiful method. This means we will change this code:

int result = isBeautiful(w);

if (result == 1) {
    System.out.print("Yes");
} else
    System.out.print("No");

to this:

if (isBeautiful(w) {
    System.out.print("Yes");
} else {
    System.out.print("No");
}

Notice I also added the {} after the else. It's best practice to add curly braces after each if/else/for/while.

One last tip is that you can also rewrite the isVowel method like so:

private static final String VOWELS = "aeiouy";

private static boolean isVowel(char c){
    return VOWELS.contains(c);
}

But this doesn't mean that it's bad to check each character separately - it's just an alternative way.


Simulate Turing machine I have written this code to simulate a Turing machine and wonder what could be improved. In particular, the --tape.end() seems a bit dodgy.

#include <array>
#include <iostream>
#include <list>

typedef enum {right, left} Direction;

template<int n>         // n is the number of symbols
struct State {
    std::array<int, n> write;       // Value to write, 0 <= write < n
    std::array<Direction, n> move;  // Direction to move tape
    std::array<State*, n> next;     // Next state, nullptr represents HALT
};

// Some test machines
std::array<State<2>, 5> A = {{
    {{1, 1}, {left, left},   {&A[1],   &A[0]}},
    {{1, 1}, {right, right}, {&A[2],   &A[1]}},
    {{1, 1}, {left, right},  {&A[0],   &A[3]}},
    {{1, 1}, {left, right},  {&A[0],   &A[4]}},
    {{1, 0}, {left, right},  {nullptr, &A[2]}},
}};

std::array<State<2>, 6> B = {{
    {{1, 1}, {left, left},   {&B[1], &B[0]}},
    {{1, 1}, {right, right}, {&B[2], &B[1]}},
    {{0, 1}, {right, right}, {&B[5], &B[3]}},
    {{1, 0}, {left, right},  {&B[0], &B[4]}},
    {{0, 1}, {left, right},  {&B[0], &B[2]}},
    {{1, 1}, {left, left},   {&B[4], nullptr}},
}};

std::array<State<3>, 3> C = {{
    {{1, 1, 2}, {right, left, left},  {&C[1], &C[1], &C[0]}},
    {{1, 1, 2}, {left, right, right}, {&C[0], &C[2], nullptr}},
    {{0, 2, 1}, {left, right, left},  {&C[0], &C[2], &C[2]}},
}};

int main() {
    std::list<int> tape = {0};
    std::list<int>::iterator head = tape.begin();
    State<2>* current_state = &A[0];
    int read;

    unsigned long long int step_count = 0;
    while (true) {
        step_count += 1;
        // Read tape
        read = *head;
        // Write to tape
        *head = current_state->write[read];
        // Move tape head
        if (current_state->move[read] == right) {
            if (head == --tape.end()) {
                tape.push_back(0);
            }
            head++;
        } else if (current_state->move[read] == left) {
            if (head == tape.begin()) {
                tape.push_front(0);
            }
            head--;
        }
        // Change state
        if (current_state->next[read] == nullptr) {
            break;
        } else {
            current_state = current_state->next[read];
        }
    }
    std::cout << "Halted after " << step_count << " steps." << std::endl;
    unsigned int sum = 0;
    for (int it : tape) {
        sum += (it != 0);
    }
    std::cout << "Total non-zeroes: " << sum << std::endl;
    return 0;
}

I believe you're looking for std::prev:

std::prev(tape.end());

The linked page even states the exact comparison of the two:

  Although the expression --c.end() often compiles, it is not guaranteed to do so: c.end() is an rvalue expression, and there is no iterator requirement that specifies that decrement of an rvalue is guaranteed to work. In particular, when iterators are implemented as pointers, --c.end() does not compile, while std::prev(c.end()) does.

I do have some additional points as well:

typedef isn't needed for enums in C++.
Since tape is a storage container of a primitive type (ints), the default constructor will zero-initialize it automatically.  As such, you don't need to assign to {0}.

std::list<int> tape;

In case you want to verify it right away, take a look at this test.
It may be more useful to use auto with head:

auto head = tape.begin();

This allows for less typing and for any iterator type (if it ever changes).
You could put these "test machines" in a separate file or somewhere separate from the main implementation.  Proper unit testing may be preferred, but I don't have much to say about that.  I'd also rename them as such as it's best not to use single-variable names outside of loop counters.

Another note: you can omit the = for each std::array for C++11 list initialization.
main() seems to be handling too much of the logic.  Have other functions handle it and only have main() initialize variables, call these functions, then output the results.
return 0 or similar isn't needed at the very end of main(); it'll always do the same return at this point.


Spaces to tabs converter I wrote a program to convert spaces to tabs. If there are 4 spaces it should convert them to tab. Please let me know how to improve it.

#include <stdio.h>
#define TABVALUE 4
int c, d, s;

int main(void) {
    c = s = d = 0;
    while ((c = getchar()) != EOF) {
        if (c == ' ') {
            s++;
            for (int j = 0; j < TABVALUE - 1; j++) {
                d = getchar();
                if (d == ' ') {
                    s++;
                }
            }
            if (s == TABVALUE) {
                putchar('\t');
                s = 0;
            }
        } else {
            putchar(c);
        }
    }
    return 0;
}

Bugs
j unconditionally loops to TABVALUE - 1. This is wrong:

If d happens to not be a space, it is not printed out.
If next d happens to be a space, it is still counted.

Test against a  b  cdef. The output is a\tf.


Creating branched narratives Recently, I was asked to develop a way to easily create a branched narrative, where the user can select options to progress the story in their own path. After a couple hours of fiddling around with different options I managed to produce this simple code.

### STORY BRANCH MANAGEMENT

class Branch:

    def __init__(self, dialogue, options={}):
        self.dialogue = dialogue
        self.options = options
        self.keys = list(options.keys())
        self.canbranch = options != {}

    def __iter__(self):
        for x in self.keys:
            yield x

    def __str__(self):
        return self.dialogue

    def choose(self, choice):
        return self.options[self.keys[choice]]

story_main = Branch("You find yourself in a room with three doors, which one do you choose?", {
    "Left door":
        Branch("You fell in a hole and died"),
    "Middle door":
        Branch("Oh hey, you win a free Parker pen"),
    "Right door":
        Branch("You tripped over the door frame", {
            "Get up and walk away":
                Branch("Oh well, at least you lived"),
            "Die on the spot":
                Branch("Nevermind...") } ),
    "All of the above":
        Branch("Well I guess that works") } )

### PLAY THE STORY

def play():
    br = story_main

    while br.canbranch:
        # Output the current branch's dialogue
        print(br)

        # Create blank line, then displays the sub-branch choices as an enumerated list
        print("")
        for i,x in enumerate(br):
            print("\t{}) {}".format(i + 1, x))

        # Have the user to select an option
        print("")
        c = -1
        while c not in range(len(br.keys)):
            c = int(input("Select --> ")) - 1

        # Update the current branch to the sub-branch that the user just chose
        br = br.choose(c)
        print("---")

    # If the user reaches a 'final' branch (i.e, no choices), output it's dialogue and close the program after enter is hit.
    input(str(br) + "\n---")

play()

The code itself makes sense I hope, but the main concern I have is with the part in the middle where we can create a story: story_main = Branch( ... ). It seems a bit... strange. I've been told that it is readable, but it doesn't seem very 'pythonic'. I assume this is because of the way I've used dictionaries to store a choice and its corresponding sub-branch as the key and value respectively.

If you can't understand how the story is written, I'll brief over what it does so that it may be easier to grasp. When the program is opened, you're told "You find yourself in a room ... " Then, you are given a list of options, "Left door", "Right door", and so on. If you chose to enter the right door, the program would output "You tripped over the doorframe" and give you some more options, "Get up and walk away" and "Die on the spot". If you chose to die on the spot, it would output "Nevermind..." and the story would end.

This is what the program looks like when run:

So as you can see it works, but is it acceptable to use this method?

EDIT: The answers so far have both recommended to save the story as an external file in some way, and then access it from within the code. I imagine this is for organisation, and it was actually what I originally had in mind. The program from which the code I provided is from did do something like this (although likely less convenient than suggested methods in most respects). If anyone has more suggestions on how storing the narrative could be optimised, that would also be appreciated.

I started reading at play. At first, I thought you had posted non-working code, because your code couldn't possibly work. Then I got up to the definition of Branch and realized that yes, it could work. But it violates so many principles that I don't know what the "real" type of a Branch is.

Let me suggest a slightly different model:

You have an outer loop. The loop ends when the narrative ends. You could express this as either an iteration stop, or as a boolean condition. I'm going to call each step in the narrative a step. You might prefer page or part or section, whatever helps you tell this story:

# Iteration stop:
for step in narrative:
    pass

# Boolean condition:
while not narrative.has_ended:
    step = narrative.current_step

Once you have a step, you can display the narrative text, and then present the user with a collection of options to choose from. Because users are users, there probably needs to be an inner loop that keeps asking for a valid option until one is entered. I would make a function for that:

    # Display the narrative text
    print(step.text)

    # Present the user with options, force a choice:
    choice = get_user_choice(step.options)

Finally, you have interacted with the user and received a choice, so update the narrative. Maybe your iterator has a .select method, or maybe your narrative does:

        step.select(choice)

        # or maybe

        narrative.branch(choice)

After looking at it, I personally prefer the idea of invoking methods on the Narrative. It shows more clearly that the narrative is an object, maintaining and updating its state. However, I would make the menu selection a "simple" cough! enhanced generator (see PEP-342):

from os import PathLike
from typing import Generator, Iterable, None, TextIO

def tell_a_story(datafile: PathLike, output: TextIO,
                 chooser: Generator[str, Iterable[str], None]):
    """
    Tell a branched-narrative story. Configure the narrative using
    "datafile", write the result on "output," interact with "chooser"
    (an enhanced generator) to select branches to follow.

    """
    narrative = Narrative(datafile)

    while not narrative.has_ended():
        step = narrative.current_step
        output.write(step.text)

        # send options to the generator, get a choice back
        choice = chooser.send(step.options)

        narrative.branch(choice)

I chose to use a Writable and an Extended Generator to help with automated testing. You should be able to supply a StringIO and a generator that ignores the options in favor of just iterating over a list of choices, and compare the generated text against what you predict for your test case.

If there is an epilog, you'll want to handle it specially. In particular, your generator should deal with an options parameter that is None. Then you could print the epilog, give the user no options, probably get None back from the generator, select None as your branch, and mark the narrative as ended.

Alternatively, you could create Narrative with some sort of .introduction text, print that outside the loop, and then loop in a different order:

output.write(narrative.introduction)

while not narrative.has_ended():
    choice = chooser(narrative.options)
    narrative.branch(choice)
    output.write(narrative.new_text)

This may tend to make your last loop iteration cleaner, and impose fewer constraints on your generator.

How Enhanced Generators Work

Generators are functions in Python that make use of the yield or yield from statements. We'll focus on yield because that's all we need.

You can create a generator like this:

def g():
    yield 1
    yield 2

You know it's a generator because you see yield. Twice!

Generators don't return a value when you "call" them. Instead, they return a generator object. It's like an iterator - it's an object that you can call next() on and it might return multiple values:

for x in g():
    print(x)

This does what you might expect: it prints

1
2

So you can see that the yield keyword is used to pass data out of a generator, and suspend execution. When the caller invokes next(generator_object) or generator_object.__next__(), execution resumes where it left off - at the yield statement, and more code is executed.

In a first attempt at supporting coroutines, python added generator.send() methods. This allows the caller to push data into the generator, and it works like this:

def eg():
    x = 0
    while True:
        x = yield x + 1

egen = eg()
p = egen.send(None)

for _ in range(5):
    print("P is:", p)
    p = egen.send(p + 3)

The output looks like:

P is: 1
P is: 5
P is: 9
P is: 13
P is: 17

As you can see, this lets you send data into the generator, then pull data back out. You just have to start off by sending a None to "prime the generator".

This is relevant to you because you need to have a 2-way conversation. You want to send out some options, and get back a selection. You could do it like this:

def get_selection(choices):
    print("\n".join(choices))
    return input("What will it be?")

But then how would you test? You'd have to use global variables or something, and where's the fun in that?

But if you built a generator, you could ignore the options and just pre-program a path through the data:

def test_selections(test_data):
    for choice in test_data:
        options = yield choice

chooser = test_selections(["a", "b", "a", "b", "a", "c"])
outfile = io.StringIO()
datafile = "story.txt"

tell_a_story(datafile, outfile, chooser)

Or, you could talk to the user:

def user_selections():
    choice = None
    while True:
        options = yield choice
        print("\n".join(options))
        choice = input("What will it be?")

Depending on whether you want to test or really use it, you just pass a different generator.

Edit (responding to changes in the original question):

First, note that etiquette on CodeReview is to append your updates, not edit them in-place. When you make in-place changes, that invalidates the answers people posted and makes other readers confused.

That said, this version looks considerably better. My main suggestion would be to remove the enumerate from getoptions (let the display code handle the enumeration), and move getoptions into Branch as a method rather than a free function. When you do that, of course, you realize that you could just let the caller access Branch.options directly, which is fine. ;-)

One other suggestion is to make the story an incoming parameter for play(), like:

if __name__ == '__main__':
    play(Story_main)


String separator, similar to strtok I made a string separator below. It's like strtok, but it separates based on entire strings instead of single char delimiters. Any comments/answers on how I could improve the performance or readability would be great. I'm also leaking some memory according to valgrind, but that is only a minor concern for my use case.

It takes two char arrays, a buffer(string?) and a separator. It returns an array of strings containing all of the separated strings. For example, if the buffer is foobar and the separator is foo, it'll return {"bar", NULL}. But if I'm passed foobarfoobarfoobar and the separator is bar, it'll return {"foo", "foo", "foo", NULL}.

Edit: Comments added to clarify intended functionality.

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

char **sepstr(const char *buf, const char *sep){
    int i = 0, j = 0, k = 0, l = 0, stringCount = 0;

    /* If separator > buffer, return NULL */
    while (buf[i]) i++;
    while (sep[j]) j++;
    if (j > i) return NULL;

    char **strings = malloc(0);

    /* While we're not at the end of the buffer */
    while (*(buf+k)){
        /* If the characters match, check to see if it is the separator */
        if (*(buf+k) == *(sep)){
            for (l = 0; l < j; l++){
                /* If it isn't the separator, break */
                if (*(buf+k+l) != *(sep+l)) break;
                /* If it is the separator and the separator isn't the
                    beginning, add buf to buf+k bytes as a string */
                if (l == j-1 && k != 0){
                    strings = realloc(strings, (stringCount+1)*sizeof (char *));
                    strings[stringCount] = malloc(k+1);
                    memcpy(strings[stringCount++], buf, k);
                    buf += (j + k), k = -1;
                /* If it is the separator, but it's the beginning
                    of the string, skip it */
                } else if (l == j-1 && k == 0) {
                    buf += j, k = -1;
                }
            }
        }
        k++;
    }

    /* Add a string for the left over bytes if sep isn't the end */
    if (i != k && *(buf)){
        while (buf[l]) l++;
        strings = realloc(strings, (stringCount+1) * sizeof (char *));
        strings[stringCount] = malloc(l+1);
        memcpy(strings[stringCount++], buf, l);
    }

    /* Append NULL to array of strings */
    strings = realloc(strings, (stringCount+1) * sizeof (char *));
    strings[stringCount] = malloc(sizeof NULL);
    strings[stringCount] = NULL;

    return strings;
}

int main(){
    const char buffer[] = "foobarfoobarfoobar";
    const char separator[] = "bar";
    char **strings = sepstr(buffer, separator);

    while(*strings){
        printf("%s\n", *(strings));
        free(*(strings++));
    }

    free(strings);

    return 0;
}

I'm also leaking some memory according to valgrind, but that is only a minor concern for my use case.

Hmm, I don't really agree. Leaking memory is harmless only if you know the cause (*), and simply choose to not spend time in fixing it. In your case, the cause (for one part, see below) is the print loop at the end of the main:

while(*strings){
    printf("%s\n", *(strings));
    free(*(strings++));  // incrementing strings here
}

free(strings); // string no longer points to the initially allocated bloc: UB

Whether it matters is your problem, but you should not reproduce this code. BTW, it caused a crash on my VS2017...

Now for the function itself:

char** strings = malloc(0); Nothing really bad here because it will be reallocated later, but as it is more of a trick, it really deserves a comment explaining the reason for that line
Your naming convention for local variables (int i = 0, j = 0, k = 0, l = 0, stringCount = 0;) does not help to understand their meaning. After analysis I would prefer:

i -> buflen
j -> seplen
k -> buf_ix  // or whatever you want saying it is intended to be an index of buf
l -> sep_ix

the strings in strings are not null-terminated. This is really bad because there is no way to find the used length: you really should add that terminating null - anyway, you have reserved memory for it...
An additional memory leak when setting last element of strings to NULL:

/* Append NULL to array of strings */
strings = realloc(strings, (stringCount+1) * sizeof (char *));
strings[stringCount] = malloc(sizeof NULL);    // hmm... a dynamic allocation...
strings[stringCount] = NULL;                   // immediately and definitively lost !

this code should become:

/* Append NULL to array of strings */
strings = realloc(strings, (stringCount+1) * sizeof (char *));
strings[stringCount] = NULL;

you should test malloc and realloc return values. Allocation error are uncommon nowadays, but if this code was to run under stressed condition very bad things would occur, because you would be using a null pointer which is Undefined Behaviour (other details on @TobySpeight's answer).

(*) Even if you had identified that the code in main caused a memory leak, it could have masked the other memory leak inside the function the deserved to be fixed. That's the reason why it is bad to leave a memory leak in a program, and even in a test only one!


Fastest way to find all neighbours of a vertex in a graph I'm looking for the fastest way to find all neighbours of a vertex in an undirected Graph.  Please improve on the code if possible.

neighbours[g_Graph, v_] :=
 Module[
  {vl = VertexList[g],
   pos},
  pos = Position[vl, v][[1, 1]];
  Pick[VertexList[g], AdjacencyMatrix[g][[pos]], 1]
 ]

I need it to work fast both for sparse and dense graphs.

It is essential to the performance of this solution that it uses SparseArrays.
This is a bottleneck in my application.  Since my graph is constantly changing, trying to precompute neighbours or using memoization will complicate things significantly.

I'd like to stress that speed is essential here.  Here's a graph to test on:

max = 4000;
c = 0.3;

tg = RandomGraph[{max, Round[c max^2]}];

neighbours[tg, 1]; // Timing

As of Mathematica 9.0 we have the function AdjacencyList[g,v].
Since this is built into Mathematica, I would assume that it is the fastest implementation.

In[1]:= g = CompleteGraph[7];
In[2]:= AdjacencyList[g, 4]
Out[2]= {1, 2, 3, 5, 6, 7}
In[3]:= g = CompleteGraph[{3,4}];
In[4]:= EdgeList[g]
Out[4]= {1 <-> 4, 1 <-> 5, 1 <-> 6, 1 <-> 7, 2 <-> 4, 2 <-> 5, 2 <-> 6, 2 <-> 7, 3 <-> 4, 3 <-> 5, 3 <-> 6, 3 <-> 7}
In[5]:= AdjacencyList[g, 5]
Out[5]= {1, 2, 3}


Reduce the redundancy in my pandas highlight function After a lot of browsing and experimenting I have managed to achieve to read excel file as a Dataframe, apply some style to it and export it to html. My main concern is that the highlight_max function looks a bit redundant. The rows I want to highlight are already predetermined and are not just simply minimum or maximum. I tried to do it with list comprehension but it did not work. Is it possible to return the style value with just one line?

import pandas as pd

def highlight_max(s):
    truth = []
    for i in (df.index):
        truth.append(i in [634, 636, 638, 640, 642, 644, 648, 649, 650, 651, 656])
    return ['background-color: yellow' if v else '' for v in truth]

df = pd.read_excel('N:\Temp\filename.xlsx', index_col=False)
df.reset_index(drop=True, inplace=True)
html = df.style.set_table_styles([{'selector': 'th,td', 'props': [('border-style','solid'),
('border-width','1px')]}]).apply(highlight_max).hide_index().render(index=False)
with open('N://temp//test.html', 'w') as f:
    f.write(html)

Your highlight_max function can be simplified a bit by passing the rows to mark as an argument (possibly with a default value), and putting it all into a list comprehension right away:

def highlight_max(column, to_mark):
    return ['background-color: yellow' if i in to_mark else '' for i in column.index]

Note that this takes the actual column as input. Yours took the df from the global scope, disregarding the input, which means that all things you did before the apply will probably be ignored.

Use it like this:

ROWS = {634, 636, 638, 640, 642, 644, 648, 649, 650, 651, 656}

if __name__ == "__main__":
    df = pd.read_excel('N:\Temp\filename.xlsx', index_col=False)
    df.reset_index(drop=True, inplace=True)
    styles = [{'selector': 'th,td',
               'props': [('border-style','solid'), ('border-width','1px')]}]
    html = df.style.set_table_styles(styles)\
             .apply(highlight_max, to_mark=ROWS)\
             .hide_index()\
             .render(index=False)

    with open('N://temp//test.html', 'w') as f:
        f.write(html)

I also made ROWS a set instead of a list for faster checking of in (not that it matters too much for such a small list, but it will if it gets bigger), used a style that makes the long chain of functions for html easier to read IMO, used the fact that any additional parameters passed to apply are passed along to the function to pass the rows to mark and wrapped the main code under a if __name__ == "__main__": guard to allow importing from this script without it running.


Calculate knight moves This is my first little python project. I want to get to a point where i can solve chess puzzles like this one from puzzling.stackexchange. I want to be able to change the board size to something like 15*15 or 3*7 and still all rules to apply.

So far, you can just run the file and it gives all possible moves for a knight on a random starting position. You can also put it a given position in string form like "A5" for the variable "startKnight".

What can I do better? The letter number swap function to swap between the chess and matrix connotation looks so big and ugly...
The knightmoves function looks good to me, but maybe you can tell me what to improve :)

from random import randint

def letterNumberSwap(x):
    # change letters to numbers and vice versa
    if x == "A":
        return 1
    elif x == "B":
        return 2
    elif x == "C":
        return 3
    elif x == "D":
        return 4
    elif x == "E":
        return 5
    elif x == "F":
        return 6
    elif x == "G":
        return 7
    elif x == "H":
        return 8
    elif x == 1:
        return "A"
    elif x == 2:
        return "B"
    elif x == 3:
        return "C"
    elif x == 4:
        return "D"
    elif x == 5:
        return "E"
    elif x == 6:
        return "F"
    elif x == 7:
        return "G"
    elif x == 8:
        return "H"
    else:
        print("Something is wrong.")

moveList = [] # list of moves in matrix connotation
possibleMoves = [] # list of moves in chess connotation

def knightMoves(position):
    # calculate all knight moves from position
    column, row = list(position.strip().upper())
    column = letterNumberSwap(column)
    c,r = int(column), int(row)
    if (0 < (c - 2) <= 8):
        if (0 < (r - 1) <= 8):
            moveList.append([c - 2, r - 1])
        if (0 < (r + 1) <= 8):
            moveList.append([c - 2, r + 1])
    if (0 < (c - 1) <= 8):
        if (0 < (r - 2) <= 8):
            moveList.append([c - 1, r - 2])
        if (0 < (r + 2) <= 8):
            moveList.append([c - 1, r + 2])
    if (0 < (c + 1) <= 8):
        if (0 < (r - 2) <= 8):
            moveList.append([c + 1, r - 2])
        if (0 < (r + 2) <= 8):
            moveList.append([c + 1, r + 2])
    if (0 < (c + 2) <= 8):
        if (0 < (r - 1) <= 8):
            moveList.append([c + 2, r - 1])
        if (0 < (r + 1) <= 8):
            moveList.append([c + 2, r + 1])
    for entry in moveList:
        # back to chess connotation
        possibleMoves.append(letterNumberSwap(entry[0])+str(entry[1]))

randomStart = str(letterNumberSwap(randint(1,8)))+str(randint(1,8))
startKnight = randomStart
knightMoves(startKnight)
print("A Knight on " + startKnight + " can go to:")
for entry in possibleMoves:
    print(entry,end = ' ')

Review

There are a few things that can be improved upon here,

  def letterNumberSwap(x):

is reinventing the wheel of str.index()
and str[index]

  randomStart = str(letterNumberSwap(randint(1,8)))+str(randint(1,8))

random.choice() might be better choice :)

  It's just implementation details. randint delegates to randrange, so it has another layer of function call overhead, and randrange goes through a lot of argument checking and other crud. In contrast, choice is a really simple one-liner. source

moveList = [] # list of moves in matrix connotation
possibleMoves = [] # list of moves in chess connotation

Stick to the scope.

Do not make these globals,.. either return or yield them from the function

if (0 < (c - 2) <= 8):
    if (0 < (r - 1) <= 8):
        moveList.append([c - 2, r - 1])
    if (0 < (r + 1) <= 8):
        moveList.append([c - 2, r + 1])

Stick to DRY, you can avoid the repetition by saving all possible directions in a list
Don't do string + string but use .format() or use the fancy f"{string}" if you use python 3.6+
in range(x) is alot faster in python3 instead of x <= !! and x >= !! @Graipher
Naming

In chess a row is called a rank, and a collumn is called a file
Avoid magic numbers

Make the WIDTH and HEIGHT constant so that they can easily be changed

Alternative

from string import ascii_uppercase
from random import choice

BOARD_HEIGHT = 5
BOARD_WIDTH = 13
CHESS_FILES = ascii_uppercase[:BOARD_WIDTH]
CHESS_RANKS = range(1, BOARD_HEIGHT+1)

def knight_moves(file, rank):
    position = (CHESS_FILES.index(file), rank)
    directions = ((1, 2), (1, -2), (-1, 2), (-1, -2), (2, 1), (2, -1), (-2, 1), (-2, -1))

    for direction in directions:
        file, rank = (d + p for d, p in zip(direction, position))
        if  file in range(BOARD_WIDTH) and rank in CHESS_RANKS:
            yield f"{CHESS_FILES[file]}{rank}"

if __name__ == '__main__':
    file = choice(CHESS_FILES)
    rank = choice(CHESS_RANKS)

    print(f"With BOARD({BOARD_WIDTH}, {BOARD_HEIGHT})")
    print(f"A knight on {file}{rank} can go to:")
    print(", ".join(knight_moves(file, rank)))


Merge sort with a linked list I'd like this code reviewed.

import java.util.NoSuchElementException;

public class MergeSort {

    private Node first;
    private Node last;

    private static class Node {
        int element;
        Node next;
        Node(int element, Node node) {
            this.element = element;
            this.next = node;
        }
    }

    public void displayList() {
        Node tempFirst = first;
        while (tempFirst != null) {
            System.out.print(tempFirst.element + " ");
            tempFirst = tempFirst.next;
        }
    }

    public void add (int val) {
        final Node l = last;
        final Node newNode = new Node(val, null);
        last = newNode;
        if (first == null) {
            first = newNode;
        } else {
            l.next = newNode;
        }
    }

    public void sort() {
        /**
         * QQ: should there be an exception check for last == null also ?
         */
        if (first == null) {
            throw new NoSuchElementException("The linkedlist doesnot contain any node.");
        }

        first = divide(first, last);
    }

    private Node getMidPoint(Node node) {
        assert node != null;

        Node fast = node.next;
        Node slow = node;

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

    private Node divide(Node node1, Node node2) {
        if (node1 == node2) return node1;

        Node midPoint = getMidPoint(node1);
        Node midpointNext = midPoint.next;
        midPoint.next = null;

        Node n1 = divide(node1, midPoint);
        Node n2 = divide(midpointNext, node2);
        return mergeLinkedList(n1, n2);
    }

    private Node mergeLinkedList(Node node1, Node node2) {
        assert node1 != null;
        assert node2 != null;

        Node current = null;
        Node currentHead = null;

        while (node1 != null && node2 != null) {

            if (node1.element < node2.element) {
                if (current == null) {
                    currentHead = node1;
                    current = node1;
                } else {
                    current.next = node1;
                    current = current.next;
                }
                node1 = node1.next;
            } else {
                if (current == null) {
                    currentHead = node2;
                    current = node2;
                } else {
                    current.next = node2;
                    current = current.next;
                }
                node2 = node2.next;
            }
        }

        current.next = node1 != null ? node1 : node2;

        return currentHead;
    }

    public static void main(String args[]) {
        // odd size list
        MergeSort ms1 = new MergeSort();
        int[] a1 = {5, 4, 3, 2, 1};
        for (int i : a1) {
            ms1.add(i);
        }
        ms1.sort();
        ms1.displayList();

        System.out.println();

        // even size list.
        MergeSort ms2 = new MergeSort();
        int[] a2 = {5, 4, 3, 2};
        for (int i : a2) {
            ms2.add(i);
        }
        ms2.sort();
        ms2.displayList();

        System.out.println();

        // list of size 1
        MergeSort ms3 = new MergeSort();
        int[] a3 = {1};
        for (int i : a3) {
            ms3.add(i);
        }
        ms3.sort();
        ms3.displayList();

        System.out.println();

        // list of size 2
        MergeSort ms4 = new MergeSort();
        int[] a4 = {2, 1};
        for (int i : a4) {
            ms4.add(i);
        }
        ms4.sort();
        ms4.displayList();

        System.out.println();

    }
}

Starting with some Java API documentation:

  public class NoSuchElementException
  extends RuntimeException

  Thrown by the nextElement method of an Enumeration to indicate that there are no more elements in the enumeration.

This is not an Enumeration and you have not called nextElement().  This is not an appropriate exception

sort() should really take a list as a parameter and return a sorted list.  Give it a list as a parameter; if a null is passed, Java will do you a nice NullPointerException, free of charge.  If the given list is simply empty, return an empty list.  Being asked to sort an empty list is not an exceptional error situation.  It's no more an error than being asked to sort a list with 1 element, which you do show you can do.

Which brings me to the more important stuff...

Method and object conflated and confused.

You've combined a sorting algorithm with the (hand-rolled) list to be sorted.  It would be better to have a properly usable list class and either a mergeSort method on that class or a static helper class with a MergeSort method that can act on any list object.  Then you could create a list, examine it to show that it was a well-functioning and populated list, create a mergeSort()-ed version, check that this was a valid list, that it still had all the right elements (none missing, none added or substituted) and that they were in the desired order.  You'd have something useful you could do more with.

List object cannot be tested/examined.

This is part of the problem created by the previous issue.  There's no way to get at the list data.  You print it out on the screen, but it's inaccessible to the rest of your code.  What use was the sort?  Did it even work?  How is this validated except by the naked eye?  Since you don't print the unsorted version out on the screen, how does the viewer know you did anything?

Make the list fully usable.  Provide a way to traverse and inspect it.  Then your code can verify the sort (and do something useful with the sorted data afterwards, if you like).

Printing directly from within the merge class.

Don't do this.  Have your class methods return useful values which can be inspected and tested.  Print out the results in your main method if you want, but don't litter your classes with println statements.

Final, what?

What do you think you are gaining with those final keywords?  Those local variables are going to evaporate when that tiny method returns, so what have you achieved?

I should go on to examine the actual sorting algorithm but I think I'll leave that to somebody else - it's late and I'm tired.


Displaying a certain element among inputted numbers I prompt the user for 9 numbers, store those numbers in 9 pointers and display the element 2. It seem to work just fine, but am I forgetting something? Can I improve it but keep it simple? I'm just a beginner testing what I have learned so far.

int n;
int *array[9];
bool isUsed[10] = {0};

for(int i = 0; i < 9; i++)
{
    cout << "Enter Number " << (i + 1) << endl;
    cin >> n;
    if((n >= 0) && (n <= 9))
    {
        if (isUsed[n] == false)
        {
            array[i] = new int;
            *array[i] = n;
            isUsed[n] = true;
        }

        else
        {
            cout << "Number has already been used." << endl;
            i--;
        }
    }

    else
    {
        cout << "Numbers from 0-9 only." << endl;
        i--;
    }

}

    cout << *array[2]<<endl;

You can definitely improve it, by replacing the array of pointers with a std::vector<int>.

If you must track usage, I suggest using a map instead, where the keys are 0...9 and the values exist only if the respective index was added.


Getting integer values and one CSS class name Here, my function is getting four integer values and one CSS class name. Based on their equality I'm creating few more CSS proprties. If all four elements passed are different then I'm using default CSS properties.

How can make this code elegant? I wanted to get rid of if else conditions.

Note: I tried with switch statements too but I felt it's the same as if else loops.

twoEqualElementsCss(cssClass1, cssClass2) {
var dict = {};
dict[cssClass1] = { padding: '12, 17' };
dict[cssClass2] = { padding: '12, 17', offset: '-2' };
return dict;
}

threeEqualElementsCss(cssClass1, cssClass2, cssClass3) {
var dict = {};
dict[cssClass1] = { padding: '12, 30', offset: '1' };
dict[cssClass2] = { padding: '12, 30', offset: '-14' };
dict[cssClass3] = { padding: '12, 30', offset: '-28' };
return dict;
}

 fourEqualElementsCss(cssClass1, cssClass2, cssClass3, cssClass4) {
var dict = {};
dict[cssClass1] = { padding: '12, 30', offset: '1' };
dict[cssClass2] = { padding: '12, 30', offset: '-14' };
dict[cssClass3] = { padding: '12, 30', offset: '-28' };
dict[cssClass3] = { padding: '12, 30', offset: '-42' };
return dict;
}

  //drawSomething is called this way from other function, which pass four values(integer) and one of the elements css class
 drawSomething(23, 26, 26, 26, element2-css);
 drawSomething(23, 26, 26, 26, element3-css);
 drawSomething(23, 26, 26, 26, element4-css);
 drawSomething(23, 26, 26, 26, element1-css);

 drawSomething(element1,element2, element3, element4, cssClassName) {

let padding = '5, 3' //default value  when all four elements are different
let offset = 100 //default value when all four elements are different
var equalElementDictionary = {}

// when 4 elements are same integer value
if (element1 == element2 && element2 == element3 && element3 == element4)
{
    //Note that each element has it's own predefined css class e.g element1 has element1-css class
    equalElementDictionary = this.fourEqualElementsCss('element1-css', 'element2-css', 'element3-css', 'element4-css')
}
else if ((element1 == element2 && element2 === element4)) { // when 3 elements are equal.
    equalElementDictionary = this.threeEqualElementsCss('element1-css', 'element2-css', 'element4-css')
}
else if ((element1 == element3 && element3 === element4)) {
    equalElementDictionary = this.threeEqualElementsCss('element1-css', 'element3-css', 'element4-css')
}
else if ((element2 == element3 && element3 === element4)) {
    equalElementDictionary = this.threeEqualElementsCss('element2-css', 'element3-css', 'element4-css')
}
else if (element4 == element1) { // when 2 elements are equal.
    equalElementDictionary = this.twoEqualElementsCss('element4-css', 'element1-css')
}
else if (element2 == element1) {
    equalElementDictionary = this.twoEqualElementsCss('element2-css', 'element1-css')
}
else if (element4 == element3) {
    equalElementDictionary = this.twoEqualElementsCss('element4-css', 'element3-css')
}
else if (element4 == element2) {
    equalElementDictionary = this.twoEqualElementsCss('element4-css', 'element2-css')
}

// when dicitionary does not have cssClassName that means all four elements are different and default values are passed to testFunction
if (equalElementDictionary[cssClassName] != undefined) {
      padding = equalElementDictionary[cssClassName].padding
  if (equalElementDictionary[cssClassName].offset != undefined) {
    padding = equalElementDictionary[cssClassName].offset
  }
}

testFunction(offset, padding)
}

First off, the question. Because some functionality is missing, I can't test my proposed solution. Try to ask self contained questions.

A possible alternative approach is to analyze the 4 integer and keep track.

function keepTrack(info, integer, cssClass){
  info[integer] = info[integer] || [];
  info[integer].push(cssClass);
}

info = {};
keepTrack(info, element1, 'element1-css');
keepTrack(info, element2, 'element2-css');
keepTrack(info, element3, 'element3-css');
keepTrack(info, element4, 'element4-css');

This way you have a list of items for each, you can just check how many css items there are per length and call the correct function with spread syntax.

for(integer in info){
  //You could use a switch here
  if(info[integer].length == 4)
    fourEqualElementsCss(...info[integer]);
  else if(info[integer].length == 3)
    threeEqualElementsCss(...info[integer]);
  else if(info[integer].length == 2)
    twoEqualElementsCss(...info[integer]);
}


Parsing psql output into multiple files I wrote the following simple Perl script to read lines from stdin that are the output from a psql call that returns lines of the form key1 | key2 | long text field and create a separate output file for each line, whose name is key1_key2_NNN.txt, where NNN is just a counter to ensure unique file names.

my $count = 0;

while (<>) {
    if (/(.*)\|(.*)\|(.*)/) {
        $count++;
        my $outname = "$1_$2_$count.txt";
        my $text = $3;
        $outname =~ s/\s+//g;
        my $outfile = new IO::File("> $outname");
        $outfile->print("$text");
    }
}

This does the trick, but there are 2 things I'm not thrilled about

I'd like to be able to do the $count++ inline while putting the incremented number into the string, but I'm not sure how to not just get 0++ plugged in (i.e. how to make it know that the ++ is a command and not part of the literal string).
I don't like that I have to save $3 to another variable, but if I don't, it gets cleared out before I need it.

Anyhow, any suggestions on making this a bit more slick?

I don't know if this is the complete script, but the absence of use strict; use warnings is pretty noticeable. Always use these as a basic safety net, no matter how small or simple your program.

Your usage of the regex /(.*)\|(.*)\|(.*)/ can be improved in some ways:

Don't use a regex to extract the data. Instead: my @cols = split /[|]/, $_, 3;. This really is the correct way to go, the following points however deal with normal regex usage.
Some guides recommend putting special characters into a character class [|] instead of escaping them \|. It's my experience that this makes complicated regexes easier to read.
Immediately after matching, assign the values of the captures to permanent variables. Otherwise, they could be modified by a subroutine you call – the next match can clear those values. Note also that ways exist to booby trap such an innocent-looking operation like incrementing an integer to clear the last match.

So we either write code such as:

if (/(.*)[|](.*)[|](.*)/) {
  my @fields = ($1, $2, $3);
  ...
}

or we don't use the capture variables at all, and use the regex in list context instead:

if (my @fields = /(.*)[|](.*)[|](.*)/) { ... }

Technically, this isn't absolutely equivalent (e.g. in case you aren't using any captures, or if some captures are conditional, or …), but it's far better style.

If course, you'd assign to a list of names rather than an array:

my ($name_a, $name_b, $text) = ...

The substitution s/\s+//g might be considered slightly silly, as it's equivalent to s/\s//g. The former variant has the advantage of less substitution operations, so it might actually be preferable. But why are you sanitizing the $outname rather than the parts of which it's made up? Note that you can apply a substitution to multiple variables like

 s/\s+//g for $x, $y;

If you don't want to remove all space in those strings (remember that most filesystems can deal with spaces in filenames all right) but only at the start or beginning of the string, you might want to split with a different separator instead:

split /\s*[|]\s*/, ...

The expression "$1_$2_$count.txt" does not contain a bug, but this is accidental. For example, "$foo_$bar" is equivalent to $foo_ . $bar – underscores can be part of variable names, but variables can't start with numbers, so the problem isn't visible here (variable names can consist solely of numbers, and all such variables are reserved for capture groups).

As a fix, use curly braces to delimit the variable names: "${1}_${2}_$count.txt", or use sprintf:

sprintf '%s_%s_%d.txt', $1, $2, $count

The line my $outfile = new IO::File("> $outname"); is a big no-no for two reasons:

Don't use the “indirect object notation” method $object @arglist. Instead use $object->method(@arglist). The former variant may look nicer, but is often ambiguous, has rather confusing precedence, and widely considered to be a syntactic mistake.

If you need help to get rid of that habit, you may enjoy the no indirect pragma.
What you're doing is basically open my $outfile, "> $outname". Use the three-argument form of open:

open my $outfile, ">", $outname

or in the object-oriented wrapper: IO::File->new($outname, '>').

The next problem here is that you aren't doing any error handling! Check the return value of the constructor to assert that you could actually open the file. Well, calling a method on an undefined value will die, but checking manually allows you to output a sensible error message.

The $outfile->print("$text") would usually be written print { $outfile } $text. For one thing, the object-oriented interface to file handles is used very rarely. You can use the normal interface directly. Also, you are unnecessarily stringifying the $text. If it isn't already a string, that will be managed by print. Note that explicit stringification creates a copy of that value, something which you would usually (but not always) want to avoid.

Then, you are using print, not say. This means (together with the fact that in a regex, . does not match newlines), that you won't end your output to the file with a newline. This is probably an oversight.

Another aspect that just came to my mind is that your input file could contain slashes, allowing a filename like /you/were/pwned/_key2_1234.txt to be created (assuming the path already exists). Let's put in a bit of validation.

The script now looks like:

use strict;
use warnings;
use autodie;  # easier than explicit, manual error handling
use feature qw/say/; # say is like print, but appends newline

my $count = 0;
while (my $line = <>) {
    chomp $line;
    my ($name_a, $name_b, $text) = split /[|]/, $line, 3
      or next;
    for ($name_a, $name_b) {
        s/\s+//g;
        die qq(The first two columns in "$line" may not contain slashes) if m[/];
    }
    my $filename = sprintf '%s_%s_%d.txt', $name_a, $name_b, ++$count;
    open my $fh, ">", $filename;
    say { $fh } $text;
}

How did this solve your problems?

Use sprintf to format a string with a variable you want to increment at the same time
Assigning captures to variables is a best practice, and should always be done.

As 200_success mentioned, it is rather suspect that you are using a command-line tool to interface with a database. Perl has the excellent DBI modules which provides a common frontend for various SQL databases. The docs have some simple examples which should get you rapidly started. Using such a module is safer (less escaping levels) and faster (no extra process, no tempfiles, prepared statements, …). Have fun exploring all the great DBI features!


SQL Dependency with Broker I have written a windows service for one of our local servers. This service works like a gem on my local machine, does what it's supposed to (ACCP to exchange db data), but I'm not overly familiar with dependencies. Is this an appropriate way to receive/send a message?

string DBP3_US = DBP3_US;
string PING_DEPENDENCY = "SELECT [SomeColumn] FROM [SomeTable];";
protected override void OnStart(string[] args)
{
    SqlDependency.Start(DBP3_US);
    Thread Ping_US = new Thread(PingThread);
    Ping_US.Name = "ping_US";
    Ping_US.Start();
}

private void PingThread()
{
    CreateCommandWithDependency(PING_DEPENDENCY, Ping_OnChange);
}

private void CreateCommandWithDependency(string queryText, OnChangeEventHandler e, string db = DBP3_US)
{
    using (SqlConnection con = new SqlConnection(db))
    using (SqlCommand cmd = con.CreateCommand())
    {
        cmd.CommandText = queryText;
        cmd.CommandType = CommandType.Text;
        cmd.Notification = null;
        SqlDependency sqlDep = new SqlDependency(cmd);
        sqlDep.OnChange += new OnChangeEventHandler(e);
        con.Open();
        cmd.ExecuteNonQuery();
    }
}

private void Ping_OnChange(object sender, SqlNotificationEventArgs e)
{
    PingDependency();
    Thread Ping_US = new Thread(PingThread);
    Ping_US.Start();
}

private void PingDependency(string db = DBP3_US)
{
    // Do whatever operations are required.
}

I refactored your service base code into another class, which raises an event DataChanged. This way you can subscribe to multiple queries and databases. I also created a SqlDatabaseDepedency class which starts and stops the SqlDepedency for a database and implemented in a singleton pattern using unity (singleton on dbName). I also used Microsoft Enterprise Library's data access block for better database code. Let me know what you think.

public class SqlDepedencyBroker
{
    public event EventHandler DataChanged;
    public string Query { get; private set; }
    private string DbName { get; private set; }
    private CancellationToken Token { get; private set; }
    public SqlDepedencyBroker(string query, string dbName = null, CancellationToken token = default(CancellationToken))
    {
        SqlDatabaseDepedency.RegisterDependency(dbName);
        this.Query = query;
        this.DbName = dbName;
        this.Token = token;
    }
    public async Task WatchQuery()
    {
        await Task.Yield();
        SqlDatabase db = this.DbName == null ? DatabaseFactory.CreateDatabase() as SqlDatabase : DatabaseFactory.CreateDatabase(this.DbName) as SqlDatabase;
        if (db == null)
            throw new InvalidOperationException();
        var cmd = db.GetSqlStringCommand(this.Query) as SqlCommand;
        if (cmd == null)
            throw new InvalidOperationException();
        cmd.Notification = null;
        SqlDependency dep = new SqlDependency(cmd);
        dep.OnChange += dep_OnChange;
        Token.ThrowIfCancellationRequested();
        db.ExecuteNonQuery(cmd);
        Token.ThrowIfCancellationRequested();
    }

    void dep_OnChange(object sender, SqlNotificationEventArgs e)
    {
        Token.ThrowIfCancellationRequested();
        var dataChanged = this.DataChanged;
        if (dataChanged != null)
            dataChanged(this, EventArgs.Empty);
        Token.ThrowIfCancellationRequested();
        Task t = WatchQuery();
    }
}
public class SqlDatabaseDepedency
{
    public string DbName { get; private set; }
    private string ConnectionString { get; set; }
    private SqlDatabaseDepedency(string dbName = null)
    {
        var db = dbName != null ? DatabaseFactory.CreateDatabase(dbName) : DatabaseFactory.CreateDatabase();
        if (db == null)
            throw new NotImplementedException();
        ConnectionString = db.ConnectionString;
        SqlDependency.Start(ConnectionString);
    }
    private static Lazy<IUnityContainer> _container = new Lazy<IUnityContainer>(() => new UnityContainer());
    private static IUnityContainer Container
    {
        get { return _container.Value; }
    }
    public static void RegisterDependency(string dbName = null)
    {
        lock (Container)
        {
            if (!Container.IsRegistered<SqlDatabaseDepedency>(dbName))
                Container.RegisterInstance<SqlDatabaseDepedency>(dbName, new SqlDatabaseDepedency(dbName));
        }
    }
    ~SqlDatabaseDepedency()
    {
        SqlDependency.Stop(ConnectionString);
    }
}


c# Efficient solution: Sequence generator for a pattern This is an interview question to generate the below pattern of numbers in a sequence, which is: 12.34, 23.45, 45.67, 78.910, 1112.1314, 1617.1819, 2223.2425, 2930.3132, 3738.3940, 4647.4849, 5657.5859, 6768.6970, 7980.8182, 9293.9495, 106107.108109, 121122.123124 We can use starting seed number as 12 and have to generate all the 16 numbers programmatically without using strings. We can use only arrays but no other built-in collection classes or data structures.
public static void GenerateSequence()
{
    int j = 1;
    ArrayList sequenceList = new ArrayList();

    for (int count = 0; count < 16; count++)
    {
        j = j + count;
        int quotent = getValue(j, j + 1);
        int reminder = getValue(j + 2, j + 3);
        int length = getLength(reminder);
        double result = Convert.ToUInt32(quotent) * getLength(reminder) + reminder;
        sequenceList.Add(result / length);
    }
    Console.WriteLine(sequenceList.ToString());
}

static int getValue(int a, int b)
{
    return a * getLength(b) + b;
}

static int getLength(int value)
{
    int length = (int)(Math.Log10(value) + 1);
    int formattedValue = 10;

    for (int i = 1; i < length; i++)
        formattedValue = formattedValue * 10;

    return formattedValue;
}

I've arrived with above solution but this looks complex and less efficient. Looking for an alternate approach with better time/space complexity.

public static void GenerateSequence() {
    double tenPower(double n) => Math.Pow(10, Math.Floor(Math.Log10(n)+1));
    double build(double n) => n * tenPower(n + 1) + n + 1;
    double merge(double n1, double n2) => n1 + n2 / tenPower(n2);

    var n = 1;
    var ans = new double[16];
    for (int j1 = 0; j1 < 16; ++j1) {
        n += j1;

        var whole = build(n);
        var frac = build(n + 2);
        ans[j1] = merge(whole, frac);
    }

    foreach (var a in ans)
        Console.WriteLine(a);
}


Substitution cipher algorithm performance boost This algorithm is meant to read a string of numbers on an input, a naive substitution cipher code (A = 1, B = 2, ..., Z = 26) and output the number of ways the code could be interpreted (e.g. 25114 could mean 'BEAN', ‘BEAAD’, ‘YAAD’, ‘YAN’, ‘YKD’ and ‘BEKD’, hence output is 6).

It is working properly, but not fast enough, though.  Is there a way to improve its performance? I commented it heavily, so it is easy to read.

#include <iostream>
#include <string>

using namespace std;

//Those are global, because handled by both functions
int ways;       //number of ways the code could be interpreted
string s;       //input

//Takes account of all the tuples after the one on position "start" using
//recursion
void findNextPossible(size_t start) {
    if(start + 2 <= s.length()){
        //Tuple considered
        string ss = s.substr(start, 2);
        //Otherwise not interpretable as a letter
        if(ss <= "26") {
            ways++;
            for(size_t i = 0; i <= s.length(); i++) findNextPossible(start+2+i);
        }
    }
}

int main() {
    size_t pos;
    bool test, someDeletedInTheMiddle;
    //Until zero on input
    while(cin >> s, s.at(0) != '0') {
        ways = 0;
        test = true;
        someDeletedInTheMiddle = false;
        pos = s.find('0');
        //In this while I look for zeros
        while(pos != string::npos) {
            //Code with 30,40,... is not valid -> output 0
            if(s.at(pos - 1) > '2') {
                test = false;
                break;
            }
            //If delete some in the middle, output smaller by 1
            if((pos >= 2 && pos < s.length()-1) && s.length() > 2) someDeletedInTheMiddle = true;
            //Don't cosider the tuple with zero in it anymore
            s.erase(pos-1, 2);
            //Any other zero?
            pos = s.find('0', pos);
        }
        if(test) {
            if(!someDeletedInTheMiddle) ways++;
            //Process the rest
            for(size_t i = 0; i < s.length(); i++) {
                findNextPossible(i);
            }
        }
        cout << ways << endl;
    }
    return 0;
}

You should use memoization on findNextPossible(), otherwise your algorithm has exponential complexity.

Also: the optimization obtained by looking for '0's will be no more useful once you use memoization... so the resulting code should become much, much smaller.

This is a possible implementation:

#include <string>
#include <map>
#include <iostream>
#include <cassert>

using namespace std;

int ways(std::string s) {
  static map<string,int> cache;
  map<string,int>::const_iterator f = cache.find(s);
  if (f!=cache.end()) return f->second;
  //  cout<<"ways("<<s<<")"<<endl;
  if (s.size()==0) return 1;
  if (s[0]=='0') return 0; // no way the first digit is 0
  if (!isdigit(s[0])) return 0;
  if (s.size()==1) return 1; // only one possibility for a single digit
  // here we have at least 2 digits
  if (!isdigit(s[1])) return 0;
  int n = 10*(s[0]-'0')+(s[1]-'0');
  if (n>26) return cache[s] = ways(s.substr(1)); // cannot merge first two digits.
  return cache[s] = ways(s.substr(1)) + ways(s.substr(2)); // two possible interpretations
}

const char *tests[]={"1213","12","214","205",0};

int main() {
  for (int i = 0; tests[i]; ++i) {
    cout<<tests[i]<<": "<<ways(tests[i])<<endl;
  }
}


Creating custom objects with custom properties using generics I have two base classes: City and Building:

public class City
{
    public string Name { get; set; }
    public double Area { get; set; }
}

public class Building
{
    public string Name { get; set; }
    public double Area { get; set; }
    public int Stories { get; set; }
}

With two classes that inherit from Building, with their own Id property:

public class MyBuilding : Building
{
    public int MyId { get; set; }
}

public class HisBuilding : Building
{
    public int HisId { get; set; }
}

And two classes that inherit from City, with their own Id and IEnumerable<Building> properties:

public class MyCity : City
{
    public int MyId { get; set; }
    public IEnumerable<MyBuilding> Buildings { get; set; }
}

public class HisCity : City
{
    public int HisId { get; set; }
    public IEnumerable<HisBuilding> Buildings { get; set; }
}

Below is a method I've created that returns a KeyValuePair<City, Building> using generics:

public static KeyValuePair<TCity, IEnumerable<TBuilding>> GetData<TCity, TBuilding>()
    where TCity : City, new()
    where TBuilding : Building, new()
{
    TCity city = new TCity();
    IEnumerable<TBuilding> buildings = new List<TBuilding>();
    return new KeyValuePair<TCity, IEnumerable<TBuilding>>(city, buildings);
}

And in my Main method I've logic that instantiates a specific City object based on a user's input.

int input = -1;
string strInput = Console.ReadLine();
int.TryParse(strInput, out input);
string json = string.Empty;
switch (input)
{
    case 1:
        KeyValuePair<MyCity, IEnumerable<MyBuilding>> myCity
            = GetData<MyCity, MyBuilding>();
        MyCity my = myCity.Key;
        my.MyId = 1;
        my.Buildings = myCity.Value;
        json = JsonConvert.SerializeObject(my, Formatting.Indented);
        break;

    default:
        KeyValuePair<HisCity, IEnumerable<HisBuilding>> hisCity
            = GetData<HisCity, HisBuilding>();
        HisCity his = hisCity.Key;
        his.HisId = 2;
        his.Buildings = hisCity.Value;
        json = JsonConvert.SerializeObject(his, Formatting.Indented);
        break;
}
Console.WriteLine(json);
Console.ReadLine();

The above generic method is just something I cobbled together in the last half hour, but I'm wondering if there is a more efficient way of creating these custom objects of mine, especially when each City has its own, different IEnumerable<Building> property.

EDIT

Here is the code I have for building the IEnumerable inside my GetData method:

public static IEnumerable<TBuilding> GetBuildingData<TBuilding>()
    where TBuilding : Building, new()
{
    DataTable table;
    string myConnectionString = @"Data Source=(LocalDb)\MSSQLLocalDb;Initial Catalog=Test;Integrated Security=True;";
    List<TBuilding> buildings = new List<TBuilding>();
    using (SqlConnection connection = new SqlConnection(myConnectionString))
    {
        using (SqlCommand command = new SqlCommand("[dbo].[GetListData]", connection) { CommandType = CommandType.StoredProcedure })
        {
            connection.Open();
            using (SqlDataAdapter adapter = new SqlDataAdapter(command))
            {
                table = new DataTable();
                adapter.Fill(table);
            }
            connection.Close();
        }
    }
    foreach (DataRow row in table.Rows)
    {
        TBuilding building = new TBuilding();
        building.Area = Convert.ToDouble(row["Area"]);
        building.Name = row["Name"].ToString();
        building.Stories = Convert.ToInt32(row["Stories"]);
        buildings.Add(building);
    }
    return buildings;
}

I will focus on..

  Creating custom objects with custom properties using generics

Which you do in this method..

public static KeyValuePair<TCity, IEnumerable<TBuilding>> GetData<TCity, TBuilding>()
    where TCity : City, new()
    where TBuilding : Building, new()
{
    TCity city = new TCity();
    IEnumerable<TBuilding> buildings = new List<TBuilding>();
    return new KeyValuePair<TCity, IEnumerable<TBuilding>>(city, buildings);
}

A KeyValuePair is archaic, you can use a ValueTuple nowadays
TBuilding does not require the new() constraint in this method
The choice for IEnumerable over IList is questionable, since you retrieve the items later on and have no way of adding them to an IEnumerable. So why create the IEnumerable in this method?
The name should reflect what you are doing. CreateCity seems more appropriate.

I would refactor this method..

public static (TCity city, IList<TBuilding> buildings) CreateCity<TCity, TBuilding>()
    where TCity : City, new()
    where TBuilding : Building
{
    var city = new TCity();
    var buildings = new List<TBuilding>();
    return (city, buildings);
}

EDIT:

Thinking about the refactored method. This doesn't even need to be a method anymore. It doesn't do anything but create objects for the types you specify.

What I would really do is throw out this ancient way of ORM, and use an existing API (EF, NHibernate, Dapper).


Printing a Φ pattern using * characters So I am a newbie obviously, This was part of a small assignment I had to do for Uni, and it works alright. It just reads a number from the console and then outputs the Greek letter Φ (sort of). Sure it could use some outofbound exceptions etc, but my question is this: My code overall (not just in this example) seems a little bit messy to me after I read it. My variable names are messy and I need to work on that and perhaps a few methods here and there, even though this assignment wasn't about methods but rather a warm up of basic Java syntax.

I want to write better code, but I don't know where to start.Should I use more variables? I am not greatly familiar with JavaDocs yet, should they be used for small programs as well? How much procedures is too much? Should I break it down more? I feel like my "algorithm" even though it works it could be better. Online tutorials usually teach syntax but I want to make my code as good and readable as possible.

import java.util.*;

public class A11{

    public static void main(String args[]){

        /*Declare Num, then parse the number from the command line
        into the variable,and finaly inform the user before moving on*/
        int temp=0;
        int num =0;
        num=Integer.parseInt(args[0]);
        System.out.println();
        System.out.println("Number = " + num);
        temp=num/2;

        /*Prints the first Line (approp spaces and a Star)*/
        if(num%2==0){
            temp-=1;
        }
        for(int v=0;v<=temp;v++){

            System.out.print(" ");
        }
        System.out.println("*");

        /*Print the remaining Shape according to the input*/

        /* Loop algorithm to calculate and output the appropreate amount of
        characters required to form the TOP half of the shape */
        if(num%2==0){temp=num/2-1;}

        for(int i=1;i<=temp;i++){

            for(int j=0;j<temp-i;j++){
                System.out.print(" ");
            }
            System.out.print("*");
            for(int k=0;k<2;k++){
                for(int j=0;j<i;j++){
                    System.out.print(" ");
                }
                System.out.print("*");
            }
            System.out.println();
        }
    /*Bottom half - if Num is an odd number flip the top shape but loose one line*/
        int space=temp;
         if(num%2==1){
            temp-=1;
         }
        for(int i=temp;i>=1;i--){

            for(int j=space-i;j>0;j--){
                System.out.print(" ");
            }
            System.out.print("*");
            for(int k=0;k<2;k++){
                for(int j=0;j<i;j++){
                    System.out.print(" ");
                }
                System.out.print("*");
}
            System.out.println();
        }
        temp=num/2;
        /*Prints the last line and Star*/
        if(num%2==0){
            temp-=1;
        }
        for(int v=0;v<=temp;v++){

            System.out.print(" ");
        }
        System.out.println("*");
    }
}

Your use of the temp variable makes your code particularly infuriating to follow:

It's not even that temporary in scope: it's used throughout the function!
temp is almost never a good name for a variable, since it doesn't give any clue what it is supposed to represent.
You redefine and repurpose it several times within the function.
There are too many if(num%2==0) special cases.  You shouldn't need any special cases if you just use the truncation that naturally happens with integer division in your favour.

For that matter, num is also a meaningless variable name.  I can see that it's an int, which is, obviously, a number.  How about size instead?

There's a lot of copy-and-paste code.  You could benefit greatly splitting the work into functions.  Decomposing the task would also force you to think about what the subroutines are, and help you organize your code.

Calling System.out.print() to print each character at a time is very tedious.  It's also bad for performance, if that matters to you.  A much better strategy would be to create a buffer to print at least a line at a time, if not the entire picture at once.  Another benefit of using a buffer is that you can just place the stars in the right place using a formula.

Suggested solution

import java.util.Arrays;

public class Phi {
    /**
     * Produces a string of spaces that has the character c
     * at the three specified indices, followed by a newline.
     */
    private static String row(char c, int left, int mid, int right) {
        char[] line = new char[right + 2];
        Arrays.fill(line, ' ');
        line[left] = line[mid] = line[right] = c;
        line[right + 1] = '\n';
        return new String(line);
    }

    public static String toString(char c, int size) {
        StringBuilder out = new StringBuilder();
        int halfWidth = (size + 1) / 2;

        // Top stem
        out.append(row(c, halfWidth, halfWidth, halfWidth));

        // Body
        for (int row = 2; row <= size / 2; row++) {
            out.append(row(c, halfWidth - row, halfWidth, halfWidth + row));
        }
        for (int row = halfWidth; row >= 2; row--) {
            out.append(row(c, halfWidth - row, halfWidth, halfWidth + row));
        }

        // Bottom stem
        out.append(row(c, halfWidth, halfWidth, halfWidth));

        return out.toString();
    }

    public static void main(String[] args) {
        int size = Integer.parseInt(args[0]);
        System.out.printf("\nNumber = %d\n%s", size, toString('*', size));
    }
}