Skrev tenta i grundläggande Java, kritik någon?

Eftersom det är grundkurs så hoppar jag över allt om OO och design etc.
Ser helt ok för å vara nybörjarkod men några saker.

• Klassnamnet är på svenska, aj aj :s

• Kommentarer på svenska, brukar vara bra å hålla sig till engelska, men det kan man ha överseende med här iof eftersom det är en tenta, vem vet, läraren kanske suger på engelska

• Query metoden har börjar med bokstav, lite petigt men men

• Skulle nog döpt om getAnswer till något annat, försöka endast använda metoder med get/setX för just getters och setters, ingen större grej men iaf.

• Strängarna i början av main() bör du istället deklarera som konstanter eftersom de ändå inte ska ändras, blir snyggare så.

• Main bör även inte gör annat än att "starta" själva programmet, även om du inte har så mkt kod där hade lösningen varit OO så hade detta varit mer självklart dock, så är väl ingen större grej nu

• query-metoden är rätt stor, hade nog varit snyggare att dela upp den lite mer genom att göra mindre metoder, en för varje ändåmål typ lite "single responsibility" tänk.

Annars överlag helt oki, inget som direkt va jättekonstigt, vilket brukar förekomma hos nybörjare hade själv löst den på ett lite annorlunda sätt men men, har man mer erfarenhet så är det ju lättare att se andra lösningar hoppas det gick bra för dig!

• Vid en första titt känns det som om det är väl mycket kommentarer. En kommentar om varför du inte använde \t är bra. En kommentar som berättar vad catch (Throwable badInput) gör för något är dålig. Man ska kunna förutsätta att den som läser koden vet vad catch gör, så en extra kommentar förvirrar bara - tex är getAnswer dubbelt så lång mot vad det skulle vara utan kommentarer.

• askWhichTable krashar programmet om man skriver in en bokstav.

• Varför skickar du runt alla texterna? Det tillför ingenting i ditt fall, bara att du måste skicka in ett helt gäng argument till dina metoder. Du skulle lika gärna kunna flytta ut dom till samma ställe som numberOfQuerys finns.

• När du bygger en sträng i en loop brukar det vara bäst o använda en StringBuilder istället för att bygga strängen med + eftersom kompilatorn inte själv kan optimera det till en stringbuilder och + är väldigt långsamt. (i just ditt fall spelar det iof inte jättestor roll, inte som att programmet går trögt ändå)

• Alla dina textsträngar ligger på en ensam rad vilket gör dom väldigt jobbiga o läsa.

• Verkar som om du försökt använda 80 tecken per rad. Personligen tycker jag att max 80 tecken är lite föråldrat nu för tiden, speciellt i Java som är ett så pratigt språk med pratiga konventioner.

Kan vara ja men med tanke på att det inte stod nåt om det i citatet från läraren så är det tveksamt, tror mer på att det var ett val gjort av TS. Om nu så var fallet är det ju bara att ignorera punkten

Hoppas det var en av d första kurserna iaf inte bra med dåliga vanor, kan dock erkänna att mina variabelnamn i grundkursen i C var mindre genomtänkta

Man kan skriva om practice_on_table genom att förkorta hur errors och multipliers skapas. På så sätt behövs inte gen_multipliers och koden blir lite enklare och kortare:

def practice_on_table(table, max_multiplier=10):
    errors = [0] * max_multiplier
    multipliers = range(1,max_multiplier+1) * 3
...

För övrigt tror jag den gör lite fel, svarar man fel får man ju samma fråga igen tills man klarar den. Det känns som en förenkling jämfört med ursprungsuppgiften

Lite onödigt att använda rekursion i rehearsal oxå, är man riktigt usel (fel ~1000 gånger i rad) kraschar programmet.

Jag skrev samma tenta så jag postar min lösning också, hoppas det är lugnt! Använde inte swing så programmet är inte så spännande att använda, men men

Gjorde en fullösning för vektorn som lagrar felaktiga svar. Ville göra den lika stor som antalet fel, men tidspressen ställde till det så jag skapade den med 100 element... Sen tolkade jag uppgiften som så att varje tabell går upp till 12, så svarar man rätt 36 gånger (12*3) är man färdig med tabellen.

Har tagit bort alla kommentarer, dels för att koden var överkommenterad (på begäran av läraren), och dels för att de var skrivna under stress ( = rörigt värre ).

import java.util.*;

public class MultiplicationTrainer
{
	public static void showStat (int[] result, int[] wrong, int table)
	{
		System.out.println();
		System.out.println("*******************");
		System.out.println("Antal rätt: " + result[0]);
		System.out.println("Antal fel: " + result[1]);
		System.out.println("*******************");
		System.out.println();
		
		System.out.println("Du hade fel på följande tal:");
		System.out.println();
		
		for (int i = 0; i < wrong.length; i++)
		{
			if (wrong[i] == 0)
			{
				break;
			}
			
			System.out.println("|*|\t" + wrong[i] + "x" + table + "\t|*|");
		}	
	}
	
	public static void multiTable (int table)
	{	
		Scanner in = new Scanner(System.in);
		
		int[] noOfTimes = new int[36];
		int[] result = new int[2];
		int[] wrong = new int[100];
		
		int count = 0;
		int answer;
		
		System.out.println("Du har valt " + table + ":ans tabell!");
		
		while (count < 36)
		{
			int num = genNum();
			
			if (countNum(noOfTimes, num) == 3)
			{
				continue;
			}
			
			String equation = num + "x" + table;
			
			System.out.println();
			System.out.println("Vad blir " + equation + "?");
			System.out.print("Svar: ");
			answer = in.nextInt();
			in.nextLine();
			
			if (answer == 0)
			{
				break;
			}
			
			if (answer == (num * table))
			{
				System.out.println("Rätt svar!");
				addNum(noOfTimes, num);
				result[0]++;
				count++;

			}

			else 
			{
				System.out.println("Fel svar!");
				addNum(wrong, num);
				result[1]++;
			}
		}
		
		showStat(result, wrong, table);
	}
	
	private static void addNum (int[] array, int num)
	{
		for (int i = 0; i < array.length; i++)
		{
			if (array[i] == 0)
			{
				array[i] = num;
				break;
			}
		}
	}
	
	private static int countNum (int[] noOfTimes, int num)
	{
		int count = 0;
		for (int i = 0; i < noOfTimes.length; i++)
		{	
			if (noOfTimes[i] == num)
			{
				count++;
			}
		}
		
		return count;
	}
	
	private static int genNum ()
	{
		int num;
		num = (int) (Math.random() * 12) + 1;		
		
		return num;
	}
	
	public static void main (String args[])
	{
		Scanner in = new Scanner(System.in);
		
		int choice = 1;
		
		while (choice != 0)
		{
			System.out.println();
			System.out.print("Vilken multiplikationstabell vill du öva på? (1 - 12, avsluta med 0): ");
			choice = in.nextInt();
			in.nextLine();
			System.out.println();

			switch (choice)
			{
			case 1:
				multiTable(choice);
				break;
			
			case 2:
				multiTable(choice);
				break;
				
			case 3:
				multiTable(choice);
				break;
			
			case 4:
				multiTable(choice);
				break;
			
			case 5:
				multiTable(choice);
				break;
			
			case 6:
				multiTable(choice);
				break;
			
			case 7:
				multiTable(choice);
				break;
				
			case 8:
				multiTable(choice);
				break;
				
			case 9:
				multiTable(choice);
				break;
				
			case 10:
				multiTable(choice);
				break;
			
			case 11:
				multiTable(choice);
				break;
				
			case 12:
				multiTable(choice);
				break;
			
			case 0:
				choice = 0;
				break;
				
			default:
				System.out.println("Välj ett tal mellan 1 - 12 eller 0 för att avsluta!");
				break;
				
			}
		}	
	}
}

...

Inte orkat gå in på så mycket detaljer, man kan säga att din switch sats i main är helt onödig iaf

Oj, nej det var inte så smart, vet inte hur jag tänkte...

Något i den här stilen hade kanske vart bättre?

			if (choice == 0)
			{
				break;
			}
			else 
				if (choice >= 1 && choice <= 12)
				{
					multiTable(choice);
				}
			else
			{
				System.out.println("Välj ett tal mellan 1 - 12 eller 0 för att avsluta!");
			}

Edit: Med lösningen ovan kanske jag skulle haft "while(true)" istället för "choice != 0" också.

switch(choice) {
   case 1:
   case 2:
   case 8:
               multiTable(choice);
               break;
   default:             
                 ....
}

Behöver du nånsin switch-satser är det ett tecken på att du gjort fel.. tex
Fel sätt

public void DoSomething(Somethings whichSomething) 
{
   switch(whichSomething)
   {
      case Somethings.DoOne:
         //code
      break;
      case Somethings.DoTwo:
        //Code
      break;
   }
}

Rätt sätt (Finns fler rätt sätt förstås)

public class DoIt
{
   private readonly IEnumerable<IDoSomethingHandler> doSomethingHandlers;

   public DoIt(IEnumerable<IDoSomethingHandler> doSomethingHandlers)
   {
      this.doSomethingHandlers = doSomethingHandlers;
   }

   public void DoSomething(Somethings whichSomething)
   {
      var handler = doSomethingHandlers.Single(handler => handler.Handels == whichSomething);
      handler.Execute();
   }
}

public void DoSomething(Somethings whichSomething) 
{
   switch(whichSomething)
   {
      case Somethings.DoOne:
         //code
      break;
      case Somethings.DoTwo:
        //Code
      break;
   }
}

public class DoIt
{
   private readonly IEnumerable<IDoSomethingHandler> doSomethingHandlers;

   public DoIt(IEnumerable<IDoSomethingHandler> doSomethingHandlers)
   {
      this.doSomethingHandlers = doSomethingHandlers;
   }

   public void DoSomething(Somethings whichSomething)
   {
      var handler = doSomethingHandlers.Single(handler => handler.Handels == whichSomething);
      handler.Execute();
   }
}

*ehm* Kursen var i "Grundläggande Java"

Sorry jag kan inte Java, men C# och Java har rätt lik syntax...

switch(choice) {
   case 1:
   case 2:
   case 8:
               multiTable(choice);
               break;
   default:             
                 ....
}

Tack för tipset! Man har ju en del att lära sig, speciellt så här i början... Fortsättningskursen går in på OOP, det ska bli intressant!

Jag snackar om switchar lilla vännen.. inte uppgiften som sådan.. Ett factory pattern av något slag är oftast bättre än en switchar eller ifs

Det är mitt ditt tankesätt som vi får klasser på 1000 rader och mer.

public DoIt(IEnumerable<IDoSomethingHandler> doSomethingHandlers)

Denna kollektion injiceras med ett DI verktyg, vilket betyder att varje handler kan ha helt egna beroenden vilket gör varje implementation av en handler mindre komplex, ökar separationen och på köpet får man ökad läsbarhet. Men du min vän jobbar ju inte i stora enterprise projekt där man som arkitekt måste tänka på att man har 10 eller fler utvecklare som ska utveckla mot samma kodbas, det hör ju jag. Jag jobbar som lösningsarkitekt i ett större projekt, och det är åsikter som dina att företagen har massor med spaghettikod. Men nu ska vi inte bråka

edit: Du har dock rätt i att det inte är 100% ett factory pattern, ett dynamiskt strategy pattern beroende på indata?

Och jag är glad att jag inte jobbar med dig (Det du kallar för jobb är väll att sitta i ett garage och jobba på timme för 400 kronor), du är uppenbart inte en professionell utvecklare och vet inte hur man bygger upp en arkitektur för stora projekt. Exemplet är inte uselt, däremot har du rätt i att det är off topic, så vi kanske ska sluta gnabbas och låta TS ha sin tråd?

Jag självgod, kul, du är den som ofta är självgod i de trådar jag sett i denna del av forumet.

Finns en anledning till att enterprisey ofta nämns på thedailywtf.. Och det beror inte på att typiska enterprise-lösningar är enkla och tydliga