Determine hour in other countries with generated random time I hope the title is clear. The objective is as following:

  There are 4 clocks in a hotel in London. One is showing the time for
  London. The others for New York, Tokyo and Hong Kong.

  Generate a random time for the clock in London, with that result,
  determine the hour for New York(+5), Tokyo(-9) and Hong Kong(-8). Then
  show the results.

This is not homework, this is practice. Is this a good way? Or can i generate a random timespan? All input will be appreciated!

Random generator = new Random();

DateTime london = DateTime.Now;

DateTime timeInLondon = london.AddMinutes(generator.Next());

DateTime timeInNewYork = timeInLondon.AddHours(5);
DateTime timeInTokyo = timeInLondon.AddHours(-9);
DateTime timeInHongKong = timeInLondon.AddHours(-8);

Console.WriteLine("London: " +london.ToShortTimeString() +"\n" +"New York: " +timeInNewYork.ToShortTimeString() + "\n" +"Tokyo: " +timeInTokyo.ToShortTimeString() + "\n" +"Hong Kong: " +timeInHongKong.ToShortTimeString());

DateTime london = DateTime.Now;

DateTime timeInLondon = london.AddMinutes(generator.Next());

This can be rewritten as

var timeInLondon = DateTime.Now.AddMinutes(generator.Next());

You can use var on the other lines as well.

Removing the london variable makes it less likely you make mistakes like this:

Console.WriteLine("London: " +london.ToShortTimeString()...

Where you actually output DateTime.Now as it was at the start of your program, but not, as you probably wanted, the randomized timeInLondon!

Now about your random time.

Your seconds will obviously not be random. Apart from that, you only add a positive random number, with a maximum of around 4085 years. As such, I guess that is enough to make the resulting time more or less random as far as hours and minutes are concerned. I would opt for adding seconds though. You still would add anything up to 68 years to the current time, still probably ending up with a random-enough distribution over the 24-hour clock, including seconds:

var timeInLondon = DateTime.Now.AddSeconds(generator.Next());

Finally, if you want to nitpick, it would be better to use time zone information instead of hard-coding magic numbers like 5 or -8. Time zones do not at all times have the same number of hours difference between them(*).

You could use TimeZoneInfo.ConvertTime Method (DateTime, TimeZoneInfo, TimeZoneInfo)

Keep in mind that if you opt for that, you might need to rethink randomisation of your time, since the date comes into play to determine the time in other places.

(*)Not all countries start and end DST on the same dates, some countries do not even use DST, and on the southern hemisphere, DST goes "the other way around". This may all be irrelevant for the locations you picked, though.


Java Concurrent Multimap I've implemented a concurrent multimap that support concurrent put and remove but I'm unsure of its correctness. I would appreciate your feedback.

public class ConcurrentMultiMap<K, V> {
    private static ConcurrentHashMap emptySet = new ConcurrentHashMap();
    private ConcurrentHashMap<K, ConcurrentHashMap<V, Object>> map = new ConcurrentHashMap<>();

    // return true if the method increased the size of the multimap or false if the multimap already contained the key-value pair
    public boolean put(K k, V v) {
        Preconditions.checkNotNull(k);
        Preconditions.checkNotNull(v);
        boolean tryAgain;
        boolean success = true;
        do {
            ConcurrentHashMap<V, Object> oldSet = map.get(k);
            if (oldSet == null) {
                tryAgain = map.putIfAbsent(k, new ConcurrentHashMap<>(Collections.singeltonMap(v, 1))) != null;
            } else {
                success = oldSet.putIfAbsent(v, 1) == null; //I do not allow multiple equal values for the same key
                if (success) {
                    // if the put was a success we check that the key was not removed from the map by a concurrent remove operation.
                    ConcurrentHashMap<V, Object> currentSet = map.get(k);
                    tryAgain = currentSet == null || !currentSet.contains(v);
                } else {
                    return false;
                }
            } while(tryAgain);
            return success;
    }

    // returns true if the multimap changed
    public boolean remove(K k, V v) {
        Preconditions.checkNotNull(k);
        Preconditions.checkNotNull(v);
        ConcurrentHashMap<V, Object> oldSet = map.get(k);
        if (oldSet == null) {
            return false;
        }
        if (oldSet.remove(v) != null) {
            if (oldSet.size() == 0) {
                map.remove(k, emptySet);
            }
            return true;
        }
        return false;
    }
}

You should have HashMap<K, Set<V>>, not ConcurrentHashMap<K, ConcurrentHashMap<V, Object>>.


Unit test for a CreateUser() method using JustMock lite I'm new to unit testing and have created a unit test for a create user action method and it's passing the test. I'm not sure if this is the right way to unit test a create method.

This is my test method:

[TestMethod]
public void Create_User()
{
    Mapper.CreateMap<UserViewModel, User>();

    //Arrange
    var membershipRepository = Mock.Create<IStaticMembershipService>();
    var userMock = Mock.Create<MembershipUser>();
    Mock.Arrange(() => userMock.ProviderUserKey).Returns(1);
    Mock.Arrange(() => membershipRepository.GetUser()).Returns(userMock);
    var UserRepository = Mock.Create<IUserRepository>();

    MemoryStream stream = new MemoryStream();
    StreamWriter writer = new StreamWriter(stream);
    writer.Write("Creating Streams for Testing");
    writer.Flush();
    stream.Position = 0;
    var mockFile = Mock.Create<HttpPostedFileBase>();
    Mock.Arrange(() => mockFile.ContentType).Returns("application/pdf");
    Mock.Arrange(() => mockFile.FileName).Returns("Test.pdf");
    Mock.Arrange(() => mockFile.InputStream).Returns(stream);
    Mock.Arrange(() => mockFile.ContentLength).Returns(1);
    UserViewModel mockUserViewModel = new UserViewModel { Description = "TestDesc", Title = "Tester", UserId = 1, File = mockFile };

    Mock.Arrange(() => UserRepository.AddUser(Arg.IsAny<User>())).Returns(new OperationStatus{Status = true});
    //Act
    UserController controller = new UserController(UserRepository, membershipRepository);
    RedirectToRouteResult actionResult = (RedirectToRouteResult)controller.Create(mockUserViewModel);

    //Assert
    Assert.AreEqual(actionResult.RouteValues["Action"], "Index");
}

This is the action method:

[HttpPost]
[ValidateAntiForgeryToken]
public ActionResult Create(UserViewModel item)
{
    if (ModelState.IsValid)
    {
        var newUser = Mapper.Map<UserViewModel, User>(item);
        //Populates file name and content type to newUser
        UserHelper.PopulateFile(item.File, newUser);

        newUser.UserId = (int)_membershipService.GetUser().ProviderUserKey;

        var opStatus = _UserRepository.AddUser(newUser);
        if (!opStatus.Status)
            throw new System.Web.Http.HttpResponseException(new   HttpResponseMessage(HttpStatusCode.InternalServerError));

        return RedirectToAction("Index");
    }
    else
        return View(item);
}

Any relevant controller info:

private IUserRepository _UserRepository;
private IStaticMembershipService _membershipService;

public UserController(IUserRepository UserRepo, IStaticMembershipService membershipService)
{
    _UserRepository = UserRepo;
    _membershipService = membershipService;
}

IUserRepository:

public interface IUserRepository
{
    List<User> GetAllUsers(int Id);

    OperationStatus AddUser(User newUser);

    User GetUserById(int Id);
}

UserRepository containing definitions of above interfaces:

public List<User> GetAllUsers(int Id)
{
    using (DataContext)
    {
        return DataContext.Users.Where(j => j.Id == Id).ToList();
    }
}

public Journal GetUserById(int Id)
{
    using (DataContext)
        return DataContext.Users.SingleOrDefault(j => j.UserId == Id);
}

public OperationStatus AddUser(Journal newUser)
{
    var opStatus = new OperationStatus { Status = true };
    try
    {
        using (DataContext)
        {
            newUser.ModifiedDate = DateTime.Now;
            var j = DataContext.Users.Add(newUser);
            DataContext.SaveChanges();
        }
    }
    catch (Exception e)
    {
        //opStatus = OperationStatus.CreateFromException("Error adding user: ", e);
    }

    return opStatus;
}

The test is running fine. I'm just not sure if it's the right way for unit testing. Please let me know if I'm following the right approach here.

I think you are not testing everything. The API should create a user but the only thing you assert is the ActionResult

Assert.AreEqual(actionResult.RouteValues["Action"], "Index");

So the method runs and the result is correct. But has it actually created a new user and add it to the repository? How about this operation?

var opStatus = _UserRepository.AddUser(newUser);

IMO you should also check the repository to see whether the API correctly added the new user to it. I don't know what interfaces the UserRepository might implement but for this

var UserRepository = Mock.Create<IUserRepository>();

assertions like these ones could be appropriate (just examples) or anything else that makes sense for the repository.

Assert.AreEqual(1, UserRepository.Count);
Assert.AreEqual(referenceUser, UserRepository.GetUser(abc));

What if someone had a problem with the repository and commented it out and actually forgot to add #if DEBUG or made any other changes ignoring the status-code?

public ActionResult Create(UserViewModel item)
{
    if (ModelState.IsValid)
    {
        var newUser = Mapper.Map<UserViewModel, User>(item);
        //Populates file name and content type to newUser
        UserHelper.PopulateFile(item.File, newUser);

        newUser.UserId = (int)_membershipService.GetUser().ProviderUserKey;

        // I temporarily disabled the repository because I didn't work for me.
        //var opStatus = _UserRepository.AddUser(newUser);
        //if (!opStatus.Status)
        //    throw new System.Web.Http.HttpResponseException(new   HttpResponseMessage(HttpStatusCode.InternalServerError));

        return RedirectToAction("Index");
    }
    else
        return View(item);
}

The test would still pass but the result would be wrong.


Tic Tac Toe game in C++ 11 It's time for my second simple game - Tic Tac Toc. I checked it on my Linux and it's working. For compiling it I used g++ with -std=c++11. I will be glad for any advice for the future.

#include <iostream>
#include <cstdlib>
#include <unistd.h>

class Game_Board
{
public:
    Game_Board();

private:
    enum Player{cross=1, circle=2};
    Player which_player = cross;
    int table_board[3][3];
    bool end = false;

//drawing board
    void draw_board(int const table_board[3][3])
    {
        system("clear");
        for (int i = 0; i<3; i++)
        {
            std::cout << "\n";
            for (int j = 0; j<3; j++)
            {
            if(table_board[j][i]==1)
            {
                std::cout << " _X_ ";
            }
            else if(table_board[j][i]==2)
            {
                std::cout << " _O_ ";
            } else
            {
            std::cout << " ___ ";
            }
            }
            std::cout << "\n\n";
        }
    }

//reading player move
    void read_move()
    {
        int *p_moveX = new int;
        int *p_moveY = new int;
        std::cout << "\nChoose a field (Player " << which_player << "): ";
        std::cin >> *p_moveX >> *p_moveY;
        write_move(*p_moveX, *p_moveY, which_player);
        delete p_moveX; p_moveX = NULL;
        delete p_moveY; p_moveY = NULL;
    }

//saving player's move to table
    void write_move( int X, int Y, int plr_number)
    {
        if(table_board[X-1][Y-1]!=0 || X<=0 || Y<=0 || X>3 || Y>3)
        {
            std::cout << "You can't do this!";
            read_move();
        }
        else
        {
        table_board[X-1][Y-1] = plr_number;
        }
    }

//checking end of game
    void check_winner()
    {
    int *end_counter = new int;
    *end_counter = 0;
        for( int i = 0; i<3; i++)
        {
            if((table_board[i][0] & table_board[i][1] & table_board[i][2])!=0 ||(table_board[0][i] & table_board[1][i] & table_board[2][i])!=0 || (table_board[i][0] & table_board[i+1][1] & table_board[i+2][2])!=0 || (table_board[i][2] & table_board[i+1][1] & table_board[i+2][0])!=0)
            {
                switch(which_player)
                {
                    case 1:
                    draw_board(table_board);
                    std::cout << "Player 1 wins!\n";
                    end = true;
                    break;

                    case 2:
                    draw_board(table_board);
                    std::cout << "Player 2 wins!\n";
                    end = true;
                    break;
                }

            }
        }
        for( int i = 0; i<3; i++)
        {
            if(table_board[i][0]!=0 && table_board[i][1]!=0 && table_board[i][2]!=0)
            {
                    *end_counter+=1;
                    if(*end_counter==3)
                    {
                    draw_board(table_board);
                    std::cout << "Only God wins!\n";
                    end = true;
                    }
            }
        }
        delete end_counter;
        end_counter = NULL;
    }

//choosing player
    void choose_player()
    {
        if(which_player==1)
        {
            which_player = circle;
        } else {
            which_player = cross;
        }
    }

//clearing table
    void clear_table()
    {
        for(int i=0; i<3; i++)
        {
            for(int j=0; j<3; j++)
            {
                table_board[i][j]=0;
            }
        }
    }
};

Game_Board::Game_Board()
{
    clear_table();
    while(!end)
    {
    draw_board(table_board);
    read_move();
    check_winner();
    choose_player();
    }
}

int main()
{
    Game_Board game;
    return 0;
}

I would recommend making a main game loop. Running a loop in a constructor breaks the fundamental contract of a constructor: to construct something. This change will also allow you to easily restart the game and keep track of score if you wish. With that said, adding a StartGame, RestartGame and EndGame functionality may help organize things.

I tend to shy away from enums. If I find myself needing an enum, I always end up realizing I actually want a class. This follows the S in SOLID: single responsibility.

There is a line of code in check_winner that goes well over 100 columns. I would wrap the line after every few boolean operators in an if statement like this.

if((table_board[i][0] & table_board[i][1] & table_board[i][2])!=0
  || (table_board[0][i] & table_board[1][i] & table_board[2][i])!=0
  || (table_board[i][0] & table_board[i+1][1] & table_board[i+2][2])!=0
  || (table_board[i][2] & table_board[i+1][1] & table_board[i+2][0])!=0)
{
...
}

You seem to be using the new operator where you don't need to. All of this can be done on the stack with less lines of code and less work for the processor. Every time you call new, the CPU has to find new space on the heap, construct the datatype and return the pointer to the memory address. Considering how small this program is, it's likely all your data could fit in the registry directly on the CPU.

void read_move()
{
    int moveX = 0;
    int moveY = 0;
    std::cout << "\nChoose a field (Player " << which_player << "): ";
    std::cin >> moveX >> moveY;
    write_move(moveX, moveY, which_player);
}

check_winner also has a similar problem.

draw_board does not need to take in a table_board because it is already accessible as a member, also I would avoid giving function parameter the same names as members. I'd recommend putting a lowercase 'm' before your member variables and using pascal case.

After the appropriate code changes, you shouldn't have any pointers in your code, but just as a side note:

nullptr is now the standard for assigning pointers to null in C++11. nullptr is of type std::nullptr_t where NULL is an int.


Insertion sort in Common Lisp This is my first bit of significant code in Common Lisp, an implementation of insertion sort. Being new to Lisp in general, I'm wondering if this code follows best Lisp practices in regards to program design.

;;; Return a portion of a list
(defun slice (elements from-index to-index)
  (cond
    ((= from-index to-index) nil)
    (t (cons (nth from-index elements) (slice elements (+ from-index 1) to-index)))))

;;; Return a list omitting a portion
(defun splice (elements from-index to-index)
  (append (slice elements 0 from-index) (slice elements to-index (length elements))))

;;; Move element in list to new index, return new list
(defun move (elements from-index to-index)
  (let ((spliced (splice elements from-index (+ from-index 1))))
    (append (slice spliced 0 to-index) (list (nth from-index elements)) (slice spliced to-index (length spliced)))))

;;; Find the determined (via predicate) correct index for an element in a list
(defun find-ordered-index (elements predicate index &optional (key index))
  (cond
    ((or (= index 0) (= key 0)) 0)
    ((funcall predicate (nth (- key 1) elements) (nth index elements)) (find-ordered-index elements predicate index (- key 1)))
    (t key)))

;;; Return sorted list
(defun insertion-sort (elements predicate &optional (index 0))
  (cond
    ((= index (length elements)) elements)
    (t (insertion-sort (move elements index (find-ordered-index elements predicate index)) predicate (+ index 1)))))

Trivial

Use 1- instead of (- ... 1).

Avoid very long lines (Emacs will indent for you).

Do not use cond when a single if without progn would do.

Memory

Use nconc instead of
append when possible to avoid
unnecessary consing (in your case, splice allocates a fresh list, so
its result can be passed to nconc).

Catastrophic

Whenever you use nth, you are
using the wrong algorithm.

Optimal search is linearithmic: O(n*log(n)).

Insert search is quadratic: O(n^2).

Your implementation is O(n^3):

nth is O(n)
slice is O(n^2) (calls nth * recursion).
splice and move are also O(n^2) (call slice).
find-ordered-index is O(n).
insertion-sort is O(n^3) (calls move * recursion).


Optimization of AngularJS function to load images on the page I have two AngularJS function in my dataContext object that receive data from a WebAPI:

(function () {
'use strict';

var serviceId = 'datacontext';
angular.module('app').factory(serviceId,
    ['common', datacontext]);

function datacontext(common) {
    var service = {

        getAllTagsByHttp: getAllTagsByHttp,
        getBlogPostsByHttp: getBlogPostsByHttp,
        getAllUserPicByHttp: getAllUserPicByHttp

    };
    return service;

    function getAllTagsByHttp() {
       return common.$http.get('/api/TagApi/alltags');
    }

    function getBlogPostsByHttp() {
        return common.$http.get('/api/BlogPostApi/AllBlogPosts');
    }

    function getAllUserPicByHttp() {
        return common.$http.get('/api/BlogPostApi/AllUserPic');
    }
}})();

I have an AngularJS controller that calls these API functions from the datacontext to receive information to display back to the user. The getblogPost function is called first because the next function needs the data from this to operate on.  The second function does a comparison on the first function's data to display back the img source to the user.

The problem I am having is that it takes quite a few seconds for the second function to receive the data and complete its operation. The amount of data being send back is a few  records.  Is there any improvements that can be made to the controller or dataContext object to reduce the time or improve the code?

(function () {
'use strict';
var controllerId = 'search';
angular
    .module('app')
    .controller(controllerId, ['common','datacontext', search]);

function search(common, datacontext) {
    var getLogFn = common.logger.getLogFn;
    var log = getLogFn(controllerId);

    var vm = this;
    vm.title = 'search';
    vm.picData = [];

    activate();

    function activate() {
        var promises = [getBlogPosts(), getUserPicData(), getUserPicUrl()];
        common.activateController(promises, controllerId)
            .then(function () { log('Search View'); });
    }

    function getBlogPosts() {
        datacontext.getBlogPostsByHttp().then(function (response) {
            vm.data = response.data;
        });
    }
    function getUserPicData() {
        datacontext.getAllUserPicByHttp().then(function (response) {
            vm.picData = response.data;

            for (var i = 0; vm.data.length > i; i++) {
                for (var x = 0; vm.picData.length > x; x++) {
                    if (vm.data[i].UserName == vm.picData[x].UserName) {
                        vm.data[i].PictureUrl = vm.picData[x].PictureUrl;
                       // console.log(vm.data[i].PictureUrl);
                        break;
                    }

                }
           }
        });
    }

   function getUserPicUrl(username) {
        getUserPicData();
        for (var i = 0; vm.picData.length < i; i++) {
            if (vm.picData[i].UserName === username) {
                return vm.picData[i].PictureUrl;
                }
            }
            return "not found";
    }
}})();

Personally, I would create a 4th service that returns all data in 1 go and be done with it.

Other than, I would return the array in the first block immediately. Secondly, I would probably go for object notation for something that simple:

function datacontext(common) {
    return {
        getAllTagsByHttp: function getAllTagsByHttp() {
          return common.$http.get('/api/TagApi/alltags');
        },
        getBlogPostsByHttp: function getBlogPostsByHttp() {
          return common.$http.get('/api/BlogPostApi/AllBlogPosts');
        },
        getAllUserPicByHttp:     function getAllUserPicByHttp() {
          return common.$http.get('/api/BlogPostApi/AllUserPic');
        }
    };
}

Furthemore, for this part: The problem i am having is that it takes quite a few seconds for the second function to receive the data and an complete its operation. You really should use the Chrome Developer Tools (or their Firefox counterpart) and figure out what is taking so long ? ( Latency, server side processing, or JavaScript processing ). If the problem is on the server side then it makes even more sense to have only 1 API call (that's what Netflix does)


VBA code to underline numbers in the header takes significant time and sometimes crashes excel Background:

For several of my workbooks I run Overview sheets with charts in them.  When printing the Overview sheet, I put my workbook name as header, where the workbook name includes the criteria for the generating the source data (e.g., date range and time stamp plus non-numeric strings).

To draw attention to these numbers/dates/times, I can manually highlight on the print out, but when others print (using a button on the Overview sheet) they don't seem to remember to do that.

I am looking to add some "flare" to my header by underlining the numeric values.

Issue:

The workbook name character length varies, so I assess character by character.  In some cases the code executes after a delay (time varies, but will say that I had a 60 character length name take ~30 seconds), and other times Excel simply doesn't complete execution after several minutes (i will press escape and get the "code execution has been interupted" msgbox; this recently happened for a 48 character name).

Question:

As this is working code (sometimes), would you please help critique and possibly help resolve the time delay?

If my approach is inferior, I am open to suggestions, though I understand that, from my time on StackOverflow, subjective questions tend to be closed.

Code in question:

Option Explicit

Sub underline_numbers_in_header()
    Dim i As Long, n As String, z As String
    n = ActiveWorkbook.Name
    'Debug.Print Len(n)
    For i = 1 To Len(n)
        If IsNumeric(Mid(n, i, 1)) Then
            z = z & "&U" & Mid(n, i, 1) & "&U"
        Else
            z = z & Mid(n, i, 1)
        End If
        ActiveSheet.PageSetup.CenterHeader = "" & z & ""
    Next i
End Sub

ActiveSheet.PageSetup.CenterHeader = "" & z & ""

There's no reason to run this instruction at every single iteration as z is still in the process of being concatenated, is there?

Move it out of the loop and assign PageSetup.CenterHeader once, only when you've figured out the entire final string value - this could have a dramatically positive impact on performance :)

The name of the procedure/macro should not have underscores in it; convention is to use PascalCase for procedure/member names.

The procedure is implicitly Public and could use an explicit access modifier.

Reading the name of the procedure I was expecting it to go through page headers and underline whatever numeric values are in there... but that's not what's happening, and the variable names aren't helping much. Take the time to spell them out, and thank yourself later!

That said I'd suggest taking in a Workbook parameter and removing the ActiveWorkbook and ActiveSheet dependencies, which force any VBA code calling this procedure to Select and Activate things, which as you know isn't ideal.

Actually, the procedure has too many responsibilities, that's why its name feels wrong. It should be a function that takes a String argument and returns the formatted string: that's what I would expect a "make numbers underline" procedure to do. Then another procedure can be responsible for knowing what worksheet to interact with, and for setting its PageSetup.CenterHeader:

Dim headerText As String
headerText = UnderlineNumbers(ActiveWorkbook.Name)
ActiveSheet.PageSetup.CenterHeader = headerText

The string-valued Mid$ function should work slightly better than the similar but variant-valued Mid function, and when the current character is being pulled from the string in 3 places, it's time to introduce a local variable ;-)

This should be pretty much instant:

Private Function UnderlineNumbers(ByVal value As String) As String
    Dim result As String
    Dim i As Long
    For i = 1 To Len(value)
        Dim current As String
        current = Mid$(value, i, 1)
        If IsNumeric(current) Then
            result = result & "&U" & current & "&U"
        Else
            result = result & current
        End If
    Next
    UnderlineNumbers = result
End Function


Replace spaces in string import re

def replace_token_regex(s, token=" "):
    return re.sub(token, '20%', s.strip())

def replace_token_inplace(s, token=" "):

    for index, char in enumerate(s):
        if ord(char) == ord(token):
            s[index] = '20%'
    return s

print replace_spaces_regex("Foo Bar ")
s = list("Foo Bar ")
replace_spaces_inplace(s)
print ''.join(s)

The run time complexity of the above code is \$O(n)\$, can it be further optimized? or is there any better way to do the above computation?

If you want to avoid replace, a faster method would just split and join. This is faster simply because .split and .join are fast:

"20%".join(string.split(" "))

For a more thorough review, I'll point out that your functions aren't equivalent. The first strips whitespace and the second doesn't. One of them must be wrong!

In the second case:

def replace_token_inplace(s, token=" "):
    for index, char in enumerate(s):
        if ord(char) == ord(token):
            s[index] = '20%'
    return s

you are doing several non-idiomatic things. For one, you are mutating and returning a list.
It's better to just not return it if you mutate:

def replace_token_inplace(s, token=" "):
    for index, char in enumerate(s):
        if ord(char) == ord(token):
            s[index] = '20%'

Secondly, it'll probably be faster to do a copying transform:

def replace_token_inplace(s, token=" "):
    for char in s:
        if ord(char) == ord(token):
            yield '20%'
        else:
            yield char

which can also be written

def replace_token_inplace(s, token=" "):
    for char in s:
        yield '20%' if ord(char) == ord(token) else char

or even

def replace_token_inplace(s, token=" "):
    return ('20%' if ord(char) == ord(token) else char for char in s)

If you want to return a list, use square brackets instead of round ones.


Collecting friends' wall posts Originally, I had everything in the base method #read_wall(). I'm not sure what happened with the array, but when I tried:

array = result + second + third + fourth. I was left with data from only the original result. So I created this working disaster. Can you please help me refactor this?

   # Gets the user's Wall
      def read_wall(fbuserid)
         result ||= graph.get_connections(fbuserid, 'feed')
      end

  def second_wall(fbuserid)
     result ||= graph.get_connections(fbuserid, 'feed')
     second ||= result.next_page
  end

  def third_wall(fbuserid)
       result ||= graph.get_connections(fbuserid, 'feed')
       second ||= result.next_page
       third ||= second.next_page
  end

  def fourth_wall(fbuserid)
        result ||= graph.get_connections(fbuserid, 'feed')
        second ||= result.next_page
        third ||= second.next_page
        fourth ||= third.next_page
   end

  # Collects your friends' wall Posts and puts the IDs into an array
  def get_post_ids(fbuserid)
     x ||= read_wall(fbuserid)
     var = []
     for i in 0..25
        if find_nil(x, [i,'id']).nil? == false
           var << x[i]['id']
        end
     end

     second_wall ||= second_wall(fbuserid)
       for i in 0..25
          if find_nil(second_wall, [i,'id']).nil? == false
             var << second_wall[i]['id']
          end
       end

      third_wall ||= third_wall(fbuserid)
        for i in 0..25
           if find_nil(third_wall, [i,'id']).nil? == false
              var << third_wall[i]['id']
           end
        end

      fourth_wall ||= fourth_wall(fbuserid)
         for i in 0..25
            if find_nil(fourth_wall, [i,'id']).nil? == false
                 var << fourth_wall[i]['id']
            end
         end

      @get_post_ids = var
  end

def fourth_wall(fbuserid)
  result ||= graph.get_connections(fbuserid, 'feed')
  second ||= result.next_page
  third ||= second.next_page
  fourth ||= third.next_page
end

A couple of problems here: result, second, third and fourth are all local variables. This means they won't persist after the method call is finished and they will never persist after the method call is finished. So:

You don't need to set the fourth variable as you never use it in the method and it won't be available after
You don't need ||= as the variables will never be set anyway.

I.e. your code is equivalent to this:

def fourth_wall(fbuserid)
  result = graph.get_connections(fbuserid, 'feed')
  second = result.next_page
  third = second.next_page
  third.next_page
end

Or just:

def fourth_wall(fbuserid)
  graph.get_connections(fbuserid, 'feed').next_page.next_page.next_page
end

The same goes for the other wall methods.

Further your fourth_wall method contains the entire code of the third_wall method (which contains the entire second_wall method and so on). This is bad style. Just let the methods call each other:

def fourth_wall(fbuserid)
  third_wall(fbuserid).next_page
end

(And again the same for the other wall methods of course)

However since you seem to only call those functions together (i.e. you never use fourth_wall without having used third_wall before) it seems much more sensible to have one method which returns all for walls:

def four_walls(fbuserid)
  first = graph.get_connections(fbuserid, 'feed')
  # For more than 4 pages the following should be replaced by a loop
  second = first.next_page
  third = second.next_page
  fourth = third.next_page
  [first, second, third, fourth]
end

In your get_post_ids method you're repeating the same code four times. This is again bad style. You're also creating an empty array and then appending to it from a loop. This is often (but not always) a sign of bad code in ruby as well. Also using x and var as variable names is not a good idea and having a variable name that starts with @get_ just seems weird and confusing.

I'm also assuming that your find_nil method is supposed to check whether either wall[i] or wall[i]['id'] are nil. There are better ways to do this.

In this case you can use map on the walls and then the numbers from 0 to 25 to create an array of user ids for each wall, then use compact to remove nils and flatten to turn the array into one single array rather than an array of four subarray. Thus your whole get_post_ids method can be replaced with this:

def retrieve_post_ids(fbuserid)
  walls = four_walls(fbuserid)
  @post_ids = walls.map do |wall|
    # Iterate over the first 26 entries in wall
    (0..25).map do |i|
      wall[i] && wall[i]['id']
     end.compact
  end.flatten
end

Though if the walls allow slicing similar to arrays, this would be better:

def retrieve_post_ids(fbuserid)
  walls = four_walls(fbuserid)
  @post_ids = walls.map do |wall|
    # Iterate over the first 26 entries in wall
    wall[0..25].map do |entry|
      entry && entry['id']
     end.compact
  end.flatten
end

If the wall never contains more than 26 elements, you don't even need the [0..25] and you can just use well.map.

However it occurs to me that the reason you needed the nil check at all was that each wall contains 25 elements and you got nil for index 25 because you did not realize that ranges using .. are inclusive. In that case you can just use:

def retrieve_post_ids(fbuserid)
  walls = four_walls(fbuserid)
  @post_ids = walls.map do |wall|
    # Iterate over the entries in wall
    wall.map do |entry|
      entry['id']
     end
  end.flatten
end


Removing duplicates in a char[] O(n) We're given a char[] containing characters A-Z. (For more characters this can easily be extended). Now we're asked to remove all duplicates in the char[].

For example, in: \$['B','A','A']\$, out \$['B','A']\$.

The algorithm:

public void removeDuplicates(final char[] s) {
  if (s == null || s.length < 2) {
    return;
  }

  int checker = 0;
  int j = 0;
  for (int i = 0; i < s.length; i++) {
    if ((checker & 1 << s[i] - 'A') == 0) {
      s[j] = s[i];
      j++;
      checker |= 1 << s[i] - 'A';
    }
  }

  while (j < s.length) {
    s[j] = 0;
    j++;
  }
}

Here, checker is an int whose bits represent whether we've seen a character before. For example, if we've seen A and C, checker would be 5 (101).

i is our next character to check, and for \$j > 0\$, \$[s[0], s[1], ... , s[j-1]]\$ contains no duplicate characters.

Worst case: \$2n-1 = O(n)\$

Test cases:

null -> null
[] -> []
['A'] -> ['A']
['A','B'] -> ['A','B']
['A','A'] -> ['A']
['B','A','A'] -> ['B','A']
['A','B','C','D'] -> ['A','B','C','D']
['A','B','A','D'] -> ['A','B','D']
['A','B','A','D','D','A'] -> ['A','B','D']

I am looking for any flaws, mistakes and opinions in general.

Caveat: Unicode has been around for over 25 years. Unicode's 110,000 characters are four orders of magnitude more than the 26 under consideration. In the messy real world, any interesting problem involving character processing is highly likely to bump into it..

Data Structures

At scale, the bit by bit data representation is impractical because parsing 110,000 bit integers [or even 256 bit integers representing all of ASCII] is likely to be much harder than the original problem (i.e. implementing consistent 256 bit integer arithmetic is non-trival).

This is not to say that the general problem is overly difficult. To the contrary, it is well understood because it is so common. A common data structure addressing this class of problems is the Hash Table because hash tables have constant time [O(1)] lookup. For large ranges of keys, the overhead of implementing a Hash Table is often overwhelmed by the savings in lookup.

Implementation

Hash Table data structures are available as a robust Java implementation:

Hash Tables are available directly in Java via HashMap.

A pseudo-code implementation:

forEach character in characters
   myHashTable.insert(character, TRUE)

return myHashTable.keys.toArray()

Performance

For special applications an approach such as that described in the question may offer better performance than a generalized approach using HashMap. A possible  alternative in such cases might be Bloom Filters. A field tested Java implementation of Bloom Filters is available via Google's Guava library.


Guess a number, any number, between 1 and 10 I am learning Ruby and would like to know how I am doing as far as writing the actual code.

Please keep in mind that this is my first Ruby script program, but I am trying to learn ruby in a very short period of time.

I would like to know about nitpicks in the language that every Ruby writer should now, nuances in syntax, and other Ruby specific ... things.

$randomNumber = rand(10) + 1

def guessRandomNumber
    puts "Guess a number between 1 and 10"
    guess = gets
    if Integer(guess) == $randomNumber
        puts "Right on, good guess"
    elsif Integer(guess) < $randomNumber
        puts "too low"
        puts "please, guess again"
        guessRandomNumber
    elsif Integer(guess) > $randomNumber
        puts "too high"
        puts "please, guess again"
        guessRandomNumber
    else
        puts "is that even a number? please, guess again"
        guessRandomNumber
    end
end

guessRandomNumber

puts "bye"
gets

Global variables

Don't use global variables(like $bar). It is always bad practice because you can mutate the global variable from anywhere. The alternative is:
RANDOM_NUMBER = rand(1..10), notice that you can pass range as an argument of rand. rand(10) + 1 can return 11 so it isn't correct.

Style

Huge percentage of the Ruby community follow the style guide and this
is the most popular one Ruby style guide.

Style issues:

Use snake case for the names of methods and variables.
Use double quotes only if you use interpolation or special symbols like \n.

About the implementation

IMO will be more readable like this:

def guess_random_number
  puts 'Guess a number between 1 and 10'

  random_number = rand(1..10)

  while random_number != (guess_number = gets.to_i)
    if guess_number < random_number
      puts "too low \nplease, guess again"
    else
      puts "too high \nplease, guess again"
    end
  end

  puts "Right on, good guess \nbye"
end

guess_random_number

You see there is no need to expose the variable in the global scope.


Basic python calculator I have a young friend of mine who is starting to learn Python in school and asked me to give him a little assignment. I am in no way a teacher nor a Python expert, but I accepted.
At first I thought it would be fun to start with a bit of parsing for the operation's input, like:
Enter your operation : 3+3

But it seemed a bit overwhelming for him so we agreed to separate it in three parts (first number, operand and second number).
I did a little correction but I find it clumsy and the point of the exercise is to show him some good practices.
So here is my code:
calculate = True
while calculate:
    try:
        number1 = float(input("Enter the first number : "))
    except ValueError:
            print("Incorrect value")
            exit()
    symbol = input("Enter the operation symbol (+,-,/,*,%) : ")
    try:
        number2 = float(input("Enter the second number : "))
    except ValueError:
            print("Incorrect value")
            exit()
    operande = ["+", "-", "*", "/", "%"]
    resSentence = "Result of operation \"{} {} {}\" is :".format(number1, symbol, number2)
    if symbol not in operande:
        print("Incorrect symbol")
    elif symbol == "+":
        print(resSentence, number1 + number2)
    elif symbol == "-":
        print(resSentence, number1 - number2)
    elif symbol == "*":
        print(resSentence, number1 * number2)
    elif symbol == "/":
        print(resSentence, number1 / number2)
    elif symbol == "%":
        print(resSentence, number1 % number2)
    restart = input("Do you want to do another calcul (Y/n) ? ")
    while restart.lower() != "y" and restart.lower() != "n":
        print(restart.lower(), restart.lower(), restart.lower()=="n")
        restart = input("Please, enter \"y\" to continue or \"n\" to exit the program : ")
    if restart.lower() == "n":
        calculate = False

I would have wanted to do a loop when number1 or number2 is not a valid float until the user enter a valid value, but I did not found a clean way to do it. I would gladly accept advices about this (even though I know this is not a question for this Stack Exchange, a good Pythonic way for doing this would be cool :) ).

Use more functions
You have a function calculate(), but if you see it does much more than just calculate, this makes your code look unreasonably clunky. But there is a very simply solution, use more functions. What if your main-loop could look like
while True:
    number1,number2,operand = take_input()
    result = calculate(number1,number2,operand)
    print(f"Answer : {numebr1} {operand} {number2} = {result}")
    if input("Do you want to play again? (y/n): ").lower() == 'n':
        break

This makes it easier to maintain your program.
continue when there is an error
try:
    number1 = float(input("Enter the first number : "))
except ValueError:
        print("Incorrect value")
        exit()

Ask yourself, why would a program terminate if the user enters invalid input? Give him a another chance xD
Maintain consistent indentation
try:
    number2 = float(input("Enter the second number : "))
except ValueError:
            print("Incorrect value")
            exit()

Try to maintain consistent indentation, since you have used 4 spaces earlier, there is no good reason to use 8 later, it might just confuse people who read the code later.
Code logic 1
Let's consider this sample input
Enter the first number : 1
Enter the operation symbol (+,-,/,*,%) : I like python
Enter the second number : 2
Incorrect symbol

Clearly, the symbol is wrong. Why did I have to enter the second number, just to find out I made a mistake while entering the symbol? It should've told me right away that my symbol was incorrect, so I could've corrected it.
Just move the if symbol not in operands statement so it sits right next to the input.
eval
Eval in Python
This would be the biggest improvement in your program, since it converts about 10-15 lines of code, into one.

The eval() function evaluates the specified expression, if the
expression is a legal Python statement, it will be executed.

That sounds familiar, aren't we basically evaluating simple expressions?
Using eval, your calculation part would look like
result = eval(f"{number1}{operand}{number2}")

Example, number1 = 5,number2 = 10, operand = '+'
This is what is basically happening
result = eval("5+10")

Final
Here is the code with the improvements
def take_input():
    err_msg = "Invalid input"
    operands = ['+','-','*','/','%']
    try:
        num1 = float(input("Enter number 1: "))
    except Exception:
        print(err_msg)
        return take_input()
    try:
        num2 = float(input("Enter number 2: "))
    except Exception:
        print(err_msg)
        return take_input()

    print("Operands: " + ', '.join(x for x in operands))
    try:
        operand = input("Enter operand: ")
    except Exception:
        print(err_msg)
        return take_input()

    if operand not in operands:
        print(err_msg)
        return take_input()

    return num1,num2,operand

def calculate(num1,num2,operand):
    return eval(f"{num1}{operand}{num2}")

def mainloop():
    while True:
        num1,num2,operand = take_input()
        result = calculate(num1,num2,operand)
        print(f"Answer: {result}")
        if input("Do you want to play again? (y/n): ").lower() == 'n':
            break

mainloop()


Implement a queue class in Python Problem:Implement a queue class in Python: It should support 3 APIs:

  queue.top(): prints current element at front of queue

  queue.pop(): takes out an element from front of queue

  queue.add(): adds a new element at end of queue

class Queue:

    def __init__(self):
        """initialise a Queue class"""
        self.items = []
        self.index = len(self.items) - 1

    def top(self):
        """returns the current element at front of queue"""
        if self.items:
            return self.items[0]
        else:
            raise Exception("Empty Queue")

    def pop(self):
        """takes out an element from front of queue"""
        if self.items:
            self.items.pop(0)
        else :
            raise Exception("Empty Queue")

    def add(self , item):
        """adds a new element at the end of queue"""
        self.items.append(item)

    def __str__(self):
        """returns the string representation"""
        return str(self.items)

    def __iter__(self):
        """iterates over the sequence"""
        return self

    def __next__(self):
        """returns the next value in sequence"""
        if self.index == 0 :
            raise StopIteration
        self.index -= 1
        return self.items[self.index]

queue_1 = Queue()
queue_1.add(12)
queue_1.add(11)
queue_1.add(55)
queue_1.add(66)
queue_1.add(56)
queue_1.add(43)
queue_1.add(33)
queue_1.add(88)
queue_1.add(56)
queue_1.add(34)
iter(queue_1)
for i in queue_1:
    print i
print queue_1
print queue_1.top()
queue_1.pop()
print queue_1.top()
queue_1.pop()
print queue_1.top()
queue_1.pop()
print queue_1.top()
queue_1.pop()
print queue_1.top()
queue_1.pop()
print queue_1.top()
queue_1.pop()
print queue_1.top()
queue_1.pop()
print queue_1.top()
queue_1.pop()
print queue_1.top()
queue_1.pop()
print queue_1.top()
queue_1.pop()
print queue_1.top()
queue_1.pop()

The code works as intended and executes the 3 APIs specified in the problem . But it gives ""TypeError: instance has no next() method"" when i try to use next method to iterate over the values in queue. What am i missing in defining the next method?

I would have just inherited from the built-in collections.deque:

from collections import deque

class Queue(deque):
    def add(self, value):
        self.append(value)

    def top(self):
        return self[0]

    def pop(self):
        return self.popleft()


jQuery - Drop Down QA Here's a fiddle: http://jsfiddle.net/YF8cg/

Focussing entirely on the javascript and on the structure of the HTML (.qapair .question and .answer), how could I improve this system?

JS:

$(document).ready(function() {
    $('.question').click(function() {
        $(this).siblings('.answer').slideToggle(400);       // Display the clicked answer

        $(this).parents('.QApair').siblings('.QApair').children('.answer').each(    // Hide all the others
            function() {
                if($(this).is(':visible')) {    // Detects if it's visible
                    $(this).slideToggle(400);   // If true then toggles it
                }
        });
    });
});

HTML:

First Question

Here would be lods of boring text, or some images, or some tables, or some fish, or some lets-pick-another-random-nouns, you get the picture. I just needed to fill space.

Second Question

I've run out of anything to write. So keyboard spas in 5... 4... 3... 2... 1... agdkfdakgjajgjregafgi iej fujagijrfgj rhgjahngufg ughuafng  uuagj u unhu up  u dnfajng hfgnajgnurngr gugnjfngja uugf n ndnfaung urgjangjnfg. That was interesting....

I'm fairly new to jQuery, so any help would be appreciated.

Thanks in advance.

HTML: The only change is that they are wrapped in a div id'ed container which will be used in event handling, explained below.

First Question

Second Question

Third Question

JS: Explained in the comments

//shorthand for $(document).ready(fn) is $(fn)
$(function () {

        //container will be used for delegated event handling
    var container = $('#container'),
        //since questions remain static, it's best we reference ahead of time
        questions = $('.question',container);

    //take advantage of delegation which assigns one handler to the parent
    //for all events in it's descendant. Here, we place a handler on container
    //for all question instead of putting a handler per question
    container.on('click', '.question', function (event) {
        var question = $(this);

        //we use 'next' which directly gets the next sibling instead of siblings(selector).
        //though internal implementations might be the same, but at least we avoid
        //writing that many selectors in our implementation
        question
            .next()
            .slideToggle(400);

        //using the referenced questions, we remove the currently clicked, pick
        //their answers that are currently visible and slide them up
        questions
            .not(question)
            .next(':visible')
            .slideUp(400);
    });
});

Here's running code. It's more of a readability and maintainability optimization rather than performance.


Code that lists and inserts data to database: Make methods more reusable in superclass I'm trying to make some methods more reusable and have now three classes that work together: AbstractDao (the superclass), MemberDao (extends AbstractDao) and ProjectTaskDao (extendsAbstractDao). The method insert is very similar in both subclasses with some minor differences:
insert method in MemberDao:
public void insert(Member member) throws SQLException {
try (Connection connection = dataSource.getConnection()){
    try(PreparedStatement statement = connection.prepareStatement(
            "INSERT INTO members (member_name, email) values (?, ?)",
            Statement.RETURN_GENERATED_KEYS
    )){
        statement.setString(1, member.getName());
        statement.setString(2, member.getEmail());
        statement.executeUpdate();

        try (ResultSet generatedKeys = statement.getGeneratedKeys()) {
            generatedKeys.next();
            member.setId(generatedKeys.getLong("id"));
        }
    }
}

}
insert method in ProjectTaskDao:
public void insert(ProjectTask task) throws SQLException {
try (Connection connection = dataSource.getConnection()){
    try(PreparedStatement statement = connection.prepareStatement(
            "INSERT INTO project_tasks (task_name) values (?)",
            Statement.RETURN_GENERATED_KEYS
    )){
        statement.setString(1, task.getName());
        statement.executeUpdate();

        try (ResultSet generatedKeys = statement.getGeneratedKeys()) {
            generatedKeys.next();
            task.setId(generatedKeys.getLong("id"));
        }
    }
}

}
Thesee methods do the same with some minor differences with the "statement.setSomething()". I`m not that advanced in Java yet so I'm kind of stuck on this. I would also like to pull these methods in as well:
list method in MemberDao:
public List<Member> list() throws SQLException {
List<Member>  members = new ArrayList<>();
try (Connection connection = dataSource.getConnection()) {
    try (PreparedStatement statement = connection.prepareStatement("SELECT * FROM members")) {
        try (ResultSet rs = statement.executeQuery()) {
            while (rs.next()) {
                Member member = new Member();
                member.setName(rs.getString("member_name"));
                members.add(mapRow(rs));
            }
        }
    }
}
return members;

}
list method in ProjectTaskDao:
public List<ProjectTask> list() throws SQLException {
List<ProjectTask>  tasks = new ArrayList<>();
try (Connection connection = dataSource.getConnection()) {
    try (PreparedStatement statement = connection.prepareStatement("SELECT * FROM project_tasks")) {
        try (ResultSet rs = statement.executeQuery()) {
            while (rs.next()) {
                //ProjectTask projectTask = new ProjectTask();
                //projectTask.setName(rs.getString(""));
                tasks.add(mapRow(rs));
            }
        }
    }
}
return tasks;

}
And here is the whole superclass as it stands currently:
public abstract class AbstractDao<T> {
protected final DataSource dataSource;

public AbstractDao(DataSource dataSource) {
    this.dataSource = dataSource;
}

protected T retrieve(Long id, String sql) throws SQLException {
    try (Connection connection = dataSource.getConnection()) {
        try (PreparedStatement statement = connection.prepareStatement(sql)) {
            statement.setLong(1, id);
            try (ResultSet rs = statement.executeQuery()) {
                if (rs.next()) {
                    return mapRow(rs);
                } else {
                    return null;
                }

            }
        }
    }
}

protected abstract T mapRow(ResultSet rs) throws SQLException;

}
Let me know if you need more information to help! Thanks :D

The first advise is to not over-architect the classes.

Keep alternatives in mind - JPA for instance eclipseLink. A bit of not reinventing the wheel as the database field has a long history of "clever" code.
This is an O/R mapping translating data base Relations into java Objects and vice versa. For a beginner I would first refrain from such magic, and continue as you did. (Your mapRow.)
The entity classes like Member in general are kept small without logic.
And for every entity class there is a repository class with data access (retrieve and such). They rely on a persistence manager singleton instance that takes care of the connection per transaction session (time doing some task).

A long ID and using generated keys is used often.
For the ID inheritance can be used. Too much inheritance can be painful however.

So keep id in its own base class, and do
public class Member extends IdEntity { ... }

public void insert(Member member) throws SQLException {
    try (Connection connection = dataSource.getConnection()) {
        try (PreparedStatement statement = connection.prepareStatement(
                "INSERT INTO members (member_name, email) values (?, ?)",
                Statement.RETURN_GENERATED_KEYS)) {
            statement.setString(1, member.getName());
            statement.setString(2, member.getEmail());
            statement.executeUpdate();

            SQLUtils.fillGeneratedKeys(member, statement);
        }
    }
}

public abstract class SQLUtils {

    public static void fillGeneratedKeys(IdEntity entity, PreparedStatement statement) throws SQLException {
        try (ResultSet generatedKeys = statement.getGeneratedKeys()) {
            generatedKeys.next();
            entity.setId(generatedKeys.getLong("id"));
        }
    }

3a. Similar is the initialisation of a time stamp field. If you disregard the database server's clock and use java's clock, think of a locale independent time.

Consider a "transaction" level; grouping of data base access.
Several database operations, say some INSERTs and UPDATEs best function under the same connection. Possible as transaction, an error raising a rollback and otherwise the data is committed at the end. This cleans up the repetition of try (Connection ....

In every case consider a connection pool or at least getting the connection by your own utility function.

For query methods do not use null results but Optional or exceptions. It not only prevents NullPointerExceptions, but allows chaining:

Several usage like:
A.retrieve().map(A::getChildren()).map(List::size).orElse(0);

protected Optional<T> retrieve(long id, String sql) throws SQLException {
    try (Connection connection = dataSource.getConnection()) {
        try (PreparedStatement statement = connection.prepareStatement(sql)) {
            statement.setLong(1, id);
            try (ResultSet rs = statement.executeQuery()) {
                if (rs.next()) {
                    return Optional.of(mapRow(rs)=;
                }
                return Optional.empty();
            }
        }
    }
}

Here Long can be long (It is a long id field too). For retrieve-by-id an exception would probably be better.
Here it could be a painful use of inheritance as the child class provides logic; but it is okay.
protected abstract String retrieveByIdSQL();

public final T retrieveById(long id) throws SQLException {
    String sql = retrieveByIdSQL();
    try (Connection connection = dataSource.getConnection()) {
        try (PreparedStatement statement = connection.prepareStatement(sql)) {
            statement.setLong(1, id);
            try (ResultSet rs = statement.executeQuery()) {
                if (rs.next()) {
                    return mapRow(rs);
                }
                throw new SQLException(...);
            }
        }
    }

Consider translating an SQLException to your own child of a RuntimeException, by rethrowing catched exceptions. Checked exceptions like SQLException must be catched,
which nowadays with Stream usage can be cumbersome. But you could postpone this for the fare future.


AI agent fails when aligned with its target I'm making an AI agent (blue entity) that pursues a target (green entity) while avoiding an obstacle (red entity).

The code works as it should most of the times, but will fail if all the entities are aligned in either the X or Y axis or any direction diagonally. I suspect this is due to dividing by zero.

I fixed the problem by simply checking if the axes were zero and then applying a slight offset.

if (vectorTargetX === 0) vectorTargetX += 0.1;
if (vectorTargetY === 0) vectorTargetY += 0.1;

However, this feels like a cheap fix and it seems like there might be some underlying problems in my code. Are there any better ways to go about this, maybe even more efficient?

You can see the how the AI agent fails by clicking the buttons in the example below.

const ctx = document.getElementById('canvas').getContext('2d');

let targetX = 150;
let targetY = 100;

let obstacleX = 200;
let obstacleY = 100;

let agentX = 300;
let agentY = 101;

function loop() {
  requestAnimationFrame(loop);

  // Calculate difference vectors
  let vectorTargetX = targetX - agentX;
  let vectorTargetY = targetY - agentY;

  let vectorObstacleX = obstacleX - agentX;
  let vectorObstacleY = obstacleY - agentY;

  // Calculate length between vectors
  const lengthTarget = Math.sqrt(vectorTargetX * vectorTargetX + vectorTargetY * vectorTargetY);
  const lengthObstacle = Math.sqrt(vectorObstacleX * vectorObstacleX + vectorObstacleY * vectorObstacleY);

  // Normalize vectors
  vectorTargetX = vectorTargetX / lengthTarget;
  vectorTargetY = vectorTargetY / lengthTarget;

  vectorObstacleX = vectorObstacleX / lengthObstacle;
  vectorObstacleY = vectorObstacleY / lengthObstacle;

  // Check if agent is within collision distance
  if (lengthObstacle < 60) {
    // Append displacement vector
    vectorTargetX -= vectorObstacleX * 0.7;
    vectorTargetY -= vectorObstacleY * 0.7;

    const displacedLength = Math.sqrt(vectorTargetX * vectorTargetX + vectorTargetY * vectorTargetY);

    // Move agent towards target while adding avoidance force
    agentX += vectorTargetX / displacedLength;
    agentY += vectorTargetY / displacedLength;
  } else {
    // Move agent towards target
    agentX += vectorTargetX;
    agentY += vectorTargetY;
  }

  if (lengthTarget < 12) {
    unaligned();
  }

  ctx.clearRect(0, 0, 500, 500);

  ctx.beginPath();
  ctx.fillStyle = '#00ff00';
  ctx.arc(targetX, targetY, 6, 0, Math.PI * 2, false);
  ctx.fill();

  ctx.beginPath();
  ctx.fillStyle = '#ff0000';
  ctx.arc(obstacleX, obstacleY, 6, 0, Math.PI * 2, false);
  ctx.fill();

  ctx.beginPath();
  ctx.fillStyle = '#0000ff';
  ctx.arc(agentX, agentY, 6, 0, Math.PI * 2, false);
  ctx.fill();

  ctx.font = '18px sans-serif';
  ctx.fillStyle = "#000000";
  ctx.fillText("X: " + agentX.toFixed(2), 10, 20);
  ctx.fillText("Y: " + agentY.toFixed(2), 10, 40);
}

function aligned() {
  agentX = 300;
  agentY = 100;
}

function unaligned() {
  agentX = 300;
  agentY = 101;
}

loop();
<canvas id="canvas"></canvas><br>
<button onclick="aligned()">Spawn agent aligned</button>
<button onclick="unaligned()">Spawn agent unaligned</button>

⚔ A.I. vs. Physics

To solve this, you need to think a bit about the physics of your simulation, because it creates a problem for your A.I.

The funny thing is, you solved it by just altering the vectors a bit, but you seem to not understand why, so let us change that!

 May the force be with you

To put it simply: when you put two vectors on the same axis perfectly aligned, all that will happen is that the stronger force (i.e. greater vector) wins. Applying that to your simulation: when you align the agents on the same axis, the force of attraction towards the green agent wins over the repulsion from the red agent, forcing it ever closer towards it. I assume we all know what happens when the distances become zero, and you inevitably divide by zero? If you don't know: you can't. Other languages would panic, and Javascript just gives back NaN. Doing arithmetic with NaN will always yield NaN so the program still crashes somewhere down the line.

So that is the problem. But why did your "hack" fix it, and how do you embrace the fix as a proper solution?

 Solution

This is where the A.I. comes in. Think about it as if you were the blue agent: if you were walking straight towards an obstacle you are not allowed to touch, what would you do? I presume you would pick a direction, and turn into that direction to avoid the obstacle. Your agent is not as smart as you, so it does nothing and keeps its direction. When you added a tiny offset to its target vector, you decided it should change direction, albeit in a clunky way.

I propose you add a "preference" to the blue agent, deciding which direction to turn when it is perfectly aligned on some axis. You can use your fix, but I would personally prefer a solution to be more deliberate:

const degreesToRadians = degrees => degrees * (Math.PI / 180);
const rotate = (x, y, radians) => [
  x * Math.cos(radians) - y * Math.sin(radians),
  x * Math.sin(radians) + y * Math.cos(radians)
];

const preference = degreesToRadians(15);
const inProximity = lengthObstacle < 60;
const aligned = vectorTargetX === 0 || vectorTargetY === 0;

if (inProximity && aligned) {
  [agentX, agentY] = rotate(agentX, agentY, preference);
}
```


A simple AI for Tic-Tac-Toe game This is an AI class that takes an array of the board as an argument, and plays the best move after evaluating the board and gets a win rate. I haven't used any java package to do this. How can I make a genetic tree and calculate all possibilities for a given situation

public class AI
{
    int turn,ID;
    AI(int pID)
    {
        ID=pID;
        turn=0;
    }

    int getWinRate(int t[])
    {
        int comb[][] = {{1,2,3},{4,5,6},{7,8,9},{1,4,7},{2,5,8},{3,6,9},{1,5,9},{3,5,7}} ;
        int AIwin,oppWin,empty,j,d;
        int WinRate = 0;
        for(int i=0;i<8;i++)  //Count AI win combinations
        {
            AIwin=oppWin=empty=0;
            for(j=0;j<3;j++)
            {
                d= comb[i][j]-1;
                 if(t[d]==2)
                 AIwin++;
                 if(t[d]==1)
                 oppWin++;
                 if(t[d]==0)
                 empty++;
            }
            //Changing win rate according to win conditions
            if(AIwin==3)
            WinRate+=100;
            if(AIwin==2 && empty==1)
            WinRate+=3;
            if(oppWin==2 && empty==1)
            WinRate-=10;
        }
        return WinRate;
    }
    void play(int t1[])
    {
        int t[]=t1.clone();
        int i,j,c=0;
        for(i=0;i<9;i++){
            if(t[i]==0)
            c++;
        }
        int[] moves=new int[c];
        c=0;
        for(i=0;i<9;i++)
        {
            if(t[i]==0)
           { moves[c++]=i;}
        }
        int rate,boardcopy[]=t.clone();boardcopy[moves[0]]=2;
        int bestMove=moves[0],maxRate=getWinRate(boardcopy);
        for(i=0;i<moves.length;i++)
        {
            boardcopy=t.clone();
            boardcopy[moves[i]]=2;
            rate=getWinRate(boardcopy);
            if(rate>maxRate)
            {
                maxRate=rate;
                bestMove=moves[i];
            }
        }
        System.out.println("The Mighty AI plays : "+(bestMove+1));
        turn++;
        t1[bestMove]=2;
    }
}

Here is some advice for your code.

General recommendations

1) For readability, I suggest that you change the way that you create your arrays; since the "C-style" array declaration is harder to read and less used in Java, in my opinion.

Before

int y[] = {};

After

int[] y = {};

2) In my opinion, I suggest that you wrap the logic of the condition / loop with braces, even if there's only one line of code; this provide consistency and prevent probable bugs if you want to add code to the condition and forget to add the braces.

Before

if (t[i] == 0)
  c++;

After

if (t[i] == 0) {
  c++;
}

getWinRate method

1)  Since the comb is always the same, I sugest that you extract it in a constant.

public class AI {
   //[...]
   private static final int[][] COMB = {{1, 2, 3}, {4, 5, 6}, {7, 8, 9}, {1, 4, 7}, {2, 5, 8}, {3, 6, 9}, {1, 5, 9}, {3, 5, 7}};
   //[...]
}

2) Instead of using a series of if on a value that can have only one of the values, I suggest that you use the if-else or a switch statement and extract the evaluation of t[d] in a variable.

Before

if (t[d] == 2)
   AIwin++;
if (t[d] == 1)
   oppWin++;
if (t[d] == 0)
   empty++;

if-else

int currentValue = t[d];
if (currentValue == 2)
   AIwin++;
else if (currentValue == 1)
   oppWin++;
else //Zero
   empty++;

or

switch

switch (t[d]) {
   case 2:
      AIwin++;
      break;
   case 1:
      oppWin++;
      break;
   default: //Zero
      empty++;
      break;
}

play method

1) In my opinion, instead of cloning an array, you can use java.util.Arrays#copyOf(int[], int).

   int[] t = Arrays.copyOf(t1, t1.length);

2) The j variable is unused.

3) I suggest that you use better names for your variable, it makes the code harder to read.

4) In my opinion, you can extract some of the logic into sub-methods.

a) Create a method to calculate the number of available moves.

private int calculateNumberOfEmptySpace(int[] game) {
     int nb = 0;

     for (int i = 0; i < 9; i++) {
         if (game[i] == 0) {
             nb++;
         }
     }

     return nb;
 }
 //[...]
void play(int[] t1) {
   int[] copyOfTheGame = Arrays.copyOf(t1, t1.length);
   int numberOfPossibleMoves = calculateNumberOfEmptySpace(t1);
}

5) The index in the last loop is not used, you can use a for-each

Before

for (i = 0; i < moves.length; i++) {
   boardcopy = t.clone();
   boardcopy[moves[i]] = 2;
   rate = getWinRate(boardcopy);
   if (rate > maxRate) {
     maxRate = rate;
     bestMove = moves[i];
  }
}

After

for (int move : moves) {
   boardcopy = t.clone();
   boardcopy[move] = 2;
   rate = getWinRate(boardcopy);
   if (rate > maxRate) {
     maxRate = rate;
     bestMove = move;
  }
}


HackerRank 'Bot Saves Princess 2' I submitted the following Python 3 code for this HackerRank challenge. Given an n × n grid with an m and a p in random cells of the grid, it prints the next step in a path that moves the m to the p. I'd like any feedback on this.

def nextMove(n,r,c,grid):
    #First, find the princess
    for i in range(len(grid)):
        for j in range(len(grid[0])):
            if grid[i][j] == "p":
                a = i
                b = j
    # i = row, j = column
    nm = ("UP" if (r-a) > 0 else "DOWN")
    if r-a == 0:
        nm = ("LEFT" if (c-b) > 0 else "RIGHT")
    return nm

n = int(input())
r,c = [int(i) for i in input().strip().split()]
grid = []
for i in range(0, n):
    grid.append(input())

print(nextMove(n,r,c,grid))

Parsing input

input().strip().split() can be reduced to input().split() as split without parameters has extra logic to remove empty strings in the result. It feels also more idiomatic to unpack the two integers using r, c = map(int, input().split())

Parsing princess position

First of, you could create the grid using a list-comprehension rather than append:

grid = [input() for _ in range(n)]

Note that I dropped 0 from the range parameter as it the default starting point.

But you don't even need to build the whole grid since you’re only interested in knowing the princess position. You could search for it while parsing the input:

def princess_position(grid_size):
    for row in range(grid_size):
        try:
            return row, input().index('p')
        except ValueError:
            # Princess not on this line
            pass
    # Princess not in grid
    return -1, -1

grid_size = int(input())
bot_row, bot_column = map(int, input().split())
princess_row, princess_column = princess_position(grid_size)
print(next_move(bot_row, bot_column, princess_row, princess_column))

Comparing positions

You don't need to put parenthesis around ternary operators. You can also simplify a - b > 0 to a > b; same for other comparisons. Last thing, you can perform less work by comparing a == r first and returning left or right before comparing for up and down:

def next_move(bot_row, bot_column, princess_row, princess_column):
    if bot_row == princess_row:
        return 'LEFT' if bot_column > princess_column else 'RIGHT'
    return 'UP' if bot_row < princess_row else 'DOWN'

Top-level code

It is better practice to wrap your top-level code under an if __name__ == '__main__' statement, as it allows you to import your file without executing the whole thing:

def next_move(bot_row, bot_column, princess_row, princess_column):
    if bot_row == princess_row:
        return 'LEFT' if bot_column > princess_column else 'RIGHT'
    return 'UP' if bot_row < princess_row else 'DOWN'

def princess_position(grid_size):
    for row in range(grid_size):
        try:
            column = input().index('p')
        except ValueError:
            # Princess not on this line
            pass
        else:
            return row, column
    # Princess not in grid
    return -1, -1

if __name__ == '__main__':
    grid_size = int(input())
    print(next_move(
        *map(int, input().split())
        *princess_position(grid_size))

I also changed the way to use unpacking to simplify variable managements.


Node, Link, Node, Link, Node: Linking Nodes into a LinkedList For practice in Python OOP (first time) and for some relaxation from trying to learn Serialization in Java, I decided to write a LinkedList in Python. Now, this class would be pretty useless, because in Python, instead of:

(Java)

int[] array = new int[size];
// initialization...
// Now add an object to the end
int[] newArray = Arrays.copyOf(array, size + 1);
newArray[size] = value; // Really annoying, is it not?

You can do:

array = [] # And other values
array.append(value)

When you want to add a value. But, since it's for practice, it's not for use.

class LinkedList:

    def __init__(self):
        self.first = None
        self.last = None
        self.size = 0

    def add(self, value):
        '''
        Adds the specified element to the end of the array.
        '''

        if not self.first: # first is None
            self.first = Node(None, None, value)
            self.last = self.first
        else:
            node = Node(self.last, None, value)
            self.last.after = node
            self.last = node

        self.size += 1

    def add_at(self, value, index):
        '''
        Adds the specified element at the specified position.

        If the given index is out of range of the list, the
        element is added to the end.
        '''

        if index >= self.size:
            self.add(value)

        if index == 0:
            new = Node(None, self.first, value)
            self.first.before = new
            self.first = new

        node = self.get_node(index)
        new = Node(node, node.after, value)
        node.after.before = node
        node.after = new

        self.size += 1

    def remove(self, index):
        '''
        Removes the specified element at the specified position.

        If the given index is out of range of the list, an
        IndexError is thrown.
        '''

        node = self.get_node(index)
        node.before.after = node.after
        node.after.before = node.before
        return node.value

    def get(self, index):
        '''
        Returns the specified element at the specified position.

        If the given index is out of range of the list, an
        IndexError is thrown.
        '''

        return self.get_node(index).value

    def get_node(self, index):
        '''
        Returns the specified node at the specified position.

        If the given index is out of range of the list, an
        IndexError is thrown.
        '''
        if index >= self.size:
            raise IndexError()

        node = None
        if index > self.size / 2:
            # From back
            node = self.last
            for i in range(index, self.size - 1):
                node = node.before
        else:
            # From front
            node = self.first
            for i in range(1, index):
                node = node.after

        return node

class Node:

    def __init__(self, before, after, value):
        self.before = before
        self.after = after
        self.value = value

Concerns:

I think I can separate Python from Java anc C++ now, so the ; is no longer there. But just in case, is there a stray ;?
Is the way this class is constructed Pythonic?
Are the docstrings good?

Methods & Naming

You're building a collection so, implicitly, you're building a contract with your user that your class will act like a collection, that you'll stick to a protocol for collections objects.

If you wan't to grasp a little more on what I’m trying to say, you can read this article.

So, even though I agree with @Dair that size would be “saffer” as _size (but hey, we’re all responsible adults around here), I strongly disagree with the size property. You should implement it as a __len__ method and use it like:

stuff = LinkedList()
length = len(stuff)

Some other methods can have more common names too:

add \$->\$ apppend;
add_at \$->\$ insert or __setitem__:

__setitem__ will allow you to do stuff[3] = 'some value' and then insert is just a call to self[index] = value.

You will need to change the signature to def __setitem__(self, key, value).
remove \$->\$ pop (or maybe __delitem__); remove is prefered to remove by value instead of by index.
get \$->\$ __getitem__: it will allow you to do the_value = stuff[3]; you should also remove get_node as there is little to no interest in exposing your internals to the user.

Methods that are missing

You could implement an __iter__ method that will allow you to do:

stuff = LinkedList()
# populate stuff
for elem in stuff: # will use __iter__, or fail back to __len__ + __getitem__ if not available
    do_something(elem)

It will also help you shorten __getitem__. You can use something along the lines of:

def __iter__(self):
    node = self.first
    while node is not None: # No self.size involved, yay
        yield node.value
        node = node.after

You can also implement the __reversed__ iterator using the same logic (and use it in __getitem__ too.

Using a parameter to __init__ that default to None would also be usefull to build a LinkedList out of any iterable:

def __init__(self, other=None):
    self._size = 0
    self.first = None
    self.last = None

    if other is not None:
        for element in other:
            self.append(element)

It is then easy to create stuff = LinkedList([1,2,8,12]).

And finally, you should consider adding a __str__ or __repr__ method so you can print(stuff). Use __iter__ to simplify it.


onSaveInstanceState() - ensuring a call to super instance When onSaveInstanceState() is customized it should always call the same method on the super instance first.

I usually write:

@Override
public void onSaveInstanceState(Bundle outState) {
    super.onSaveInstanceState(outState);
    saveState(outState);
}

private void saveState(Bundle outState) {
    outState.putParcelable("item1", item1);
    outState.putParcelable("item2", item2);
    // ...
    outState.putParcelable("item10", item10);
}

instead of putting all code in the onSaveInstanceState() method, in order not to accidentally skip call of super.onSaveInstanceState() (it is very hard to notice in a large project).

Is it a bad habit?

I think it is a very good practice.

As a side note, if you want to design a method like those, I'd suggest you to make the API method final and let the custom behaviors to be injected with the strategy design pattern (if you want to use composition, which is what I like the most) or with the implementation of an abstract method (if you want to use inheritance).

In that way it won't be possible to skip, even on purpose, the call to the superclass method.


C++ implementation of a Stack with dynamic C-style array I implemented a Stack in C++ using a dynamic C-style array. I tried sticking to the most important functions a Stack has to have to be usable. This is meant to only be for integers. I appreciate any suggestions on how to improve this. Here is my code:
Stack.h
#ifndef STACK_H
#define STACK_H
#include <cstdint>
#include <algorithm>

class Stack
{
public:
    Stack(int32_t u_size = 10) : m_current_ind{ -1 }, m_size{ 0 }, m_capacity{ u_size }{  // constructor
        m_stack_arr = new int32_t[u_size];
    };
    ~Stack() {                                                                     // destructor
        delete[] m_stack_arr;
    };
    Stack(const Stack& other) : m_current_ind{ -1 }, m_size{ 0 }, m_capacity{ 0 }{        // copy constructor
        for (int32_t i = 0; i < other.size(); i++)
        {
            push(other.m_stack_arr[i]);
        }
    }
    Stack(Stack&& other) : m_current_ind{ -1 }, m_size{ 0 }, m_capacity{ 0 }{             // move constructor
        m_stack_arr = std::exchange(other.m_stack_arr, nullptr);
    }
    Stack& operator=(const Stack& other) {                                         // copy assignment operator
        if (this != &other)
        {
            clear();
            for (int32_t i = 0; i < other.size(); i++)
            {
                push(other.m_stack_arr[i]);
            }
        }
        return *this;
    }
    Stack& operator=(Stack&& other) {                                              // move assignment operator
        if (this != &other)
        {
            m_stack_arr = std::exchange(other.m_stack_arr, nullptr);
            m_size = other.m_size;
            m_capacity = other.m_capacity;
            m_current_ind = other.m_current_ind;
        }
        return *this;
    }
    void clear(int32_t new_capacity = 10); // clear stack and initialise new empty array
    bool is_empty() const; // check if stack is empty
    int32_t pop(); // delete value on top and return it
    int32_t peek() const; // return value on top of the stack
    void push(int32_t u_value); // add value to stack
    int32_t size() const; // return size

private:
    void extend(); // double capacity

    int32_t m_current_ind;
    int32_t m_size;
    int32_t m_capacity;
    int32_t* m_stack_arr;
};

#endif

Stack.cpp
#include "Stack.h"
#include <stdexcept>

void Stack::clear(int32_t new_capacity)
{
    delete[] m_stack_arr;
    m_stack_arr = new int32_t[new_capacity];
    m_capacity = new_capacity;
    m_current_ind = -1;
    m_size = 0;
}

bool Stack::is_empty() const
{
    return !m_size;
}

int32_t Stack::pop()
{
    if (!is_empty())
    {
        m_size--;
        return m_stack_arr[m_current_ind--];
    }
    throw std::out_of_range("Stack is already empty.");
}

int32_t Stack::peek() const
{
    if (!is_empty())
    {
        return m_stack_arr[m_current_ind];
    }
    throw std::out_of_range("Stack is empty.");
}

void Stack::push(int32_t u_value)
{
    m_size++;
    m_current_ind++;
    if (m_size > m_capacity)
    {
        extend();
    }
    m_stack_arr[m_current_ind] = u_value;
}

int32_t Stack::size() const
{
    return m_size;
}

void Stack::extend()
{
    int32_t* temp = new int32_t[m_capacity * 2];
    m_capacity *= 2;
    std::copy(m_stack_arr, m_stack_arr + m_size, temp);
    delete[] m_stack_arr;
    m_stack_arr = temp;
}
```

Prefer std::vector to dynamic allocation
I understand this project was meant for practice, I have done a similar project, who wouldn't want to mess around with memory allocation, but doing so leads to errors when the programmer is not careful or chooses to ignore certain things. Using std::vector gives you the following for free

Copy constructor
Copy assignment
Move constructor
Move assignment

This enables you to focus more on the business logic of the problem.
Stack class does not cater for default constructed objects
Right now, your class assumes that users do not have default constructible objects. This is okay because you are holding Integral values, but you might change it in the future. The following
int *p = new int{}

allocates and construct an int in memory, this is okay for int because it has a default constructor. Your class might hold objects that are very expensive to construct and as such, users might need to just have a stack that can hold their default constructible objects and construct them when they are needed. This give rise to placement new, see more about placement new here
CODE REVIEW

In Stack constructor, you declared the following

Stack(int32_t u_size = 10)

This is okay, you want a default stack to have 10 elements, A better approach is to declare a default stack to hold 0 elements, this is the behavior of std::vector. The stack can grow when necessary. If the user wants to explicitly request for space, declare a reserve method that allocates space. You might also consider a shrink_to_fit method, well am going overboard for a simple stack class.

You forgot to allocate memory in your copy constructor. This is dangerous, you are reading into memory that was never allocated. The result is undefined, it might work today and crash tomorrow. A correct approach is this

 Stack(const Stack& other) : m_current_ind{ -1 }, m_size{ 0 }, m_capacity{ other.m_capacity } {        // copy constructor
    m_stack_arr = new int32_t[other.m_size];
    for (int32_t i = 0; i < other.size(); i++)
    {
        push(other.m_stack_arr[i]);
    }
}

Now this is better, we explicitly created m_stack_arr.

Move constructor does not work. On testing your move constructor, I resulted to a segmentation fault. m_size was not given the appropriate size, the same case with m_capacity. using a swap method, your move constructor can be defined as follow

void swap(Stack& lhs, Stack& rhs)
{
    using std::swap;
    swap(lhs.m_current_ind, rhs.m_current_ind);
    swap(lhs.m_size, rhs.m_size);
    swap(lhs.m_capacity, rhs.m_capacity);
    swap(lhs.m_stack_arr, rhs.m_stack_arr);
}

This method just swap the internal representation of objects. Using this method, move constructor and move assignment becomes easy
Stack(Stack&& other) noexcept {             // move constructor
    swap(*this, other);
    other.m_stack_arr = nullptr;
}

We set the value of other.m_stack_arr to a nullptr to make it a cheap resource for the compiler to destroy.
Move assigment is also similar, a self move is not bad and no need for a check here.
Stack& operator=(Stack&& other) noexcept {
    swap(*this, other);
    other.m_current_ind = -1;
    other.m_size = 0;
    other.m_stack_arr = nullptr;
    return *this;
}

Both methods are marked noexcept because we are guaranteed that move assignment and move constructor would not throw an exception, this gives room for compiler to optimize some certain things.

In clear method, you delete m_stack_arr before allocating a new one, what if new throws an exception, you have lost m_stack_arr.

void Stack::clear(int32_t new_capacity)
{
    int32_t* temp_arr; = new int32_t[new_capacity];
    delete[] m_stack_arr;
    std::copy(m_stack_arr, m_stack_arr + m_size, temp_arr);

    //.. Your code goes in here
}

Here we created our memory and store it temp_arr, if that doesn't throw, we delete our old array and copy the new one.
A more optimized version would be to use memcpy to copy the bytes from memory, but that is a different topic entirely.


Functionaly Finding Prime Factors with Multiplicity I am quite new to Haskell so any suggestions are welcome, also I noticed that it works quite slow even for something like

prime_factors_mult (2*2*2*2*3*3*3*5*5*7*7*7*7*13*13*13*17*19)

Also I may not know of coding conventions/formatting styles and better ways to do things,so you may also help me with that:

prime_factors_mult :: Int -> [(Int, Int)]
prime_factors_mult n =
    reverse . fst $
        foldl
            (\acc@(factors, val) x
                -> if val `mod` x == 0 then
                    let (k, v) = reduce val x in ((x, k) : factors, val `div` v)
                else
                    acc
            )
        ([], n)
        [2 .. floor . sqrt . fromIntegral $ n]

reduce :: Int -> Int -> (Int, Int)
reduce val x =
    if val `mod` x == 0 then
        let (k, v) = reduce (val `div` x) x in (1 + k, v * x)
    else
        (0, 1)

First of all, we can make your prime_factors_mult easier to read if we use where:

prime_factors_mult :: Int -> [(Int, Int)]
prime_factors_mult n = reverse . fst . foldl go ([], n) $ candidates
  where
    candidates = [2 .. floor . sqrt . fromIntegral $ n]
    go acc@(factors, val) x
      | val `mod` x == 0 = let (k, v) = reduce val x in ((x, k) : factors, val `div` v)
      | otherwise        = acc

By the way, Haskell programs use camelCase, so primFactorsMult would be appropriate. However, does this function really capture your intend? Let us write it as imperative variant for a second:

# pseudo-python
def prime_factors_mult(n):
    result = []

    for i in range(2, sqrt(n)):
        count = 0
        while n % i == 0:
            n = n / i
            count = count + 1

        if count > 0:
            result = result.append({i, count})

    return result

This is a lot easier to read. We should strive to have an algorithm in Haskell that's just as easy to read, right? By the way, both the pseudo-code as well as your original code contained an error: what happens on 2*7? \$\sqrt{14} < 7\$, therefore we never check the second prime. We should check that too:

# pseudo-python
def prime_factors_mult(n):
    result = []

    for i in range(2, sqrt(n)):
        count = 0
        while n % i == 0:
            n = n / i
            count = count + 1

        if count > 0:
            result = result.append({i, count})

    # n is prime at this point:
    if n > 1:
        result = result.append({n, 1})

    return result

Allright. Now let's try to get a variant of Haskell that's just as readable. We do that by explicit recursion and then later figure out how we can get rid of that:

isDivisibleBy n k = n `rem` k == 0

primeFactorsMult :: Int -> [(Int, Int)]
primeFactorsMult = go 2
  where
    go _ 1 = []
    go k n
       | n `isDivisibleBy ` k = (k, m)   : go (k + 1) (n `div` d)
       | n < k * k            = [(n, 1)]
       | otherwise            = go (k + 1) n
      where
        (m, d) = reduce n k

Allright, that seems to work. However, do we actually need the isDivisibleBy check? No. reduce does that for us. So let's get rid of that:

primeFactorsMult :: Int -> [(Int, Int)]
primeFactorsMult = filter ((0 /=) . snd) . go 2
  where
    go _ 1 = []
    go k n
       | n <= k    = [(n, 1)]
       | otherwise = (k, m)   : go (k + 1) (n `div` d)
      where
        (m, n') = reduce n k

Great. Now we're getting somewhere. We have now two paths we can choose to continue. Either we take our new approach, which is essentially mapAccumL, or we inline reduce. Let us follow both.

Mapping with a state

In the code above, we essentially mapped over the list of factors [2..n] with a state: the current value of n, or rather n `div` d. It turns out that there is already a function for that kind of operation, namely mapAccumL. We can incorporate into our function:

primeFactorsMult :: Int -> [(Int, Int)]
primeFactorsMult n = filter ((0 /=) . snd) . snd . mapAccumL go n $ 2 : [3,5..n]
  where
    go n k = (n `div` d, (k, m))
      where
        (m, n') = reduce n k

As you can see, we got rid of the recursive calls in go and the base case, since that's now handled by mapAccumL. We also swapped the arguments, since mapAccumL gives the state first. Also, since primes are odd (except for 2), we've optimized the list slightly.

Unfortunately, we still need to get rid of those with 0 multiplicity. However, what if we didn't return a pair?

Return factors, one by one

This time, we inline the functionality of reduce:

primeFactors :: Int -> [Int]
primeFactors n = go 2
  where
    go _ 1 = []
    go k n = case n `quotRem` k of
               (q, 0) -> k : go k q
               _      ->     go (k + 1) n

This will now return a list like [2,2,2,2,2,2,2,2,2,5,5,5,7,7,9,9,10]. By the way, this quotRem shows a possible optimization of reduce:

reduce :: Int -> Int -> (Int, Int)
reduce val x =
    if m == 0 then
        let (k, v) = reduce d x in (1 + k, v * x)
    else
        (0, 1)
 where
    (d, m) = val `divMod` x

But when all your numbers are positive, you want to use quotRem since it's usually faster. Note that many modern processors do mod and div at the same time, so using the appropriate function when you ened both results can be a boon.

Back to our prime factors. Now we have the factors. But how can we get the multiplicity? We use group:

primeFactorsMult :: Int -> [(Int, Int)]
primeFactorsMult = map (\xs -> (head xs, length xs)) . group . primeFactors

Since group returns only non-empty lists, we can safely apply head.

Remarks on style

I strongly suggest you to move large lambdas into their own bindings. That made the first variant of prime_factors_mult more readable and is the usual style, for example in GHC's code.

Also, if you use both the results of div and mod use divMod, or—if you use positive numbers only or don't want the specific behaviour behind divMod—quotRem.

Don't be afraid to give something in your code a name. That makes it easier to grasp and to change later. Also, you might notice that a function is reusable.

And, by the way, primes are a "hard" topic, as in: it's not easy to get proper bounds of the numbers you have to check, since you would need to know whether n is already a prime, or whether n / 2 is a prime, or n / 3 and so on.

Exercises

In go you can use (k + 2) if you handle 2 separately. How can you do that?
In most go functions we can stop much, much sooner if n is a prime if we know the original n. Why? (Hint: what is the largest prime factor that can be in a number? And what is the second largest prime factor?)


Batch Processing : Design check with SRP and OCP prinicple I have to create a batch process which reads data from DB , process it and saves it in db . The batch data is fetched organization wise ( each organization has set of data to be processed) at once.We need to process only specific set of data i.e organization related and save it into db. Hence i have designed below classes . Does this design breaks SRP and OCP?

Should the process method in DepositProcess have the code to iterate the batch data ?

Batch Processor Interface

package com.ibank.batch;

public interface BatchProcessor {

    public void process();

}

Deposit Processort class

package com.ibank.batch;

import java.util.HashMap;
import java.util.Iterator;
import java.util.List;
import java.util.Set;

import com.ibank.batch.svc.DepositBatchProcessorSvc;

public class DepositProcessor implements BatchProcessor {

    @Override
    public void process() {
        DepositBatchProcessorSvc svc = new DepositBatchProcessorSvc();
        HashMap<String, List<String>> batchData = svc.fetchBatchData();
        Set<String> key = batchData.keySet();
        Iterator<String> keyItr = key.iterator();
        while (keyItr.hasNext()) {
            String organizationId = (String) keyItr.next();
            List<String> claimsList = batchData.get(organizationId);
            svc.saveBatchData(claimsList);
        }
    }

}

Service interface and implementation class

package com.ibank.batch.svc;

import java.util.List;

public interface BatchProcessorSvc {

    public Object fetchBatchData();

    public <T>  void  saveBatchData(List<T> data);

}

package com.ibank.batch.svc;

import java.util.HashMap;
import java.util.List;

public class DepositBatchProcessorSvc implements BatchProcessorSvc {

    @Override
    public HashMap<String, List<String>> fetchBatchData() {
        return null;

    }

    @Override
    public <String> void saveBatchData(List<String> claimList) {

    }

}

DAO interface and implementation

package com.ibank.batch.dao;

import java.util.List;

public interface BatchProcessorDao {

    public Object fetchBatchData();

    public <T>  void  saveBatchData(List<T> data);

}

package com.ibank.batch.dao;

import java.util.HashMap;
import java.util.List;

public class DepositBatchProcessorDao implements BatchProcessorDao {

    @Override
    public HashMap<String, List<String>> fetchBatchData() {
        return null;
    }

    @Override
    public <String> void saveBatchData(List<String> claimList) {

    }

}

Quick question, have you tried testing your code? Trying doing so, and you will find the leaks in your code.

DepositProcessor has a high dependency on DepositBatchProcessorSvc, why not to inject it and make the code flexible (easier to test and change).

public class DepositProcessor{
   private final DepositBatchProcessorSvc service;
   DepositProcessor(DepositBatchProcessorSvc service){
    this.service = service;
  }
}

BatchProcessorSvc could be totally generified,

/*K type of the object's promary key, and T is the object type*/
public interface BatchProcessorDao<K,T> {
  @Override
  Map<K, List<T>> fetchBatchData();
}

Always try to return interfaces and not implementations, ex:

HashMap<String, List<String>> fetchBatchData()

could be refactored to

Map<String, List<String>> fetchBatchData()

Save should never be fire and forget, I know a lot of db clients do it, but it is really bad, I need to know if things were saved successfully, wishful programming is bad, you could return the objects after saving them

List<T> saveBatchData(List<T> data);


Query of top players of the month In a web game there is a ranking of the top points games played of the month:

// QUERY
SELECT * FROM game
WHERE date >= date_sub(CURDATE(),INTERVAL 1 MONTH)
ORDER BY points DESC limit 0, 20

But, in that case one player can be the top 1,2,3... so I have changed the query to add a distinct, and then only appears one game for player (the game with max points):

// Query that filters player names
SELECT g.*
FROM game g
INNER JOIN ( SELECT playerName, MAX(points) AS MaxPoints
             FROM game
             GROUP BY playerName
           ) groupGame
        ON g.playerName = groupGame.playerName AND g.points = groupGame.MaxPoints
WHERE date >= date_sub(CURDATE(),INTERVAL 1 MONTH)
ORDER BY points DESC limit 0, 20

The query works OK but the second query is much slower than the first. Is there any way to optimize it?

You will have to test performance, but a GROUP BY should help:

SELECT playerName, MAX(points) AS points /*[, other columns]*/
FROM game
WHERE date >= date_sub(CURDATE(),INTERVAL 1 MONTH)
GROUP BY playerName /*[, other columns]*/
ORDER BY points DESC
LIMIT 0, 20