Haskell 'n' layered ANN forward pass I'm trying to write a simple 'n' layered ANN in haskell for supervised learning, it will eventually have back prop and you'll be able to use it in a step by step fashion through a GUI which will graph the error function. This is for learning (so I'm not looking for suggestions on libraries that already solve this problem). In this review I'm mainly looking for feedback on how I've arranged my code and if there are better ways to represent a neural network in Haskell.

The approach I'm trying to take is to separate the forward pass from the backward pass, so that the training can be controlled by the consumer (rather than having a network that is just a one shot IO which does forward -> back recursively until error < x). The reason for this is that I can then separate the rendering of the networks state at any given pass (i.e. after each forward pass I can easily just render the current loss of the whole network by applying the cost function across the output vector).

Here is the current code for a forward pass of the network. It takes a list of double (which is the input vector) and a list of matrix of double (where each element in the list is a weight matrix representing the weights of each connection on a given layer in the network). The activation function then creates an output vector by recursively applying a forwardPass function through each layer until the final layer 'n' has been evaluated.

Here is the code for that:

module Training (randomWeights, activate, cost) where

  import Data.Matrix
  import System.Random

  activate :: [Double] -> [Matrix Double] -> Matrix Double
  activate i weights = forwardPass inputs weights where
    inputSize = length i
    inputs = fromList 1 inputSize i
    forwardPass inputs weights
      | length weights == 1 = squashedOutputs
      | otherwise = forwardPass (squashedOutputs) (tail weights)
      where
        squashedOutputs = mapPos (\(row, col) a -> leakyRelu a) layerOutputs where
          layerOutputs = multStd2 inputs (head weights)
          leakyRelu a
            | a > 0.0 = a
            | otherwise = 0.01 * a

  randomWeights :: (RandomGen g) => g -> Int -> Int -> Matrix Double
  randomWeights generator inputSize outputSize = weights where
    weights = matrix inputSize outputSize (\(col, row) -> (take 10000 $ randomRs (-1.0, 1.0) generator)!!(col * row))

The randomWeights function is consumed in the main function of my program and is used to generate the list of matrix of double to be passed to the forwardPass (i.e. each layers weights).

The main function looks like this:

  main :: IO()
  main = do
    generator <- newStdGen

    let expected = 0.0
    let inputs = [0, 1]

    let inputWeights = randomWeights generator (length inputs) 3
    let hiddenWeights = randomWeights generator 3 1
    let outputWeights = randomWeights generator 1 1

    let outputs = activate inputs [inputWeights, hiddenWeights, outputWeights]

    print inputs
    print outputs

So it a bit like a unit test (eventually I will wrap the activate/backprop loop into a structure that a user can control by a 'next' button on a GUI, but for now I simply want a solid forwardPass foundation to build off.

Does all of this look like reasonable haskell, or are there some obvious improvements I can make?

\(col, row) -> (take 10000 $ randomRs (-1.0, 1.0) generator)!!(col * row)

Oh man you got me going "no no no no no no" like I long haven't :D. take 10000 does nothing here. col * row is going to come out to the same when you switch row and col, perhaps you want col + inputSize * row? randomRs is going to be recalculated for each (col,row) pair - fromList fixes that. Calling the line's result weights is little more than a comment. MonadRandom can avert the generator passery, and also stop you generating the same random values for each call to randomWeights.

activate :: [Double] -> [Matrix Double] -> Matrix Double
activate i = foldl squash (fromLists [i]) where
  squash inputs weights = fmap leakyRelu $ multStd2 inputs weights
  leakyRelu a
    | a > 0.0 = a
    | otherwise = 0.01 * a

randomWeights :: Int -> Int -> IO (Matrix Double)
randomWeights rows cols = fromList rows cols <$> getRandomRs (-1.0, 1.0)

main :: IO ()
main = do
  let inputs = [0, 1]

  inputWeights <- randomWeights (length inputs) 3
  hiddenWeights <- randomWeights 3 1
  outputWeights <- randomWeights 1 1

  let outputs = activate inputs [inputWeights, hiddenWeights, outputWeights]

  print inputs
  print outputs


In Memory Database in Unit tests, isolate the tests I have stumbled across these unit tests in a code review that are using in memory db:

private DatabaseContext _context;
private Fixture _fixture;

[SetUp]
public void Setup()
{

    _fixture = new Fixture();
    _fixture.Customize(new AutoNSubstituteCustomization());

    var options = new DbContextOptionsBuilder<DatabaseContext>()
    .UseInMemoryDatabase(databaseName: "testdb")
    .Options;
    _context = new DatabaseContext(options);
}

[TearDown]
public void CleanUp()
{
    var context = _context;
    if (context == null || context.Database.ProviderName != "Microsoft.EntityFrameworkCore.InMemory")
    {
        return;
    }

    context.Database.EnsureDeleted();
    _context = null;
}

#region EmptyDB
[Test]
public void Test1()
{
    // Setup
    var logger = _fixture.Freeze<ILogger<UserRepository>>();

    var userRepo = new UserRepository(_context, logger);

    var userViews = new List<UserView>();
    userViews.AddRange(_fixture.CreateMany<UserView>(10));

    // ACT
    userRepo.UpdateUsers(userViews, CancellationToken.None).GetAwaiter().GetResult();

    // ASSERT
    Assert.AreEqual(10, _context.Users.CountAsync().GetAwaiter().GetResult());
}

[Test]
public void Test2()
{
    // Setup
    var logger = _fixture.Freeze<ILogger<UserRepository>>();

    var userRepo = new UserRepository(_context, logger);

    var identityViews = new List<IdentityView>();
    _fixture.Register<IEnumerable<UserView>>(() =>
    {
        return new UserView[] { new UserView("fish") };
    });
    userViews.AddRange(_fixture.CreateMany<UserView>(10));

    // ACT
    userRep.UpdateUsers(userViews, CancellationToken.None).GetAwaiter().GetResult();

    // ASSERT
    Assert.AreEqual(10, _context.Users.CountAsync().GetAwaiter().GetResult());
}

As you can see, the tests are using the same in memory db, which I really don't like. I also don't like the new UserRepository(_context, logger). Is it a bad practice to use the new-keyword like this?

I would prefer something like this instead:

[Test]
public void Test1()
{
    // Setup
    var provider = RegisterServices();
    var logger = _fixture.Freeze<ILogger<UserRepository>>();
    var userRepo = provider.GetRequiredService<IUserRepository>();

    var userViews = new List<UserView>();
    userViews.AddRange(_fixture.CreateMany<UserView>(10));

    // ACT
    userRepo.UpdateUsers(userViews, CancellationToken.None).GetAwaiter().GetResult();

    // ASSERT
    Assert.AreEqual(10, _context.Users.CountAsync().GetAwaiter().GetResult());
}

private ServiceProvider RegisterServices([CallerMemberName] string memberName = "")
{
    var services = new ServiceCollection();

    services.AddDbContext<IDatabaseContext, DatabaseContext>(options =>
        options.UseInMemoryDatabase(memberName));

    services.AddPersistence("https://localhost");

    return services.BuildServiceProvider();
}

As you can see, I have added a RegisterService method that takes the calling test as a parameter, and then uses this to create the inmemorydb. I really like this because you are isolating your tests more this way. I also think it's cleaner to read.

How would you guys do in this case? Is the first approach the way to go, or is my approach the more "right" way to do it? Or is it another better and more best practice way to do it?

I just want to know your opinions about this and about the two approaches above.

Generally speaking whenever we are about to write unit tests we should follow the F.I.R.S.T. principles. It is an acronym, which stand for:

Fast: The execution time should be measured in milliseconds. If it takes a second or two then it can be considered as slow and should be revised.
Isolated: Each test is self-contained and does not rely on any other data. They can be run in any order.
Repeatable: The test runs must be repeated over and over again. In each and every run they should either pass every time or always fail.
Self-validating: There is no need for human interpretation whether or not the test succeeded or failed. The test result should be self-explanatory.
Timely: The code and related tests should be written in almost the same time. A new code without relevant unit tests should not be deployed.

Let's examine these ideas for your proposals:

Single database and cleanup

Fast: If for whatever reason a previous cleanup phase missed / failed then your database will have some trash data in it, which might impact the performance of your database operations. Executing the cleanup during setup and teardown might solve the problem, but it will definitely have performance impact.
Isolated: They are sharing the same database, so they are not truly isolated. It might be the case that they can't run in parallel, because ordering might matter.
Repeatable: Because they are using the same database, that's why order might affect the result of your assertions. In case of MSTest you can define ordering but if you need to use them that means your tests are not really isolated.
Self-validating: Because there is a chance for race condition your test results are non-deterministic, which means human-intervention is needed to interpret several resultsets, reproduce the issues (if it is even possible) and fix them.
Timely: It is irrelevant in our discussion

Separate database for each test case

Fast: Creating new in-memory database for each test should not impose performance penalty onto tests if there not too many tables and constraints.
Isolated: Separate databases are used for each test, means no shared resource is being used, which helps isolation.
Repeatable: Because each and every time you run your test against a brand new database, there won't be any trash data, which could cause race condition.
Self-validating: By being deterministic, no human intervention is needed to understand the test results.
Timely: It is irrelevant in our discussion

If you don't want to examine the test data manually, then you don't really need use the test name in the database name. You can use any random value:

int jitter = idGenerator.Next();
var condigBuilder = DbContextOptionsBuilder<TestContext>()
    .UseInMemoryDatabase(databaseName: $"TestDb{jitter}")
    .Options;

or

Guid jitter = Guid.NewGuid();
var condigBuilder = DbContextOptionsBuilder<TestContext>()
    .UseInMemoryDatabase(databaseName: $"TestDb{jitter}")
    .Options;


Pseudo Game of Cups in Python DESCRIPTION:
  [Inspired by Chandler's GameOfCups with Joey in "Friends"].
     Program gets 5-digit zipcode from user.  (Assume user won't make a
         mistake, and will enter exactly 5 digits).
     Program awards points based on a series of rules, and reports the total
         points earned at the end.
     The 8 rules are embedded as comments in the code.
     For each rule, besides adding points (or not) to the total, rule displays
         "Rule _ got _ points, so total is now _"
         (It prints this even if rule added 0 points to total).

"""
RULES
    +5 when first and last digit match
    +6 when second digit is twice the first AND third digit is greater than second or fourth digit
    +7 if any 7 is in the zipcode
    +8 when there's no "13" in MIDDLE the zipcode
    +9 when all three middle digits match
    +10 when third and fourth digits match
    +11 when zipcode is palindrome (12121 == 12121, while 12345 != 54321)
"""

Here is my solution to the challenge above:

zipcode = input("Enter your zipcode: ")
total_points = 0

#Rule 1

points = 5 if zipcode[0] == zipcode[-1] else 0
total_points += points
print(f"Rule 1 got {points} points, so total is now {total_points}")

#Rule 2

points = 6 if (int(zipcode[1]) * 2) > int(zipcode[0]) and (int(zipcode[2]) > int(zipcode[1]) or int(zipcode[2]) > int(zipcode[3])) else 0
total_points += points
print(f"Rule 2 got {points} points, so total is now {total_points}")

#Rule 3

points = 7 if "7" in zipcode else 0
total_points += points
print(f"Rule 3 got {points} points, so total is now {total_points}")

#Rule 4

points = 8 if "13" not in zipcode[1:-1] else 0
total_points += points
print(f"Rule 4 got {points} points, so total is now {total_points}")

#Rule 5

points = 9 if zipcode[1] == zipcode[2] == zipcode[3] else 0
total_points += points
print(f"Rule 5 got {points} points, so total is now {total_points}")

#Rule 6

points = 10 if zipcode[2] == zipcode[3] else 0
total_points += points
print(f"Rule 6 got {points} points, so total is now {total_points}")

#Rule 7

points = 11 if zipcode == reversed(zipcode) else 0
total_points += points
print(f"Rule 7 got {points} points, so total is now {total_points}")

print(f"{zipcode} got {total_points} points!")

I feel like there is a much better way to do this. The print statements are repetitious, and reassigning points each time I check the zip code feels weird. Any suggestions are helpful and appreciated.

Your code can be simplified using a simple loop, eliminating most of the duplicated code:

def game_of_cups(zipcode, rules):

    total_points = 0

    for num, rule in enumerate(rules, 1):
        rule_passes = rule(zipcode)
        points = num + 4 if rule_passes else 0
        total_points += points
        print(f"Rule {num} got {points} points, so total is now {total_points}")

    print(f"{zipcode} got {total_points} points!")

You just need the appropriate rule functions, like:

def rule1(zipcode):
    return zipcode[0] == zipcode[-1]

def rule2(zipcode):
    a, b, c, d, e = map(int, zipcode)
    return b * 2 > a and c > min(b, d)

... etc ...

And then a list of rules to pass to the game:

rules = [ rule1, rule2, rule3, rule4, rule5, rule6, rule7 ]

Feel free to name the functions more appropriately; they don’t need to be named rule#.

Are you missing a rule?  You said there were 8 rules.

Your implementation of rule#2 doesn’t match the comment description of rule #2.  I think it should be b == a * 2, not b * 2 > a.


Part one of blackjack game I had to make this blackjack game for school, but this is only the first part. We only had to use a random value between 2 and 11 to generate the cards, as we will have a library of cards later, and the dealer will keep pulling cards until their total value is at or equal to 17. The player needs to decide whether they want to hit or stay, and will always bet $25. I want to clean up the code to make it clearer, as it seems a bit messy.

import random

def main():

  restart = True

  bank_balance = 1000
  player_name = input("Please enter your name: ")
  while (bank_balance >= 25 and restart == True):

    print (f"Welcome {player_name}, your bank balance is ${bank_balance} ")
    print (f"Betting $25")
    bank_balance-=25

    deal_hand = play_hand(player_name)

    play=input("Type 'y' to play again, or type any key to quit" + '\n')

    if (play == 'y'):
      restart=True
      print('\n')
    elif (play != 'y'):
      print('Thanks for playing ')
      restart=False

def play_hand(name):

  player= []
  dealer= []

  play_again = True

  dealer.append(random.randint(1, 11))

  player.extend([random.randint(1, 11), random.randint(1, 11)])

  print ('The dealer received card of value', *dealer)
  print(name, 'received cards of value', player[0], 'and', player[-1])
  print(f'Dealer total is {sum(dealer)}')
  print(f"{name}'s total is {sum(player)}", '\n')

  stay = False
  bust = False

  while (sum(player) <= 21 and stay == False and play_again == True):
    hors= input(f"Type 'h' to hit and 's' to stay ")
    if (hors == 'h'):
      new_card= random.randint(1, 11)
      player.append(new_card)
      print(f'{name} pulled a {new_card}')

      print(f'Dealer total is {sum(dealer)}')
      print(f"{name}'s cards are", *player)
      print(f"{name}'s total is {sum(player)}", '\n')

    elif (hors == 's'):
      stay=True
      print('stay')

  if (sum(player) > 21 ):
    bust = True
    print('You busted!')

  while (stay == True and sum(dealer) < 17 and bust == False and play_again == True):
    dealer.append(random.randint(1, 11))

    print('The dealers total is', sum(dealer), '\n')

  if (sum(dealer) <= 21 and sum(dealer) > sum(player)):
    print("The dealer wins!")
  elif (sum(player) <= 21 and sum(player) > sum(dealer)):
    print("You win!")
  if (sum(dealer) > 21):
    print ('You win! The dealer busted!')
  if (sum(dealer) == sum(player)):
    print('Its a Tie! ')

main()

My main tip is to use break and continue to control the flow of your while loops rather than having a bunch of extra bools in the loop condition.  For example, instead of:

  restart = True
  while (bank_balance >= 25 and restart == True):

    print (f"Welcome {player_name}, your bank balance is ${bank_balance} ")
    print (f"Betting $25")
    bank_balance-=25

    deal_hand = play_hand(player_name)

    play=input("Type 'y' to play again, or type any key to quit" + '\n')

    if (play == 'y'):
      restart=True
      print('\n')
    elif (play != 'y'):
      print('Thanks for playing ')
      restart=False

get rid of the restart flag and do:

  while bank_balance >= 25:

    print (f"Welcome {player_name}, your bank balance is ${bank_balance} ")
    print (f"Betting $25")
    bank_balance-=25

    deal_hand = play_hand(player_name)

    play=input("Type 'y' to play again, or type any key to quit" + '\n')

    if (play == 'y'):
      print('\n')
    elif (play != 'y'):
      print('Thanks for playing ')
      break

(Also, for a true/false value instead of saying flag == True or flag == False just say flag or not flag.)

You can apply this same principle to the loops in play_hand where you're checking several bools in each while loop:

  player_bust = False
  dealer_bust = False

  while True:
    hors= input(f"Type 'h' to hit and 's' to stay ")
    if (hors == 'h'):
      new_card= random.randint(1, 11)
      player.append(new_card)
      print(f'{name} pulled a {new_card}')
    elif (hors == 's'):
      print('stay')
      break
    print(f'Dealer total is {sum(dealer)}')
    print(f"{name}'s cards are", *player)
    print(f"{name}'s total is {sum(player)}", '\n')
    if sum(player) > 21:
      player_bust = True
      break

  if not player_bust:
    while sum(dealer) > 17:
      dealer.append(random.randint(1, 11))
      print('The dealers total is', sum(dealer), '\n')
  if sum(dealer) > 21:
    dealer_bust = True

  if player_bust:
    print("You busted!")
  elif dealer_bust:
    print("You win!  The dealer busted!"
  elif sum(dealer) > sum(player):
    print("The dealer wins!")
  elif sum(player) < sum(dealer):
    print("You win!")
  else:
    print('Its a Tie! ')

The idea is to minimize the number of conditions that each check depends on; it gives you less to keep track of as you read your code and try to figure out what it's going to do in any given situation.


Order loaded modules by amount of functions I am trying to solve some exercises from the "Programming Erlang" book.

One of them is "Write a function to determine which module exports the most functions"

This is my solution:

-module(module_info).
-export([modules_ordered_by_amount_of_functions/0]).

modules_ordered_by_amount_of_functions() ->
  Modules = loaded_modules_with_functions(),
  SortedModules = lists:sort(fun compare_functions_length/2, Modules),
  lists:map(fun extract_amount_of_functions/1, SortedModules).

loaded_modules_with_functions() ->
  LoadedModules = code:all_loaded(),
  LoadedModuleNames = lists:map(fun extract_module_name/1, LoadedModules),
  lists:map(fun extract_functions_from_module/1, LoadedModuleNames).

extract_module_name({ModuleName, _}) -> ModuleName.

extract_functions_from_module(ModuleName) ->
  [_, {exports, Functions} | _] = ModuleName:module_info(),
  {ModuleName, Functions}.

compare_functions_length ({_, FunctionsA}, {_, FunctionsB}) ->
  length(FunctionsA) >= length(FunctionsB).

extract_amount_of_functions ({ModuleName, Functions}) ->
  {ModuleName, length(Functions)}.

It doesn't really feel elegant to me.

Breaking the problem into tiny, useful functions is a very good way to write Erlang code. So your code is easy to read and gives us a great start in understanding and analyzing it.

The one thing I notice is that, in loaded_modules_with_functions, you map a function over a list and then map a function on the resulting list. This creates an intermediate list which will just be thrown away (GCed.) I'd combine the two operations so there's only one pass through the list. Since the result of this function is the resulting list, I'd use a list comprehension:

loaded_modules_with_functions() ->
  [ extract_functions_from_module(ModName) || {ModName, _} <- code:all_loaded() ].

In extract_functions_from_module, you're making an assumption that the export list is always the second element. Maybe this is the case. Or maybe in your OTP release this is always the case. The next release, however, may break your code if module_info adds more information. You can use the proplists module (in the standard library) to generalize the look-up:

extract_functions_from_module(ModuleName) ->
  {ModuleName, proplists:get_value(exports, ModuleName:module_info(), [])}.

Finally, I'd put a list comprehension at the end of modules_ordered_by_amount_of_functions and inline extract_amount_of_functions.


Elegantly avoid columns containing NaNs? The next bit of code does exactly what i want it to do, but it is 'rather' unelegant and unpythonic, so the question is quite simple: how can this code be changed into something more elegant

import numpy
from scipy.stats import nanmean
A = [[1, numpy.nan, 2],[2,3,4], [2,3,4], [numpy.nan, 5, 5]]
A1 = numpy.array(A)
print A1

def dosomething(d2):
    a0 = d2[0, :]
    a1 = d2[1, :]
    a2 = d2[2, :]
    dnan = ~numpy.isnan(a0+a1+a2)
    a01 = (a0-numpy.mean(a0[dnan]))/numpy.std(a0[dnan])
    a11 = (a1-numpy.mean(a1[dnan]))/numpy.std(a1[dnan])
    a21 = (a2-numpy.mean(a2[dnan]))/numpy.std(a2[dnan])
    l = nanmean(numpy.array([a01,a11,a21]), axis=0)
    return l

print dosomething(A1)

[[  1.  nan   2.]
[  2.   3.   4.]
[  2.   3.   4.]
[ nan   5.   5.]]

[-1.  0.  1.]

Edit: i simplified it a bit more

You can use d2[:, dnan] to select the wanted columns. (I'm assuming you want to process all rows unlike your current code)

def dosomething(d2):
    dnan = ~numpy.isnan(d2.sum(axis=0))
    validcols = d2[:, dnan]
    normalized = numpy.transpose((d2.T - numpy.mean(validcols, axis=1)) / numpy.std(validcols, axis=1))
    l = nanmean(normalized, axis=0)
    return l


Login modal with MVVM pattern I would like some feedback on whether this follows MVVM principles well.

The MainPage has a Login button that opens the login modal page. Upon successful login, the LoginPageViewModel will raise an event which is handled in the parent viewmodel.

I use Action objects to do the show/hide modals. The Actions are defined in a page's codebehind and invoked in its viewmodel.

MainPage.xaml

<Button Command="{Binding ShowLoginModalCommand}" Text="Login"/>

MainPage.xaml.cs

public MainPage()
{
    var vm = new MainPageViewModel();
    vm.ShowLoginModal += (lvm) => Navigation.PushModalAsync(new LoginPage(lvm));
    vm.HideLoginModal += () => Navigation.PopModalAsync();
    BindingContext = vm;
    InitializeComponent();
}

MainViewModel.cs

public Action<LoginViewModel> ShowLoginModal;
public Action HideLoginModal;

public ICommand ShowLoginModalCommand => new Command(() =>
{
    var lvm = new LoginViewModel();

    lvm.LoginSucceeded += (se, ev) =>
    {
        this.Customer = ev.Customer;
        HideLoginModal();
    };

    ShowLoginModal(lvm);
}

public Customer Customer
{
    get => _customer;
    set
    {
        _customer = value;
        OnPropertyChanged(nameof(Customer));
    }
}

LoginPage.xaml

<Button Command="{Binding LoginCommand}" Text="Login"/>

LoginViewModel.cs

public ICommand LoginCommand => new Command(async () =>
{
    var customer = await VerifyLogin(); // Assume valid login.
    LoginSucceeded(this, new LoginSuccessEventArgs(customer));
}

public class LoginSuccessEventArgs : EventArgs
{
    public Customer Customer { get; set; }
    public LoginSuccessEventArgs(Customer customer) => Customer = customer;
}
public event EventHandler<LoginSuccessEventArgs> LoginSucceeded;

Generally speaking I find a good implementation and like the simplicity of the approach. I would point out some problems I found though.

Inconsistent MVVM style

You are using two different ways to instantiate views and viewmodels and linking them together in both views, an inconsistence that could get bigger the and messier the more complex the program becomes.

In the MainPage you're creating the viewmodel in the view's constructor, and assigning it to the BindingContext right away. But in the LoginPage you create the viewmodel in the handler of the main page and pass it to the constructor of the view (not show here, but I guess it also assigns the BindingContext with it).

Specially for bigger applications, it's important to clearly define one style, either view first or viewmodel first, and live with it (I don't even know if it's called "style", but it sounds like that to me). A more concerning problem is that when VMs begin to take dependencies on services and other stuff, you most likely will want to use an IoC container for injecting them, and then it becomes more important to clearly define who'll create viewmodels and how. Personally, I like viewmodel-first approach, in which viewmodels create other viewmodels and are passed to the views.

MainPage viewmodel

Why are ShowLoginModal and HideLoginModal plain public fields? In situations like this, I would expect them to be normal events, raised from the viewmodel and handled in the view. This way feels a bit non-standard.
On the ShowLoginModalCommand method, the viewmodel creates the login viewmodel directly. Maybe its unnecesary now, but consider using an IoC container or other factory to create this, as generally when other viewmodels take extra dependencies it becomes harder to create them all inline.
The Customer setter doesn't needs to be public, a private one will do. You only want to set the current user from the login and nowhere else, so make that crystal clear.
The invocations of the ShowLoginModal and HideLoginModal delegates don't contain a null validation, and will result in a NullReferenceException if no one suscribed to them. A nice shortcut to call them safely is to use the ShowLoginModal?(lvm); syntax.

LoginPage viewmodel

Beware of the async reentrancy in the LoginCommand method. The VerifyLogin call inside it could potentially be a slow one, but since it's an await call, the UI will remain responsive. That means the user could do other things in the meanwhile, including changing the user/password or even click the login button again! That could have many unanticipated effects. I would counter it by disabling the UI while the verification takes place, using something like this in the VM:
public bool EnableForm { get; set; }
public ICommand LoginCommand => new Command(async () =>
{
    //Disable UI before doing anything
    EnableForm = false;
    OnPropertyChanged(nameof(EnableForm));
    var customer = await VerifyLogin(); // Assume valid login.
    LoginSucceeded(this, new LoginSuccessEventArgs(customer));
}

And in the view bind the controls to that property:
<Button Command="{Binding LoginCommand}" Text="Login" IsEnabled="{Binding EnableForm}"/>

In the LoginSuccessEventArgs the Customer property doesn't needs to be read-write. Once again, since you're just informing a value, there is no benefit in changing it after creation. Also, the constructor should check for null on the customer passed.
Possibly it's not shown here or just a work-in-progress thing, but there is no handling of the login failure case, that should just reenable the controls (following my above suggestion), show some error message to the user and not raise the LoginSuccess event.
Like in the main page, the raising of the event is not null checked and will blow up if nobody happens to be suscribed.
The call to the VerifyLogin method, although maybe simplified here, could imply a lot of work under the cover. That's the kind of thing that's better delegated to a service to do all the heavy work, and just have the presentation layer handle the visual thing. I would inject it in the constructor and call it passing the user/password entered, and it would return success or failure. Something along the lines of this:
private readonly ILoginService _loginService;
public LoginViewModel(ILoginService loginService)
{
    if(loginService == null) throw new ArgumentNullException(nameof(loginService));
    _loginService = loginService;
}
public ICommand LoginCommand => new Command(async () =>
{
    var customer = await _loginService.VerifyLogin(Username, Password);
    if(customer == null)
        //show some error message here
    else
        LoginSucceeded(this, new LoginSuccessEventArgs(customer));
}


Speed up a function using NumPy I have a function written in Python 2.7 that I call several times in a program. This function is rather slow, and I'd like to rewrite it in NumPy. However, I don't know how to do it.

I basically have a canvas of width * height pixels, and I'm making something rather complicated inside. I'd like to know how to replace all the parts with:

for i in range(x):
    do x[i]
    ...

by NumPy-style operations.

I know that I can start by making an array of zeros with np.zeros, but how can I implement the equations / operations in NumPy?

def Circular_cumsum_spot(width,height,centre,radius,spectrum,red_or_violet):
    """makes a "light spot" by cumsums
    from the center to the periphery

    width, height = size of canvas
    x, y = center of the spot
    radius = magnitude of the spot in pixels
    spectrum = light source
    red = to red, violet = to violet

    returns a numpy array"""

    x,y = centre[0], centre[1]

    data = []
    X = Y = Z = 0

    for i in range(height):
        for j in range(width):

            distance = radius - np.sqrt((i-x)**2+(j-y)**2)

            if distance >= radius:
                cumsum = 0
                X = Y = Z = 0
            else:
                cumsum = round((distance/radius) * (len(spectrum)))

                if red_or_violet == "red":

                    for k in range(cumsum):

                        X += spectrum[k][0]
                        Y += spectrum[k][1]
                        Z += spectrum[k][2]

                else:

                    for k in range(cumsum):

                        l = len(spectrum)-k-1

                        X += spectrum[l][0]
                        Y += spectrum[l][1]
                        Z += spectrum[l][2]

            data += [(X,Y,Z)]
            X = Y = Z = 0
    data = np.array(data)

    divisor = np.max(data[:,1])

    if divisor == 0:
        data = data
    else:
        data = np.divide(data, divisor)

    return data
    print "spot created is", red_or_violet

don't know how to create the XYZ list within numpy, but some other parts can be converted to numpy (see below). Didn't test the code, but the help pages for those numpy functions should help you, in case this doesn't run ;)
Instead of the double for-loop one can use np.nditer which might speed things up, but I haven't really used this much myself.

Otherwise, you want to remove for loops and just operate on the whole matrix or vector using numpy, e.g. calculate the distances in a single line instead of looping over it using two python loops. Using the axis=0 you can also remove the for loop for the spectrum variable.

def Circular_cumsum_spot(width,height,centre,radius,spectrum,red_or_violet):
    """makes a "light spot" by cumsums
    from the center to the periphery

    width, height = size of canvas
    x, y = center of the spot
    radius = magnitude of the spot in pixels
    spectrum = light source
    red = to red, violet = to violet

    returns a numpy array"""

    x,y = centre[0], centre[1]

    data = []
    X = Y = Z = 0

    # calc an 2d array of distances
    Xi, Yj = np.mgrid(0:height, 0:width)
    distance = np.hypot(Xi-y, Yj-x)

    # create a mask
    mask = distance < radius

    # calculate all cumsum values
    cumsum = np.zeros_like(distance)
    cumsum[mask] = np.round(distance[mask]/radius)*len(spectrum)

    # not sure, if this is already a numpy array
    spectrum = np.array(spectrum)

    for i in range(height):
        for j in range(width):
            if distance[i, j] >= radius:
                X = Y = Z = 0
            else:
                if red_or_violet == "red":
                    X, Y, Z = np.sum(spectrum[:cumsum[i, j],:], axis=0)
                else:
                    X, Y, Z = np.sum(spectrum[-cumsum[i, j]:,:], axis=0)
            data += [(X,Y,Z)]
    data = np.array(data)

    divisor = data[:,1].max()

    if divisor != 0:
        data = data/divisor

    return data
    print "spot created is", red_or_violet


Quick sort with a predicate I want to pass a predicate into quick sort. The issue I'm running into is that the predicate must have two arguments, the pivot and the element of the list's tail. As far as I know though, predicates can only have 1 argument. Here's what I have so far (it runs, I just want to improve it):

import Data.List

quicksort :: [a] -> (a -> a -> Bool) ->  [a]
quicksort [] _ = []
quicksort (x:xs) pf = quicksort left pf ++ [x] ++ quicksort right pf
    where   pred = pf x
            pt = partition pred xs
            left = fst pt
            right = snd pt

As 200 said, the predicate should be first. Think of filter, takeWhile, dropWhile and similar functions. They take the predicate first.

Also, it's usually a good idea to get rid of arguments that get repeated in every recursive call. GHC should take care of that, but we can help it a little bit:

quicksort :: (a -> a -> Bool) -> [a] -> [a]
quicksort pf = go
  where
    go [] = []
    go (x:xs) = go left ++ [x] ++ go right
      where (left, right) = partition (comp x) xs

However, we're sorting. Therefore calling the comparison function a  predicate is a misnomer. A predicate is a function a -> Bool, whereas a -> a -> Bool is a comparison. But a -> a -> Bool doesn't convey its meaning by its type. Do we return True if the first shall come first in the sorted arrangement? Or do we return False? Will quickSort (>) sort ascending or descending?

Those questions are easier to answer if we use a -> a -> Ord instead. This signature follows compare's, so we have a pretty good feeling what x `compare` y = LT means, namely that x < y holds true.

If we keep that in mind, we can write the following variants:

quicksortBy :: (a -> a -> Ord) -> [a] ->  [a]
quicksortBy comp = go
  where
    go []     = []
    go (x:xs) = go lesser ++ (x : equal) ++ go greater
      where
        (lesser, equal, greater) = partitionOrd (`comp` x) xs

quicksort :: Ord a => [a] -> [a]
quicksort = quicksortBy compare

The definition of partitionOrd :: (a -> Ord) -> [a] -> ([a], [a], [a]) is left as an exercise. Other than that, well done. Keep in mind that this isn't really Quicksort, though.


Game Engine :: Entity Component Design - Handling Input I am currently creating a game engine for educational purposes.

ECS Design Pattern

The first design pattern I've included is the Entity/Component/System pattern. Therefore I've got a base singleton class called cEngine, which contains all systems (PhysicsSystem, GraphicsSystem, etc.) so it can update them on each frame.

void cEngine::Run()
{
    while( m_Run )
    {
        m_Timer.Update();
        for(int i = 0; i < m_Systems.size(); ++i)
            m_Systems.at(i)->Update();
            //NOTE: I will add a SystemManager to update them in the proper order
    }
}

Also, I can create new entities (actually just IDs) and add different components (position, velocity, renderable, ...), which then get saved in the right system. So for example, the position and the velocity components will be saved in the physics system, as this one is responsible for modifying those values. Other systems, however, may have a pointer to those components, because they may need those values (the graphic system for example needs the positional information, but won't modify it).

State Design Pattern

The second design pattern I use is a state system:

class iState
{
    virtual void Init() {}
    virtual void Quit() {}
    virtual void Update() {}
    virtual void UpdatePause() {}
    virtual void Obscuring() {} //call when another state gets pushed
    virtual void Revealed() {} //call when upper state gets poped
private:
    bool m_isActive; //true, when it is at the top of the state stack
};

The user will be able to create customizable states (overwrite the virtual functions), which then get pushed onto the StateStack. Those functions will be called properly for every state by the StateManager class.

class cStateManager
{
    void Init();
    void Update();
    void Quit();

    void PushState( iState* state );
    void PopState();
    void ChangeState( iState* state );
private:
    std::vector<iState*> m_stateStack;
};

Questions

Is this the right choice? In my opinion this design is pretty good, as it is easy for the user and it should have good performance. However, I appreciate suggestions!
This is related to Input Handling. My design would've been a singleton Input class.

class cInput
{
    void Update();

    bool wasKeyPressed(int key);
    bool wasKeyReleased(int key);

    bool isKeyDown(int key);
    bool isKeyUp()int key);

private:
    //...
}

Once again, the engine class would Update() this class every frame, so it would always have the right key information. The huge benefit by not handling this class as a system, but as a segregated singleton class is, that the user simply can create a new state, create some entities in it, and easily handle input in the virtual Update() function of that state.

Something like this:

//overwriting virtual state functions
void Init(){
    id = cEngine->AddEntity();
}

void Update(){
    //add some components, bla bla...

    //simple input handling
    if(cInput->IsKeyDown(KEY_W)){
        //do something...
    }

    if(cInput->WasKeyPressed(KEY_A)){
        //do something
    }
}

As I've already said, in my opinion this "hybrid" design, which does not see the Input as another ECS system, but as a "independent" class is better and much more comfortable for the user than a "input component". However, I haven't seen this pattern on the Internet, which makes me a little bit insecure.

Did I miss one huge benefit of an input component or a huge disadvantage with this approach?

Not having seen a given pattern usually means that you didn't use exactly the right search string, or you've modified a pattern that already exists enough so it looks different, so I wouldn't worry about finding solutions that you haven't seen before.

Having a class handling input is the way that I'd initially go to design the structure, unless there was a particularly compelling reason to do it in another way. I'd have my key presses push an Event onto a stack, which can be popped somewhere else. So for instance, if the 'W' key was pressed, I'd push a "Forwards" event (or whatever was relevant), onto a message stack. This check would be within an Update() function of the Input class.

Later on, I'd pass either that message, or an interpretation of that message to the entity that it was relevant to so that it could handle the instruction. This would mean that as long as the message name doesn't change, it shouldn't affect the rest of the system what form of input you're using, as long as it sends those messages.


Format multiple dates to look good in text, using functional approach I had a piece of code that solved really simple problem  - it formated two dates to look better on a webpage. It looked like this:
function formatDateRange(
  days: { firstDay: string, lastDay: string }
): string {

  let first, last;
  first = dayjs(days.firstDay); //dayjs parses string that formatted like this "YYYY-MM-DD"
  last = dayjs(days.lastDay);   //and returns a wrapper for Date object
  if (first.isSame(last, "date")) {
    first = first.format("DD MMMM, YYYY")
    last = ""

  } else if (first.isSame(last, "month")) {
    first = first.format("DD") + " ... "
    last = last.format("DD, MMMM YYYY")

  } else if (first.isSame(last, "year")) {
    first = first.format("DD MMM") + " ... "
    last = last.format("DD MMM, YYYY")

  } else {
    first = first.format("DD MMM, YYYY") + " ... "
    last = last.format("DD MMM, YYYY")
  }
  return first + last
}

Than I decided to refactor it, and I got this:
//adapter that allows to use objects and arrays in function-composition
//EDIT: was called "callable"
function pluckFrom<T extends {}>(obj: T) {
  return (key: keyof T) => obj[key]
}

// a function for function-composition
// (same as compose from redux but order is reversed)
function pipe(...funcs: Function[]) {
  return funcs.reduce((f1, f2) => (...args: any) => f2(f1(...args)))
}

//provides an interface to apply array of functions to array of data
function apply<A>(maps: Array<(a: A) => any>) {
  return {to: (arr: A[]) => arr.map((item, index) => maps[index](item))}
}

//takes array of dates, wrapped in Dayjs object and returns
//true if they're all the same in terms of unit of time
function isSame([day1, ...rest]: Dayjs[]) {
  return (unit: UnitType) => rest.every(day => day.isSame(day1, unit))
}

interface DateRange {
  firstDay: string;
  lastDay: string;
}
//EDIT: removed redundant "same_" prefix
const case_to_format_map = {
  "date": ["DD MMMM, YYYY", " "],
  "month": ["DD ...", "DD, MMMM YYYY"],
  "year": ["DD MMM ... ", "DD MMM, YYYY"],
  " ": ["DD MMM, YYYY ... ", "DD MMM, YYYY"]
}

type Case = keyof typeof case_to_format_map

//EDIT: slightly refactored
function formatDateRange({firstDay, lastDay}: DateRange): string {

  const
    getCase = (days: Dayjs[]) => (
      (["date", "month", "year", " "] as UnitType[])
        .filter(isSame(days))[0]
    ) as Case
    ,
    makeFormatter = (f: string) => (d: Dayjs) => d.format(f)
    ,
    getFormatters = pipe(
      getCase,
      pluckFrom(case_to_format_map),
      formats => formats.map(makeFormatter)
    )
    ,
    days = [firstDay, lastDay].map(dayjs)

  return apply(getFormatters(days)).to(days).join(" ")
}

It's more code, but I like it because now I have 4 additional utility functions, it's more readable for me. Other than that, I'm not sure if I made better or worse.
And so I have these questions:
Is it readable to you?
Is it aligned with functional style and best practices?
I'm not sure if I will ever need to extend this code but how can I make it more extendable?

I personally prefer the first one, but this is very subjective. From a person reading the code for the first time, I can skim over the first version and have a good general idea of what the code is about. For the second version, I see a bunch of abstract functions and it takes me longer to see what the code actually does.
While the second version has some advantages, I don't believe it's worth it in this particular case unless you know something about the future of this code that I don't. I guess the point I'm trying to make is that, you should only reformat your code like that if you have a good reason for it. For example, you want to be able to change some part of the function behavior at runtime or when it's called from different parts of the codebase.


Building a string made easier? I have these lines of code in my view:

<?php
    $count = count($product->getTags());
    $tagsStr = '';
    foreach($product->getTags() as $key => $tag){
        $tagsStr.= " " . $tag->getTag();
        if(($key == 0 && $count < 1) || ($key == 0 && $count >1 && $key != $count)){
            $tagsStr .= ',';
        }
    } ?>

It prints strings like:

  Fish, Onions

Or

  Fish, Onions, Eggs

All these items are stored in an ArrayCollection $product->getTags();.

I find that these are a lot of lines for completing something this simple. I was wondering if you have ideas on simplifying this code.

Peter Kiss's code works, but it's not fast.

<?php

$tags = $product->getTags();
$c    = count($tags);
$str  = null;
for ($i = 0; $i < $c; ++$i) {
    if ($str) {
        $str .= ", ";
    }
    $str .= $tags[$i]->getTag();
}

?>

Yes, it’s not short (and not much different to your solution) but it’s highly efficient and therefor the solution I’d go for.


Generating words based on a list of letters I wrote a program to take a list of letters and from every permutation of them and check that against the dictionary and print out valid words.  The constraints are that there is a control letter which means that letter must be in the word, and you can't repeat any letters.

Everything works, but the running time is way too long.  I'm hoping to get some feedback on how to cut down my running time in O-notation. Also, if you know the running times of the built in function, that would be great. Also, comment if my code style is not what it should be for Python.

import itertools
from multiprocessing import Process, Manager

dictionary_file = '/usr/share/dict/words'
letter_list = ['t','b','a','n','e','a','c','r','o']
control_letter = 'e'

manager = Manager()
word_list = manager.list()

def new_combination(i, dictionary):
    comb_list = itertools.permutations(letter_list,i)
    for item in comb_list:
        item = ''.join(item)
        if(item in dictionary):
            word_list.append(item)
    return

def make_dictionary():
    my_list = []
    dicts = open(dictionary_file).readlines()
    for word in dicts:
        if control_letter in word:
            if(len(word) >3 and len(word)<10):
                word = word.strip('\n')
                my_list.append(word)
    return my_list

def main():
    dictionary = make_dictionary()
    all_processes = []
    for j in range(9,10):
        new_combo = Process(target = new_combination, args = (j,dictionary,))
        new_combo.start()
        all_processes.append(new_combo)
    while(all_processes):
        for proc in all_processes:
            if not proc.is_alive():
                proc.join()
        all_processes = [proc for proc in all_processes if proc.is_alive()]
        if(len(all_processes) == 0):
            all_processes = None

    print list(set(word_list))

if __name__ == '__main__':
    main()

import itertools
from multiprocessing import Process, Manager

dictionary_file = '/usr/share/dict/words'
letter_list = ['t','b','a','n','e','a','c','r','o']
control_letter = 'e'

By convention, global constant should be named in ALL_CAPS

manager = Manager()
word_list = manager.list()

def new_combination(i, dictionary):
    comb_list = itertools.permutations(letter_list,i)
    for item in comb_list:
        item = ''.join(item)
        if(item in dictionary):

You don't need the parens. Also, dictionary in your code is a list. Checking whether an item is in a list is rather slow, use a set for fast checking.

            word_list.append(item)

Having a bunch of different processes constantly adding onto a single list, performance will be decreased. You are probably better off adding onto a seperate list and then using extend to combine them at the end.

    return

There is no point in an empty return at the end of  a function

def make_dictionary():

I'd call this read_dictionary, since you aren't really making the dictionary

    my_list = []

Name variables for what they are for, not their types

    dicts = open(dictionary_file).readlines()
    for word in dicts:

Actually, you could just do for word in open(dictionary_file) for the same effect

        if control_letter in word:
            if(len(word) >3 and len(word)<10):
                word = word.strip('\n')

I recommend stripping before checking lengths, just to make it easier to follow what's going on

                my_list.append(word)
    return my_list

def main():
    dictionary = make_dictionary()
    all_processes = []
    for j in range(9,10):
        new_combo = Process(target = new_combination, args = (j,dictionary,))

You don't need that last comma

        new_combo.start()
        all_processes.append(new_combo)
    while(all_processes):

Parens unneeded

        for proc in all_processes:
            if not proc.is_alive():
                proc.join()
        all_processes = [proc for proc in all_processes if proc.is_alive()]

What happens if a process finishes between the last two lines? You'll never call proc.join() on it.

        if(len(all_processes) == 0):
            all_processes = None

an empty list is already considered false, so there is no point in this.

Actually, there is no reason for most of this loop. All you need is

for process in all_processes:
    process.join()

It'll take care of waiting for all the processes to finish. It'll also be faster since it won't busy loop.

    print list(set(word_list))

if __name__ == '__main__':
    main()

I've given a few suggestions for speed along the way. But what you should really do is invert the problem. Don't look at every combination of letters to see if its a word. Look at each word in the dictionary and see if you can form it out of your letters. That should be faster because there are many fewer words in the dictionary then possible combination of letters.


Color swatch selector with multiple mouseover and mouseout functions I am working on a simple mouseout and mouseover functions. Basically, I have color boxes (or swatches) and I have to get the color of the specific box when hovered and when selected. You can see how it works in the fiddle.

So I came out with a working script. But I am not sure how I can optimize or shorten the script since I will be having several color swatches. I made 2 samples swatches in this demo.

The question: is there a way to optimize/shorten it? Contents in the script were almost the same for each functions. The class names were the only ones changed.

/* COLOR SWATCH 1*/

$(".color").mouseover(function() {
   var hex = $( this ).css("background-color");
   var hexVal = $( this ).attr("value");
   var styles = {
      backgroundColor : hex
    };
    $('.selected-color').css(styles);
    $('.selected-color').text(hexVal);
});

$(".color").mouseout(function() {
    var hex = $('.active').css( "background-color");
    var hexValue = $('.active').attr("value");
    var styles = {
      backgroundColor : hex
    };
    $('.selected-color').empty();
    $('.selected-color').css(styles);
    $('.selected-color').text(hexValue);
});

$('.color').on('click', function() {
   $(this).addClass('active').siblings().removeClass('active');
    var hex = $(this).css("background-color");
    var hexVal = $('.active').attr("value");
    var styles = {
      backgroundColor : hex
    };
    $('.selected-color').css(styles);
    $('.selected-color').text(hexVal);
});

/* COLOR SWATCH 2*/

$(".color-2").mouseover(function() {
   var hexs = $( this ).css("background-color");
   var hexVal = $( this ).attr("value");
   var styles = {
      backgroundColor : hexs
    };
    $('.selected-color-2').css(styles);
    $('.selected-color-2').text(hexVal);
});

$(".color-2").mouseout(function() {
    var hex = $('.active-2').css( "background-color");
    var hexValue = $('.active-2').attr("value");
    var styles = {
      backgroundColor : hex
    };
    $('.selected-color-2').empty();
    $('.selected-color-2').css(styles);
    $('.selected-color-2').text(hexValue);
});

$('.color-2').on('click', function() {
   $(this).addClass('active-2').siblings().removeClass('active-2');
    var hex = $(this).css("background-color");
    var hexVal = $('.active-2').attr("value");
    var styles = {
      backgroundColor : hex
    };
    $('.selected-color-2').css(styles);
    $('.selected-color-2').text(hexVal);
});
.color-swatch div {
    width: 25px;
    height: 25px;
    float: left;
    border: 1px solid #313131;
    margin-right: 5px;
  }

.selected-color, .selected-color-2 {
    width: 90px;
    height: 20px;
    color: #FFF;
    text-align: center;
    padding: 2px;

}
.active, .active-2 {
    border: 3px solid #151515 !important;
}

SELECTED COLOR 1: Pink

SELECTED COLOR 2: Aqua

HTML abuse

The HTML5 specification does not allow a
 element to have a value attribute.  You can use a data-* attribute (named data-color, for example).  A title attribute would also be reasonable.

You are misusing CSS classes as IDs.  An ID is a unique name for an individual element.  A class is a set of elements that should all be treated alike.  You should not have color and color-2 as two class names.  Rather, they should be a single class.

Confused terminology

Your wording is a bit off, in my opinion.  Each individual sample is a "swatch".  A color selector is a widget consisting of three swatches.

When you make a decision by clicking on a swatch, the swatch isn't just "active" — I would say that that color has been selected.

Based on the above, I would rename…

the color and color-2 classes both to swatch
the color-swatch class to swatch-selector
the active class to selected

jQuery

Once you rename the classes so that both widgets have the same class names, it becomes possible to write code that treats the swatch selector as a generic widget.

If you need to define both a mouseover and a mouseout handler, use hover() instead.

$(function() {
    'use strict';

    function showActiveColor($selector) {
        var selectorId = $selector.attr('id');
        var $selected = $selector.find('.swatch.selected');
        $('#active-' + selectorId).empty()
                                  .text($selected.data('color'))
                                  .css({ backgroundColor: $selected.css('background-color') });
    }

    $('.swatch-selector .swatch').hover(
        function mouseOver(event) {
            var colorName = $(this).data('color');
            var color = $(this).css('background-color');
            var selectorId = $(this).closest('.swatch-selector').attr('id');
            $('#active-' + selectorId).text(colorName)
                                      .css({ backgroundColor: color });
        },
        function mouseOut(event) {
            showActiveColor($(this).closest('.swatch-selector'));
        }
    ).click(function onClick(event) {
        $(this).addClass('selected')
               .siblings().removeClass('selected');
    });

    // Initialization so that you don't have to hard-code the span's
    // background-color and text to match the initial selection
    $('.swatch-selector').each(function(index, el) { showActiveColor($(el)); });
});
.swatch-selector .swatch {
    width: 25px;
    height: 25px;
    float: left;
    border: 1px solid #313131;
    margin-right: 5px;
}
.swatch-selector .swatch.selected {
    border: 3px solid #151515;
}
.color-name {
    width: 90px;
    height: 20px;
    color: #FFF;
    text-align: center;
    padding: 2px;
}

SELECTED COLOR 1:

SELECTED COLOR 2:


My first login class in PHP with PDO and bcrypt This is the first time using a class. Please review this and tell me if it's secure and if it's the right way to do it.  The code itself is working, but I have doubts in the way I used all this.

// register.php
<?php
...
$db_password = $hash = password_hash($pass, PASSWORD_BCRYPT, array("cost" => 10));
....
// store in the 'users' table only the hash of the password provided by user.
?>

// clases/manageUsers.php

<?php
include_once('connect.php');

class ManageUsers{
public $link;
public $hash;

function __construct(){
    $db_connection = new dbConnection();
    $this->link = $db_connection->connect();
    return $this->link;
}
public function LogIn($id,$email,$pass,$hash){
    $query = $this->link->prepare("SELECT id,username,email,pass FROM users WHERE email = :email AND activated = 'y' LIMIT 1");
    $query->bindParam(':email', $email, PDO::PARAM_STR);
    $query->execute();
    $data = $query->fetch(PDO::FETCH_ASSOC);
    return $data;

        if(password_verify($pass,$hash)){
            return TRUE;
        }else{
            return FALSE;
        }
}

public function logOut($id){
    $query=$this->link->prepare("UPDATE users SET verify = '0', activenow = 'n' WHERE id= :id LIMIT 1");
    $query->bindParam(':id', $id, PDO::PARAM_INT);
    $query->execute();
    $countsOut = $query->rowCount();
    return $countsOut;
}

}
?>

// login.php

<?php
session_start();
error_reporting(E_ALL);
ini_set('display_errors', '1');

$errorMsg = '';
$email = '';
$pass = '';
$hash="";
$id='';
$token='';
$token = md5(uniqid(rand(),TRUE));

if (isset($_POST['e'])) {
$email = $_POST['e'];
$pass = $_POST['p'];

$email = stripslashes($email);
$pass = stripslashes($pass);
$email = strip_tags($email);
$pass = strip_tags($pass);

if ((!$email) || (!$pass)) {
    //header("location: index.php");
    $errorMsg = 'Please fill in both fields';

} else {
    include_once 'clases/manageUsers.php';
    include_once 'clases/connect.php';

$loginUsers = new ManageUsers();
$data = $loginUsers->LogIn($id,$email,$pass,$hash);
            $hash=$data['pass'];

    if(password_verify($pass,$hash) == TRUE){

        $db = new dbConnection();// <- New connection to database to update members table
        $dbh = $db->connect();   //  with new login data. I think this is WRONG...

            session_regenerate_id(TRUE);
            $id=$data['id'];
            $username=$data['username'];

            $query = $dbh->prepare("UPDATE users SET  verify= :token, lastlog=NOW(), activenow='y' WHERE id= :id LIMIT 1");
            $query->bindParam(':id', $data['id'], PDO::PARAM_INT);
            $query->bindParam(':token', $token, PDO::PARAM_STR);
            $query->execute();

                $_SESSION['id'] = $data['id'];
                $_SESSION['username'] = $data['username'];
                $_SESSION['token'] = $token;

            header("location: profile.php?id=$id");
            exit();

    } else {
            session_destroy();
            header("location: login.php");
            $errorMsg = "Incorrect login data, please try again";
    }
}
}
?>

//logout.php

<?php
session_start();
include_once 'clases/manageUsers.php';

if(isset ($_SESSION['id']) && isset ($_SESSION['username']) && isset ($_SESSION['token'])){
$id=$_SESSION['id'];
    $LgOut = new ManageUsers();
    $loggingOut = $LgOut->logOut($id);
    if($loggingOut > 0){
    session_destroy();
    session_regenerate_id(TRUE);
    header ('location: login.php');
    }else{
    echo 'Not Logged Out';
    }
}
?>

Your class manages Users. Its job is to handle users. So connecting to a database is not its job! Inject the database connection to the class like so:

public function __construct(\PDO $connection) {

Return values from __construct() are discarded when used from new Object.
When you return, the function ends. So in your LogIn method anything after return $data won't run. Basically, you won't verify the password, and $data will almost always mean true when checked against a boolean. (Basically, you are accepting any password to an existing account).
password_verify() returns a boolean, you can simplify to:

return password_verfiy($pass, $hash);

A user's logged in status should normally not be kept in the database, but in a session.
There's no point in initializing variables with '' if you're immediately overwrite those values with other literals. (It only makes sense if you append data to them in a loop or something similar).
Using stripslashes() and strip_tags means that passwords with certain characters in them would be changed. You don't care about a password having HTML tags because you are not displaying that password as HTML, ever.
if (SOMETHING == true) can be simplified to if (SOMETHING).
If you determined that the class's responsibility is to manage the authentication of your users, all of the authentication logic should be in that class. It's pointless if you do it outside the class :)
What's the point of a token in a session? A token is usually used in a cookie for the purpose of "remember me". It has no meaning in a session.
"Not logged out" is not really a good error message. Explain what went wrong.
Coding style, spacing and indentation - functionNames() and $variableNames should be camel cased, ClassNames should be camel caps. Indent your lines according to the structure block they are found in. Sometimes you use param,param,param and sometimes param, param, param. Pick one and stick with it.


Web-uploading of Cyrillic-named images For web-uploading of Cyrillic-named images, I've written the script below:

#!/usr/bin/python
# coding=utf-8

import os
import argparse
import shutil

class Error: pass
class FolderError(Error): pass

def copytranstree (src, dst):
   names = os.listdir( src )

   try:
       os.makedirs( dst )
   except WindowsError:
       pass

   errors = []

   for name in names:

       srcname = os.path.join( src, name )

       dstname = os.path.join( dst, ''.join([translit.get(x, x) for x in name.decode( input_parametres.encoding ).lower()]).encode( input_parametres.encoding ) )

       try:
           if os.path.isdir( srcname ):
               copytranstree( srcname, dstname )
           else:
               shutil.copy2( srcname, dstname )
       except (IOError, os.error), why:
           errors.append( ( srcname, dstname, str( why ) ))
       except Error, err:
           errors.extend( err.args[0] )

   try:
       shutil.copystat( src, dst )
   except WindowsError:
       pass
   except OSError, why:
       errors.extend( (src, dst, str(why) ) )
   if errors:
       raise Error( errors )

parser = argparse.ArgumentParser(description='Copy directory tree with transliterated pathnames')
parser.add_argument('-e','--encoding', help='File system encoding', default='utf-8')
parser.add_argument('-s','--src', help='Source folder name', default='./' )
parser.add_argument('-d','--dst', help='Destination folder name', default='./')

input_parametres = parser.parse_args()

translit = dict(zip(u"абвгдеёзийклмнопрстуфхьъ", "abvgdeeziyklmnoprstufh'~"))
translit.update({u'ц': 'tz', u'ч': 'ch', u'ш': 'sh', u'ы': 'yi',
                u'э': 'ye', u'ю': 'ju', u'я': 'ja', u' ': '_'})

src = os.path.abspath( input_parametres.src )
dst = os.path.abspath( input_parametres.dst )

if not os.path.exists( dst ):
   dstname = ''.join([translit.get(x, x) for x in dst.decode( input_parametres.encoding ).lower()]).encode( input_parametres.encoding ).lower()
   os.makedirs( dstname )
   dst = os.path.abspath( dstname )

if not os.path.exists( src ) or os.path.isfile( dst ):
   raise FolderError()

copytranstree( src, dst )

The script transliterates and copies a tree of files from a source folder to destination, considering encoding passed to the script (UTF for Ubuntu, CP121 for Windows).

Any ideas for common improvement of structure for Linux OS?

All exceptions in python should
inherit from an Exception class.
WindowsError only works on Windows, its better to use the more general OSError
You twice have a very long line to transliterate a string. Put it in a function and call it
You should put any code that is part of the script proper in a main function.
You shouldn't throw exceptions that you know won't be caught.
The way you collect errors by throwing them up the stack is ugly.
If you split the walking logic from the file operation logic the code will be clearer.

My updates (completely untested and probably wrong):

#!/usr/bin/python
# coding=utf-8

import os
import argparse
import shutil

class CommandError(Exception):
    """
    Thrown when the user has made a mistake in the command line
    """

def transliterated_walk(encoding, src, dst):
    """
    A generator which will walk over src producing them
    along with the transliterated versions in dst
    """
    files = []
    for filename in os.listdir(src):
        src_path = os.path.join(src, filename)
        dest_path = os.path.join(dest, transliterated(encoding, filename))

        if os.path.isdir(path):
            for entry in transliterated_walk(encoding, src_path, dest_path):
                yield entry
        else:
            files.append( (src_path, dest_path) )
    yield src, dest, files

def copytranstree (encoding, src, dst):
    errors = []

    for src_path, dest_path, filenames in transliterated_walk(encoding, src, dest):
        try:
           os.makedirs( dest_path )
        except WindowsError:
           pass

        for src_filename, dest_filename in filenames:
            try:
                shutil.copy2( src_filename, dst_filename )
            except (IOError, os.error) as why:
                errors.append( (src_filename, dest_filename, str(why) )

        try:
           shutil.copystat( src, dst )
        except OSError as why:
           errors.append( (src_path, dst_path, str(why) ) )

TRANSLIT = dict(zip(u"абвгдеёзийклмнопрстуфхьъ", "abvgdeeziyklmnoprstufh'~"))
TRANSLIT.update({u'ц': 'tz', u'ч': 'ch', u'ш': 'sh', u'ы': 'yi',
                u'э': 'ye', u'ю': 'ju', u'я': 'ja', u' ': '_'})

def transliterate(encoding, string):
    decoded = string.decode(encoding).lower()
    transliterated = ''.join(translit.get(x, x) for x in decoded)
    return transliterated.encode(encoding).lower()

def main():
    try:
        parser = argparse.ArgumentParser(description='Copy directory tree with transliterated pathnames')
        parser.add_argument('-e','--encoding', help='File system encoding', default='utf-8')
        parser.add_argument('-s','--src', help='Source folder name', default='./' )
        parser.add_argument('-d','--dst', help='Destination folder name', default='./')

        input_parametres = parser.parse_args()

        src = os.path.abspath( input_parametres.src )
        dst = os.path.abspath( input_parametres.dst )

        if not os.path.exists( dst ):
           dstname = transliterate(dst)
           os.makedirs( dstname )
           dst = os.path.abspath( dstname )

        if not os.path.exists( src ):
            raise CommandError('%s does not exist' % src)
        if os.path.isfile( dst ):
            raise CommandError('%s is a file' % dest)

        copytranstree( encoding, src, dst )
    except CommandError as error:
        print error

if __name__ == '__main__':
    main()


Block same instance of a program for running again I am implementing a library (Linux) and I created some functions to block new instances of a running program and I was wonder if there are some better improvements for it.

Here is a program which describes what I am trying:

#include <errno.h>
#include <fcntl.h>
#include <stdio.h>
#include <string.h>
#include <stdlib.h>
#include <unistd.h>
#include <sys/wait.h>

#define SECONDS 20

static char *program_path = NULL;

char *my_strtok         ( char *const msg, const char *const ch );
void block_new_instance ( const char *const instance );
void clean_instance     ( void );

int main ( int argc, char *argv[] )
{
    if ( argc != 1 )
    {
        printf( "\n\t*** Arguments are NOT allowed. ***\n" );
        exit( EXIT_FAILURE );
    }

    block_new_instance( argv[ 0 ]  );
    sleep( SECONDS );
}

void block_new_instance( const char *const instance )
{
    char prog_name[ strlen( instance ) + 1 ];
    memset( prog_name, '\0', sizeof( prog_name ) );
    strcpy( prog_name, instance );
    char *buffer = my_strtok( prog_name, "/" );
    struct flock file_lock;
    char *dir = getenv( "HOME" );

    if ( dir == NULL || dir[0] != '/' )
    {
        fprintf( stderr, "Wrong Directory, getenv(): %s (%d)\n", strerror( errno ), errno );
        exit( EXIT_FAILURE );
    }

    program_path = calloc( sizeof ( *program_path ), strlen( dir ) + ( strlen( buffer )  + sizeof ( "/" ) ) );
    if ( program_path == NULL )
    {
        printf( "Error, malloc()\n" );
        exit ( EXIT_FAILURE );
    }

    memcpy( program_path, dir, strlen( dir ) );
    memcpy( program_path + strlen( dir ), "/", sizeof( "/") );
    memcpy( program_path + ( strlen( dir ) + strlen( "/" ) ), buffer, strlen( buffer ) );

    int file_desk = open( program_path, O_RDWR | O_CREAT, 0600 );
    if ( file_desk < 0 )
    {
        fprintf( stderr, "open: %s (%d)\n", strerror( errno ), errno );
        exit( EXIT_FAILURE );
    }

    file_lock.l_start   = 0;
    file_lock.l_len     = 0;
    file_lock.l_type    = F_WRLCK;
    file_lock.l_whence  = SEEK_SET;

    if ( fcntl( file_desk, F_SETLK, &file_lock ) < 0 )
    {
        fprintf( stderr, "%s is already running\n", buffer );
        exit( EXIT_FAILURE );
    }
    atexit( clean_instance );
}

char *my_strtok( char *const msg, const char *const ch )
{
    char *ret = NULL;
    char *tmp = strtok( msg, ch );
    while ( tmp != NULL )
    {
        ret = tmp;
        tmp = strtok( NULL, ch );
    }

    if ( ret == NULL )
    {
        return NULL;
    }
    return ret;
}

void clean_instance( void )
{
    unlink ( program_path );
    free   ( program_path );
}

Possible Outputs are:

  *** Arguments are NOT allowed. **

or:

  Program is already running

I would like to know which improvements are needed?

Three calls to memcpy seem to emulate sprintf(program_path, "%s/%s", dir, buffer);
my_strtok is a not very clean substitute for dirname.
The lock file is always created in the home directory, and only accounts for the base name of the executable. It means that /usr/foo would block /opt/foo.
The locker does not account for the links (again, it only cares about the base name of the executable). Different names may refer to the physically same file; invocations via links would not lock each other out.
A callback registered with atexit is only guaranteed to be called if the program exits normally. If the program is terminated by the signal, the lock file would not be removed.


Edit button toggles modal and load data from PHP using AJAX and JQUERY I've been working on an edit/update button that will toggle a modal that looks like this:

Is there a better code for this JavaScript code that I made?

<script>

$('body').on('click', '.editButton',function(){
    let edit = $(this).val()
    console.log("Edit this : " + edit);

    $('#updateEmpModal').modal('toggle');

    $.ajax({
        type    :   'POST',
        url     :   'get.php',
        data    :   {'edit' : edit},
        dataType:   'json',
        success: function(data){
            $("#updateEmpForm input[name='EMAIL']")         .val(data.EMAIL);
            $("#updateEmpForm input[name='PASSWORD']")      .val(data.PASSWORD);
            $("#updateEmpForm input[name='RIGHTS']")        .val(data.RIGHTS);
            $("#updateEmpForm input[name='LAST_NAME']")     .val(data.LAST_NAME);
            $("#updateEmpForm input[name='FIRST_NAME']")    .val(data.FIRST_NAME);
            $("#updateEmpForm input[name='MIDDLE_NAME']")   .val(data.MIDDLE_NAME);
            $("#updateEmpForm input[name='SUFFIX']")        .val(data.SUFFIX);
            $("#updateEmpForm input[name='GENDER']")        .val(data.GENDER);
            $("#updateEmpForm input[name='BIRTHDATE']")     .val(data.BIRTHDATE);
            $("#updateEmpForm input[name='BIRTHPLACE']")    .val(data.BIRTHPLACE);
            $("#updateEmpForm input[name='CITIZENSHIP']")   .val(data.CITIZENSHIP);
            $("#updateEmpForm input[name='RELIGION']")      .val(data.RELIGION);
            $("#updateEmpForm input[name='ADDRESS']")       .val(data.ADDRESS);
            $("#updateEmpForm input[name='CONTACT']")       .val(data.CONTACT);
            $("#updateEmpForm input[name='ICE_NAME']")      .val(data.ICE_NAME);
            $("#updateEmpForm input[name='ICE_CONTACT']")   .val(data.ICE_CONTACT);
            $("#updateEmpForm input[name='DEPARTMENT']")    .val(data.DEPARTMENT);
            $("#updateEmpForm input[name='POSITION']")      .val(data.POSITION);
            $("#updateEmpForm input[name='EMP_TYPE']")      .val(data.EMP_TYPE);
            $("#updateEmpForm input[name='edit']")          .val(data.ID);
        }
    });
});

</script>

It is working, but I want to know if this is a bad practice and if there are better solutions.

Welcome to Code Review! Hopefully you enjoy using this site and receive valuable feedback.

Wow, what patience you have to type out/copy all of those lines to update the form inputs. Instead of doing that, you could loop through the input fields and check if the name matches a key in the returned data. One can use .each() to iterate over the input fields, then check the name using .attr() and the in operator.

$('#updateEmpForm input').each(function() {
    const inputName = $(this).attr('name');
    if (inputName in data) {
        $(this).val(data[key]);
    }
});

To exclude certain fields, like any password inputs, the selector could be updated to include pseudo-class selectors like `:not() - for example:

$('#updateEmpForm input:not([type="password"])').each(function() {

That should handle all but the last repeated line - which can be included before or after the call to .each():

$("#updateEmpForm input[name='edit']").val(data.ID);

You could also define a mapping of input names to keys and look in the mapping for values. For example:

const inputNameKeyMapping = {
    edit: 'ID',
    //any other names that don't match keys directly
}

And use that mapping when assigning the value - something like:

$('#updateEmpForm input').each(function() {
    const inputName = $(this).attr('name');
    const key = inputNameKeyMapping[inputName] || inputName;
    if (key in data) {

That way you wouldn't need to include manual value settings lines.

Also, instead the click handler can be simplified from:

$('body').on('click', '.editButton',function(){

To using the .click()  (short-cut) method on only elements with that class name editButton:

$('.editButton').click(function(){

I see let is used to declare the variable edit:

let edit = $(this).val()

However, that value is never re-assigned. To avoid unintentional re-assignment, you could use const instead.

const edit = $(this).val()

And it is best to be consistent with line terminators - if most lines have them then make sure all do.


Three-player number-guessing game This is a sample exercise tutorial that would be used to test understanding of how simple classes communicate/message and are designed.

Tester class

public class GuessingGameTester {

    public static void main(String[] args){
        //create a new game
        GuessingGame g1 = new GuessingGame();

        //call the start method
        g1.startGame();

    }
}

Player class

public class GuessingGamePlayer {

    boolean areTheyRight = false;

    //player random guess
    int getPlayerGuess(){
        return (int) (Math.random()* 20);
    }
}

Game class

public class GuessingGame {

    private int numberToGuess;
    private boolean keepPlaying;
    private int round;

    //constructor
    GuessingGame(){
        //create a random number for players to guess
        numberToGuess = (int) (Math.random() * 20);//cast to int
        keepPlaying = true;
        round = 1;
    }

    void startGame(){

        //create 3 players to play
        GuessingGamePlayer p1 = new GuessingGamePlayer();
        GuessingGamePlayer p2 = new GuessingGamePlayer();
        GuessingGamePlayer p3 = new GuessingGamePlayer();

        System.out.println("The number to guess is : " + numberToGuess);

        //giving 100 attemps for the computer to guess the correct number
        while(keepPlaying && round <= 100){

            //get their guess
            int p1guess = p1.getPlayerGuess();
            int p2guess = p2.getPlayerGuess();
            int p3guess = p3.getPlayerGuess();

            System.out.println("round " + round);
            System.out.println("p1 guessed: " + p1guess);
            System.out.println("p2 guessed: " + p2guess);
            System.out.println("p3 guessed: " + p3guess);

            System.out.println("\n");

            if(p1guess == numberToGuess){
                p1.areTheyRight = true;
                keepPlaying = false;
            }else if(p2guess == numberToGuess){
                p2.areTheyRight = true;
                keepPlaying = false;
            }else if(p3guess == numberToGuess){
                p3.areTheyRight = true;
                keepPlaying = false;
            }

            round ++;
        }//end while

        //game over - echo who guessed the correct num and the round
        System.out.println("\n");
        if(p1.areTheyRight){
           System.out.println("p1 guessed the correct number during round " + round);
        }else if(p2.areTheyRight){
            System.out.println("p2 guessed the correct number during round " + round);
        }else{
            System.out.println("p3 guessed the correct number during round " + round);
        }

    }//end startGame
}

I'd add only a couple of minor things.

The comments are more trouble than they're worth. I understand it's a tutorial, but if they're going to be in there, they should do more--right now they don't add value. Since it's a learning exercise, use those comments to teach them something, including terminology.

// Create a new GuessingGame instance to represent the current game.
// Begin playing by calling the startGame() method.

But I'd leave them out altogether, and have the corresponding text/lecture do the describing; you will achieve more by writing or talking about them outside of the code.

Secondly, I'd consider keeping the players in an array or a list. If not now, then in "part two". This simplifies the code conceptually, because you can directly represent "for each player, check their guess" in code. If not in "part one", then I'd at least move the guess-getting into another method--you only need to return a boolean, the players hold whether or not it's a right guess.

(IMO the player shouldn't maintain that state, rather the game should.)

Next, I'd refactor more; the startGame does more than starts the game. Either refactor, or rename. Right now, startGame starts, plays, and ends the game. By refactoring, the code becomes more story-like, at the cost of more methods. But good methods should be tightly focused and easy to reason about.

On a more theoretical level, the end conditions as implemented makes a certain assumption about the nature of the game. Asking if anyone can spot a potential "gotcha" with the current implementation might be a good "extra credit" problem.

In real life, what would happen if two players both guessed correctly? In the current implementation, the players are always checked in numerical order. In real life, a typical game would be either who guessed the fastest (which I supposed the player order could be said to represent), or multiple players could win, or...?

To put that into "real life" terms, let's say the guesses are access control votes, or search result weightings, etc. In those cases, a tie does matter. So instead of having a single winner, there would be a collection of "winners". How to decide if there should be a single winner, or a group, depends on the nature of the game.

Lastly, I'd make some of the things more configurable, although perhaps with reasonable defaults. This also mirrors real life a bit more accurately, and I tend towards adding a bit of complexity early, since it can be ignored.

Some of my suggestions are more appropriate for a "second round" implementation, while some make sense for what you have now. Pick and choose, or ignore them all :) I've appended everything in a single blob.

public class GuessingGameTester {

    public static void main(String[] args){
        GuessingGame game = new GuessingGame();
        game.startGame();
    }

}

public class GuessingGamePlayer {

    private int playerNumber;

    public GuessingGamePlayer(int playerNumber) {
        this.playerNumber = playerNumber;
    }

    int getGuess(){
        return new Random().nextInt(GuessingGame.MAX_GUESSING_NUMBER) + 1;
    }

    @Override
    public String toString() {
        return "Player " + playerNumber;
    }

}

public class GuessingGame {

    private int numberToGuess;
    private int roundsToPlay;
    private int numberOfPlayers;
    private List<GuessingGamePlayer> players;

    public static int MAX_GUESSING_NUMBER = 20;

    public GuessingGame() {
        init(100, 3);
    }

    public GuessingGame(int roundsToPlay, int numberOfPlayers) {
        init(roundsToPlay, numberOfPlayers);
    }

    public void init(int numberOfRounds, int numberOfPlayers) {
        this.roundsToPlay = numberOfRounds;
        this.numberOfPlayers = numberOfPlayers;

        numberToGuess = new Random().nextInt(MAX_GUESSING_NUMBER) + 1;

        initPlayers();
    }

    public void initPlayers() {
        players = new ArrayList<GuessingGamePlayer>();
        for (int i = 0; i < numberOfPlayers; i++) {
            players.add(new GuessingGamePlayer(i+1));
        }
    }

    public void startGame() {
        int currentRound = 0;
        GuessingGamePlayer correctGuesser = null;
        System.out.println("The number to guess is: " + numberToGuess);

        while((correctGuesser == null) && (currentRound <= roundsToPlay)) {
            ++currentRound;
            System.out.println("\nRound " + currentRound);

            for (GuessingGamePlayer player : players) {
                int playerGuess = player.getGuess();
                System.out.println("  " + player + " guessed " + playerGuess);
                if (playerGuess == numberToGuess) {
                    correctGuesser = player;
                }
            }
        }

        System.out.println();

        if (correctGuesser == null) {
            System.out.println("Nobody guessed the right number!");
        } else {
            System.out.println(correctGuesser + " guessed the number in round " + currentRound);
        }
    }

}


N-dimensional array in Rust I am practicing Rust by creating data structures, such as this N-dimensional array.
The purpose of this structure is to easily define and address into arbitrarily-deeply nested arrays without having to write out Vec<Vec<Vec<T>>> and incur the performance penalty coming from this (although performance is not a priority).
I am looking for feedback on how idiomatic the interface and implementation are,
as well as how to improve my tests since I don't think they're very robust.
use std::ops::{Index, IndexMut};

#[derive(Debug, Clone)]
pub struct NdArray<T, const N: usize> {
    dim: [usize; N],
    data: Vec<T>,
}

impl<T, const N: usize> NdArray<T, N> {
    fn calculate_capacity(dim: &[usize; N]) -> usize {
        let mut cap = 1;
        for &x in dim {
            cap = usize::checked_mul(cap, x).expect("vector capacity overflowed usize");
        }
        cap
    }

    pub fn new(dim: [usize; N], default: T) -> Self
    where
        T: Clone,
    {
        let cap = Self::calculate_capacity(&dim);
        NdArray {
            dim,
            data: vec![default; cap],
        }
    }

    pub fn new_with(dim: [usize; N], generator: impl FnMut() -> T) -> Self {
        let cap = Self::calculate_capacity(&dim);
        NdArray {
            dim,
            data: {
                let mut v = Vec::new();
                v.resize_with(cap, generator);
                v
            },
        }
    }

    fn get_flat_idx(&self, idx: &[usize; N]) -> usize {
        let mut i = 0;
        for d in 0..self.dim.len() {
            assert!(
                idx[d] < self.dim[d],
                "index {} is out of bounds for dimension {} with size {}",
                idx[d],
                d,
                self.dim[d],
            );
            // This cannot overflow since we already checked product of all dimensions fits usize
            // and idx < self.dim.
            i = i * self.dim[d] + idx[d];
        }
        i
    }
}

impl<T, const N: usize> Index<&[usize; N]> for NdArray<T, N> {
    type Output = T;

    fn index(&self, idx: &[usize; N]) -> &T {
        &self.data[self.get_flat_idx(idx)]
    }
}

impl<T, const N: usize> IndexMut<&[usize; N]> for NdArray<T, N> {
    fn index_mut(&mut self, idx: &[usize; N]) -> &mut T {
        let i = self.get_flat_idx(idx);
        &mut self.data[i]
    }
}

#[cfg(test)]
mod tests {
    use super::*;
    use std::panic::catch_unwind;

    #[test]
    fn nd_array_1d() {
        let mut array = NdArray::new_with([10], || 1);
        assert_eq!(array[&[0]], 1);
        assert_eq!(array[&[9]], 1);
        assert!(catch_unwind(|| array[&[10]]).is_err());

        array[&[5]] = 99;
        assert_eq!(array[&[4]], 1);
        assert_eq!(array[&[5]], 99);
        assert_eq!(array[&[6]], 1);
    }

    #[test]
    fn nd_array() {
        let mut array = NdArray::new([2, 3, 4], 0);
        array[&[1, 2, 3]] = 10;
        assert_eq!(array[&[1, 2, 3]], 10);
        assert_eq!(array[&[1, 0, 0]], 0);
        assert!(catch_unwind(|| array[&[2, 0, 0]]).is_err());
        assert_eq!(array[&[0, 2, 0]], 0);
        assert!(catch_unwind(|| array[&[0, 3, 0]]).is_err());
        assert_eq!(array[&[0, 0, 3]], 0);
        assert!(catch_unwind(|| array[&[0, 0, 4]]).is_err());
    }

    #[test]
    fn nd_array_overflow() {
        // Panics at the allocator code since sizeof(usize) > 2
        // NdArray::new([usize::MAX / 2], 0);
        assert!(catch_unwind(|| NdArray::new([usize::MAX / 2 + 1, 2], 0)).is_err());
        assert!(catch_unwind(|| NdArray::new(
            [usize::MAX / 3, usize::MAX / 3, usize::MAX / 3 + 1],
            0
        ))
        .is_err());
    }
}

Nice example of const generics! In general your code is well organized and idiomatic. Some comments:

In your new() function, you are using an argument as a default value. It's more idiomatic to use the Default trait for this:
pub fn new(dim: [usize; N]) -> Self
where
    T: Clone + Default,
{
    let cap = Self::calculate_capacity(&dim);
    NdArray {
        dim,
        data: vec![Default::default(); cap],
    }
}

In some of your unit tests this means you have to use the turbofish to initialize: e.g. NdArray::new becomes NdArray::<usize, 2>::new. However, this only occurs when the user does not access / modify anything in the NdArray, and if they do that then type inference kicks in. So I recommend providing both options to the user, say NdArray::new(dim: ...) for T: Default and NdArray::new_fillwith(dim: ..., default: T) if the user wants to specify their own default value.

Your structure could use some more functionality. For any collection of T, .len() and .iter() would be the minimum:
pub fn len(&self) -> usize {
    self.data.len()
}

pub fn iter(&self) -> impl Iterator<Item = &T> {
    self.data.iter()
}

What about adding two nd-arrays? Multiplying by a constant? Concatenating them along a certain dimension? Generalized matrix multiplication? Some of these will probably require certain utility functions.

For example the following would probably be quite useful and a good exercise to write: iteration along a sub-dimensional slice of the array, like pub fn iter_subarray(&self, coords: &[Option<usize>; N]) -> impl Iterator<Item = &T> where if the coord is None then you iterate along all coordinates in that dimension, but if it's Some(x) then that coordinate is fixed.

You probably want to #[derive(PartialEq, Eq)] in addition to Clone and Debug.

Both the implementation and unit tests could use some handling of edge cases, in particular when the dimension is empty and when any of the dimension lengths is zero. Whether you allow these cases is up to you; I would personally allow the former but not the latter. Here's a unit test:
#[test]
fn nd_array_edgecases() {
    let array_dim0 = NdArray::<usize, 0>::new([]);
    assert_eq!(array_dim0.len(), 1);
    assert_eq!(array_dim0[&[]], 0);
    let array_dim1 = NdArray::<usize, 1>::new([0]);
    assert_eq!(array_dim1.len(), 0);
    assert!(catch_unwind(|| array_dim1[&[0]]).is_err());
}

Great job with the unit tests for vector overflow, that is super useful.

In get_flat_idx, Clippy recommends: for (d, &dlen) in self.dim.iter().enumerate() (then use dlen instead of self.dim[d]). I agree, though in this case we still have to use d to index into idx.

In your implementation, the field .data is not intended to dynamically change size. For this reason, you could initialize with Vec::with_capacity(n) instead of Vec::new() in your new_with() function. You might also consider using a fixed-size or fixed-capacity array implementation, for example the ArrayVec crate.


Simple function to turn array of objects to array of key value pairs I have an application that has a lot of very complex objects that in most cases I only need to access an id value and a name value from. I'd like to reduce the overhead of serializing these large objects to JSON.  This is a simple function I created that takes an array of objects and returns an array of structures that contains only two keys, an id and a value.  This also needs to be compatible with CF8 servers.

I'd like to get some feedback on ways to make the code more expressive and any ways I can optimize this as well.

<cffunction name="toKeyValuePairArray" returntype="Array" output="false" access="public">
    <cfargument name="theArray" type="Array" required="true" hint="an array of objects" />
    <cfargument name="idGetterFunctionName" type="string" />
    <cfargument name="valueGetterFunctionName" type="string" />
        <cfset var local = structNew() />
        <cfset var retArray = [] />
        <cfset var obj = {} />
        <cfloop array="#arguments.theArray#" index="local.theComponent">
            <cfset obj = {} />
            <cftry>
                <cfinvoke component="#local.theComponent#" method="#arguments.idGetterFunctionName#" returnvariable="local.id" />
                <cfinvoke component="#local.theComponent#" method="#arguments.valueGetterFunctionName#" returnvariable="local.value" />

                <cfset obj.id = local.id />
                <cfset obj.value = local.id />

                <cfset arrayAppend(retArray,obj) />
            <cfcatch></cfcatch>
            </cftry>
        </cfloop>

        <cfreturn retArray />
</cffunction>

Ryan,

Here's how I would do it. Since you're making something backwards compatible with CF8, which does not explicitly have the 'Local' scope, and you've already declared it via the 'var', then we'll put all other function local variables within this struct, and only return that which is needed.

<!---
 *  FUNCTION toKeyValuePairArray
 *  Method to create a simple object reference array to reduce overhead and improve performance.
 *  This assumes that your objects have Getters for their primary properties.
 *  (no need to instantiate every object, every time, when all you need is a number or a name)
 *
 *  @theArray (array) - an array of objects
 *  @idGetterFunctionName -  Function name for getter function that pulls the object's id
 *  @valueGetterFunctionName - Function name for getter function that pulls the object's name
 --->
<cffunction name="ToKeyValuePairArray" returntype="array" output="false" access="public">
    <cfargument name="theArray" type="Array" required="true" hint="an array of objects" />
    <cfargument name="idGetterFunctionName" required="true" type="string" hint="Function name for getter function that pulls the object's id" />
    <cfargument name="valueGetterFunctionName" required="true" type="string" hint="Function name for getter function that pulls the object's name" />

    <cfset var LOCAL = StructNew() />
    <cfset LOCAL.retArray = ArrayNew(1) />

    <cfloop array="#ARGUMENTS.theArray#" index="LOCAL.theComponent">
        <cfset LOCAL.tmpObjVals = StructNew() />
        <cftry>
            <cfinvoke component="#LOCAL.theComponent#" method="#ARGUMENTS.idGetterFunctionName#" returnvariable="LOCAL.id" />
            <cfinvoke component="#LOCAL.theComponent#" method="#ARGUMENTS.valueGetterFunctionName#" returnvariable="LOCAL.value" />

            <cfset LOCAL.tmpObjVals["id"] = LOCAL.id />
            <cfset LOCAL.tmpObjVals["value"] = LOCAL.value />

            <cfset ArrayAppend(LOCAL.retArray,Duplicate(LOCAL.tmpObjVals)) />
            <cfcatch><!--- Empty Catch ---></cfcatch>
        </cftry>
    </cfloop>

    <cfreturn LOCAL.retArray />
</cffunction>

Since we 'var' one core variable (creating our own function local scope, so to speak) we no longer have to declare the temp object at the top of the method, since you reset it at the top of each loop iteration anyway. I also standardized your struct and array initialization across the method, to maintain that backward compatibility (and standardization of form is just good practice).

You aren't really doing any additional processing on the return variables from your 'getter' calls, so I applied them directly to the tempObj, so we don't create any unnecessary variables. You don't have to upper case scope names. I do that to make them stand out better when looking at code. I did uppercase the function names. This is just something I got from college: begin uppercase the function calls, and begin lowercase on variable names, then camel case the remainder.

NOTE: Adjusted some of my code, to work in some of Dan's (good) suggestions. I understand the 'try/catch' conceptually, but I did make them required args so it would throw errors if you didn't have them.


Robot OOP Class in Python Hello everyone!
Here is the thing. So I wanted at first to create Robot class, I want him to move around some board. Maybe 10x10 or something like that, then i wanted to invent a Robot fights class, I want to add class players (becuase i want to play between player 1 and player 2) but here it is what I have so much.
Lets focus on this code because I want it to make it much better then start to create robot fights!
class Robot:

    def __init__(self, name, place, start = (0,0), power = 9):
        self.name = name
        self.start = start
        self.place = place
        self.power = power

    def get_name(self):
        return self.name

    def get_start(self):
        return self.start

    def get_place(self):
        return self.place

    def get_power(self):
        return self.power

    def set_name(self, x):
        if isinstance(x, str):
            self.name = x
        else:
            raise TypeError("must be a string")

    def set_start(self, x):
        if isinstance(x, tuple):
            self.start = x
        else:
            raise TypeError("must be a tuple")

    def set_place(self, x):
        if isinstance(x, list):
            self.place = x
        else:
            raise TypeError("must be a list")

    def set_power(self, x):
        if isinstance(x, int):
            self.power = x
        else:
            raise TypeError("must a int")

    def check_power(self):
        if self.power <= 0:
            raise ValueError("No power")

    def left(self, value):
        self.check_power()

        self.power -= value

        if self.place[0] - value < 0:
            self.place[0] = self.place[0] - value + 8

        else:
            self.place[0] = self.place[0] - value

    def up(self, value):
        self.check_power()

        self.power -= value

        if self.place[1] + value > 7:
            self.place[1] = self.place[1] + value - 8

        else:
            self.place[1] = self.place[1] + value

        if self.place[1] == 5:
            self.power += 2

    def __str__(self):
        return self.name, self.place, self.power

Also want I want to make better in this one. Well power will be important in the Robot fights, because if some robot from player 1 will be near to robot from player 2 I want them to you know, fight, so the power will be pretty good right there, if they will be near to each other the power will decrease until the robot is destroyed. But lets focus on above code to make it better.
Any tips how to make this SHORTER and more neat, closer to a advanced or just better solution will be definitely on point.
Have a great day!

Type annotations
Using type annotations will make your program a lot more readable and easier to debug. For example:
def __init__(self, name, place, start = (0,0), power = 9):
    # assignments

becomes
def __init__(self, name: str, place : List[int], start: Tuple[int, int] = (0,0), power: int = 9):
    # assignments

I imported List, Tuple from the typing module from the standard library:
from typing import List, Tuple

Here's some further information on using type annotations.

Getter and Setter
Using getters and setters is rather unpythonic. You can instead modify the getting and setting behaviours by using the @property decorator like this:
def __init__(self, name: str, place : List[int], start: Tuple[int, int] = (0,0), power: int = 9):
    self._name = name

    # further assignments

@property
def name(self):
    return self._name

@name.setter
def name(self, value):
    if isinstance(value, str):
        self._name = value
    else:
        raise TypeError("must be a string")

This will allow you to use the properties without explicitly calling getter and setter methods:
my_robot.name = 'foo'
print(my_robot.name)

This question on StackOverflow contains some more detailed explanations.

Logic

You should also take a look at your setters for place and start.
You only check the type of the passed argument value, but do not
verify if the elements in the list or tuple are of type int.
You could consider using a custom class Position or similiar for
handling the positional logic of the robot. That way you can access
the different dimensions by their names instead of indices of
place (self.place[0] or self.place[1]). You'll also be able to put further logic (like clipping, etc.) into it.
power and check_power(): Depending on your intended functionality you might want to limit the number of steps that can be taken by a robot to the power that it has left. As it is now, a robot can take any number of steps as long as it has power > 0 left.
You should check the functionality left(value) and up(value) for big values, especially values that are > 2 * board_dimension. I suspect the results might be unintended.